[needs-packaging] nemos-dev-key

Hello Laider, I hope this finds you well.

I performed a comprehensive review of your package, and it is completely Lintian-clean, matching (and exceeding) my expectations for sponsorship of a package. Thank you.

I will leave the final determination to the Ubuntu Archive Administrators on whether it is in our interest to ship this source package, but from a packaging perspective, I see no reason to hold it back.

A few small notes:
 - I've added this bug to the latest changelog entry.
 - *Usually*, I see a single "Initial release" changelog entry, but in this case I am willing to accept that this may have been in an OEM archive of some sort before this. Not sure if this is a hard requirement, but I'm looking past it.
 - Usually, source NEW packages are uploaded to Debian. That being said, I understand this is an exceptional circumstance, being an OEM package. Looking past it.
 - It would be a good idea to set yourself as an Uploader and a team as the Maintainer, in a future upload. This is simply best practice, looking past it.
 - I really like to see autopkgtests, even if it's just a minimal, 50 line Bash script testing the validity of the keys. It really wouldn't hurt, but it's a wishlist item.
 - I usually don't see copyright definitions in control; for files under debian/, it is usually assumed the copyright declaration for that is handled in debian/copyright (using a wildcard for the whole folder). What you currently have in debian/copyright is completely acceptable.

All of these are extremely specific nitpicks. Great job, uploaded to Noble. :)

Shipping this package in stable releases may be done with appropriate version numbers, at the same time as (or following) the acceptance of this package into Noble. From my preliminary code inspection, the regression potential of including this package in a stable release is slim-to-none.

Thanks.

*** This is an automated message ***

This bug is tagged needs-packaging which identifies it as a request for a new package in Ubuntu. As a part of the managing needs-packaging bug reports specification, https://wiki.ubuntu.com/QATeam/Specs/NeedsPackagingBugs, all needs-packaging bug reports have Wishlist importance. Subsequently, I'm setting this bug's status to Wishlist.

*Since the development series is moved to Noble. Update the description target to Noble.

Hi Simon (tsimonq2),

Thanks for your sponsored.
- *Usually*, I see a single "Initial release" changelog entry, but in this case I am willing to accept that this may have been in an OEM archive of some sort before this. Not sure if this is a hard requirement, but I'm looking past it.
=> [Laider] Yes, this package comes from an OEM archive for an OEM project.

- It would be a good idea to set yourself as an Uploader and a team as the Maintainer, in a future upload. This is simply best practice, looking past it.
=> [Laider] Thanks, the uploader and maintainer are updated.
Maintainer: NemOS Team <email address hidden>
XSBC-Original-Maintainer: Isaac True <email address hidden>
Uploaders: Isaac True <email address hidden>

===some tests log with latest version===
$ lintian nemos-dev-key_1.3_source.changes
<no E/W/I>
$ lintian nemos-dev-key_1.3+nemos~202311221124~ubuntu24.04.1_all.deb
<no E/W/I>
$ lintian nemos-dev-cert_1.3+nemos~202311221124~ubuntu24.04.1_all.deb
<no E/W/I>
$ lintian nemos-dev-cert-u-boot_1.3+nemos~202311221124~ubuntu24.04.1_all.deb
<no E/W/I>

