1830978 - p7sign: add -a hash and -u certusage (also p7verify cleanups)

Status: RESOLVED FIXED

(NSS :: Tools, enhancement, P3)

Description

[наб]

Attachment: hg export -r 16526:baddc0143bbf -r 16527:58b095cee576 -r 16528:780e5c084021 -r 16529:4f7236f7bb06 -r 16530:42d9c9b3c5e1 -r 16531:6d5a6e5169ea

Steps to reproduce:

Added -a hash and -u certusage flags to p7sign.

Actual results:

I'm using
p7sign -k 'babtop DB 2023' -i zfs.ko -o zfs.ko.p7s -a sha256 -u 6
with
sign-file -s zfs.ko.p7s sha256 db2023.der zfs.ko
(wrapped up) to sign kernel modules from an NSS-managed key store on my laptop.

Expected results:

I've opened a set of... bugs? patches? differentials? at https://phabricator.services.mozilla.com/D174327
https://phabricator.services.mozilla.com/D174328
https://phabricator.services.mozilla.com/D174329
https://phabricator.services.mozilla.com/D174330
https://phabricator.services.mozilla.com/D174331
https://phabricator.services.mozilla.com/D174332

A patch is (hopefully) attached, but this patchset can be obtained from https://hg.sr.ht/~nabijaczleweli/nss as

changeset: 16531:6d5a6e5169ea
tag: tip
user: наб <[e-mail]>
date: Fri Mar 31 21:32:08 2023 +0200
summary: p7sign: fix usage string

changeset: 16530:42d9c9b3c5e1
user: наб <[e-mail]>
date: Fri Mar 31 20:57:27 2023 +0200
summary: tests/smime: exercise p7sign -u and -a

changeset: 16529:4f7236f7bb06
user: наб <[e-mail]>
date: Fri Mar 31 20:56:44 2023 +0200
summary: p7verify: handle sha{256,384,512,224} signatures

changeset: 16528:780e5c084021
user: наб <[e-mail]>
date: Fri Mar 31 20:56:01 2023 +0200
summary: p7verify: don't atoi(strdup(optarg))

changeset: 16527:58b095cee576
user: наб <[e-mail]>
date: Fri Mar 31 20:55:31 2023 +0200
summary: p7verify: actually accept -u IPSec, per usage string

changeset: 16526:baddc0143bbf
user: наб <[e-mail]>
date: Fri Mar 31 03:42:17 2023 +0200
summary: p7sign: add -u to specify usage and -a to specify the hash algorithm

Comment 1

[Anna Weine]

Could I suggest adding the bug id to your patches?

How: you can edit any patch you created. In the edit section there is Bugzilla ID.

Comment 2

[наб]

Yep, clicked through all of them now.

Comment 3

[наб]

Attachment: p7sign: add -a hash and -u certusage (also p7verify cleanups)

With this patchset, you can do p7sign -k 'babtop DB 2023' -i zfs.ko -o zfs.ko.p7s -a sha256 -u 6, then sign-file -s zfs.ko.p7s sha256 db2023.der zfs.ko to sign kernel modules from NSS-managed key stores.

Comment 4

[наб]

Attachment: p7verify: actually accept -u IPSec, per usage string

Comment 5

[наб]

Attachment: p7verify: don't atoi(strdup(optarg))

Comment 6

[наб]

Attachment: p7verify: handle sha{256,384,512,224} signatures

Comment 7

[наб]

Attachment: tests/smime: exercise p7sign -u and -a

Comment 8

[наб]

Attachment: p7sign: fix usage string

Currently, nothing within it corresponds to itself or the actual option parsing.

Comment 9

[John Schanck [:jschanck]]

I updated the metadata in phabricator so that these patches depend on one another. Use moz-phab patch D174328 to update your local copy. The workflow for making changes is then: hg up <revision number>, edit files, hg commit --amend, and hg evolve. When you're done, hg up to the top of the stack and moz-phab submit.

This is a pretty complicated workflow for your first time using phabricator. Normally we'd use a single revision for a small set of patches like this. Or, if they were more independent, then we'd open multiple bugs and attach one revision to each bug.

Feel free to squash these into a single revision and abandon the existing ones.

Comment 10

[наб]

Attachment: cmd/digest: actually accept all digest methods

Per https://phabricator.services.mozilla.com/D174327#5865013

Comment 11

[John Schanck [:jschanck]]

These patches are missing context information, which makes them hard to review in phabricator. Did you not use moz-phab submit?

I'm also not sure that our tools will be able to land these automatically. The only way that I was able to apply your patches was to run:

moz-phab patch D177925 --no-commit --apply-to here

which only updates the work tree.

Could you please re-submit these as a single patch?

Check out a clean copy of NSS, or update to the tip of the default branch, and then run:

moz-phab patch D177925 --no-commit --apply-to here
hg commit -m 'Bug 1830978 - p7sign: add -a hash and -u certusage. r=jschanck'
moz-phab submit

Feel free to replace "p7sign: add -a hash and -u certusage" with any commit message you like.

Comment 12

[наб]

Attachment: Bug 1830978 - p7sign: add -a hash and -u certusage (also p7verify cleanups). r=jschanck

Comment 13

[наб]

Yeah, I'd just pasted the patches in; rebase was almost clean, and ended up consisting of just deleting the expanded HashTypeToOID in digest.c again), moz-phab submit yielded https://phabricator.services.mozilla.com/D180584

Comment 14

[наб]

Just had to rebuild libnss3-utils with D180584 (applies cleanly to 3.95) to regain kernel module signing, so bumping this.

Comment 15

[John Schanck [:jschanck]]

https://hg.mozilla.org/projects/nss/rev/425660da5f297d7583783eb27f877865289efc29