$ autopkgtest . -- lxd images:ubuntu/24.04
autopkgtest [11:07:18]: starting date and time: 2023-11-22 11:07:18+0100
autopkgtest [11:07:18]: version 5.28ubuntu1
autopkgtest [11:07:18]: host Isaac-Laptop; command line: /usr/bin/autopkgtest . -- lxd images:ubuntu/24.04
autopkgtest [11:07:48]: testbed dpkg architecture: amd64
autopkgtest [11:07:51]: testbed running kernel: Linux 6.5.0-10-generic #10-Ubuntu SMP PREEMPT_DYNAMIC Fri Oct 13 13:49:38 UTC 2023
autopkgtest [11:07:51]: @@@@@@@@@@@@@@@@@@@@ built-tree .
autopkgtest [11:07:51]: testing package nemos-dev-key version 1.3
autopkgtest [11:07:51]: test verify: preparing testbed
Reading package lists...
Building dependency tree...
Reading state information...
Starting pkgProblemResolver with broken count: 0
Starting 2 pkgProblemResolver with broken count: 0
Done
0 upgraded, 0 newly installed, 0 to remove and 0 not upgraded.
1 not fully installed or removed.
After this operation, 0 B of additional disk space will be used.
Setting up autopkgtest-satdep (0) ...
(Reading database ... 15779 files and directories currently installed.)
Removing autopkgtest-satdep (0) ...
autopkgtest [11:07:59]: test verify: [-----------------------
Signature Verified Successfully
autopkgtest [11:07:59]: test verify: -----------------------]
autopkgtest [11:08:00]: test verify: - - - - - - - - - - results - - - - - - - - - -
verify PASS
autopkgtest [11:08:00]: @@@@@@@@@@@@@@@@@@@@ summary
verify PASS

Hi, this hasn't been accepted yet from the Ubuntu NEW queue. When I look at this package I'm puzzled. If this is a snakeoil key that everyone has access to the private part of, why does it need to be in the archive? I guess that might be because you want the artifacts from the other packages named to be signed by it consistently so that (for dev purposes only) the hardware can have the single public key enrolled and consume them directly, without having to do local re-signing. Do I have that right?

Hi Steve,

Yes, you are right.
This package is a dependency for LP: #2034642, LP: #2034648, and LP: #2039037 to be signed.

Therefore, this package should be uploaded to the archive before LP: #2034642, LP: #2034648, and LP: #2039037.

If there are no other concerns, I will request the sponsor team's help to upload again. Tks.

Reference the comments from other [needs-packaging] cases for the same project.
Refine debian/* and bump to the v1.4 version.

Please help to review and upload to the noble queue again if there are no concerns. Tks.

Good to see an autopkgtest included, unfortunately it's testing the keys in the source package when it really needs to be testing those installed as part of the binary package. This also requires that the verify test depends on "@" (if Depends is specified, it doesn't implicitly include the binary packages created by the package).

In other words:

* d/t/control should specify Depends: @, openssl

* d/t/verify should use the keys in /usr/share/nemos/{public,private}.pem rather than the ones in the current dir (which is the checkout of the source package)

The d/copyright looks better but I'm a little confused that the license header is declared "GPL-2.0+" while the test declares specifically "version 2", not "version 2 or, at your option, any later version". However, that's probably irrelevant as I'm also told that it's policy that new packages originating from Canonical (i.e. without a prevailing upstream license) should be licensed GPL-3.0 only (no "later version") so this should probably all just be changed to that.

I'll unsubscribe ubuntu-sponsors for now, but please do subscribe it again when it's ready for another review.

Thanks for the comment.

The d/t/* and d/copyright are fixed in v1.5.
Please help to review again. Tks.

$ head debian/control
# SPDX-License-Identifier: GPL-2.0-or-later
# Copyright 2023 Canonical Ltd.

Source: nemos-dev-key
Section: admin
Priority: optional
Maintainer: Isaac True <email address hidden>
Standards-Version: 4.6.2
Build-Depends: debhelper-compat (= 13)
Rules-Requires-Root: no
$

Do not put SPDX tags in debian packaging files. This is a reject, please upload without this.

The debian/copyright also lists GPL-2.0+ as the license. Canonical's license policy is GPL-3 by default - not GPL-3+, not GPL-2, not GPL-2+. If you have an exception for your project to license this as GPL-2+, please point to relevant documentation internally.

(Separately, it is questionable whether anything here is copyrightable under US law. It may be copyrightable under other jurisdictions.)

debian/rules:

override_dh_auto_install:
        install -Dm0644 rsa2048_private.pem \
                debian/nemos-dev-key/usr/share/nemos/private.pem
        install -Dm0644 rsa2048_public.pem \
                debian/nemos-dev-cert/usr/share/nemos/public.pem
        install -Dm0644 u-boot-signature.dtsi \
                debian/nemos-dev-cert-u-boot/usr/share/nemos/u-boot-signature.dtsi

I would encourage you to do this with debian/$pkg.install files and use dh_exec to handle the renaming of files. But this is not a blocker for accepting.

Thanks for the review and comments on the nemos-dev-key (1.2).

The SPDX tags of d/control and GPL-3 of d/copyright are fixed in v1.5.
The dh_exec of d/install is fixed in v1.6.

The "Ubuntu sponsors" is resubscribed, could anyone from this group help to upload nemos-dev-key v1.6 to the noble queue? Tks.

The v1.6 passed the lintian checking and autopkgtest.
autopkgtest [04:35:47]: test verify: [-----------------------
Signature Verified Successfully
autopkgtest [04:35:47]: test verify: -----------------------]
verify PASS
autopkgtest [04:35:47]: test verify: - - - - - - - - - - results - - - - - - - - - -
autopkgtest [04:35:47]: @@@@@@@@@@@@@@@@@@@@ summary
verify PASS

I'm getting all files installed in nemos-dev-key and the two other packages are empty:
W: nemos-dev-cert: empty-binary-package
W: nemos-dev-cert-u-boot: empty-binary-package

Did you perhaps not git add newly added debian/*.install files?

Thanks, I miss the d/*.install files with the correct package names.
It's fixed in v1.7 and passed the lintian checking and autopkgtest.

Tks.

I was a little confused as to why the GPL license in d/copyright refers specifically to "Indicator Weather" which typically isn't in the GPL3 boilerplate (and which doesn't appear related to this package). Still, the rest of the package looks reasonable, so I'll patch d/copyright to contain just the standard GPL3 boilerplate and sponsor this.

Thanks, I believe it's a mistake from my side when copy-and-paste the wrong GPL-3 content. Sorry for that and it is fixed in v1.8.

Hi, I saw that Laider addressed the last remaining issue called here (the specific debian/copyright Indicator Weather copy-paste typo); reviewing only this change compared to 1.7, I sponsored the delta.

Since this is a new package and the first time I am sponsporing it, I took a wider look and noted the following opportunities for improvement:
 * the control file lists Isaac as original maintainer with his Canonical address; this should be removed
 * I've personally just now subscribed to the nemos-team@LP mailing-list, perhaps Laider and others should too
 * description of the package is a bit terse, should mention that it is used to sign the boot in generated images for the NemOS project
 * subjective/personal perspective: I suspect this is broken down into too many very small packages; could probably be just a single package
 * there is no explanation/documentation for the Provides; this is likely a case of someone being able to craft their own key-carrying deb and host that privately (would need to think of hosting this so that only CI/CD for image generation could get access to the private key!)
 * there is no documentation as to where the pre-built files (rsa2048_private.pem, rsa2048_public.pem, u-boot-signature.dtsi) are coming from, nor how these were generated; these are fairly obvious (perhaps the u-boot device tree source snippet a bit less), but would seem useful

This looks ok now and I am accepting, however:

$ head u-boot-signature.dtsi -n3
// SPDX-License-Identifier: GPL-2.0-or-later
// Copyright 2023 Canonical Ltd.
// Generated using ubootpubkey
$

You still have a wrong SPDX header in the file, even though debian/control is correct.

(Also, asserting copyright over a number is bullshit, but meh.)

Hi Loic,

Thanks for the input.
I fixed suggestions in v1.9.
And I also subscribed the nemos-team@LP mailing-list.

Fixed items:
1. Correct SPDX header(GPL-3) in the u-boot-signature.dtsi (for comment#16)
2. Isaac is removed from the maintainer
3. Refine the description of d/control with NemOS information
4. Add a README file to explain the "Providers" and how to generate the development key files

About the packages to change to a single one, as a discussion at yesterday.
We can keep the current design at first, and we should construct a better way to handle the key for boot assets and kernel FIT.