Hi Simon,
> > + # Find the OpenSSL pkcs11 provider module via the modules dir
> > + # reported by 'openssl version -m'. This handles multiarch
paths
> > + # like /usr/lib/x86_64-linux-gnu/ossl-modules on Debian/Ubuntu,
> > + # which OPENSSLDIR does not point at.
> > + m = re.search(r'MODULESDIR: '([^']+)"',
> > + tools.run('openssl', 'version', '-m'))
>
> You construct the bintool instances above (openssl, softhsm2_util,
> pkcs11_tool, p11_kit) but then call tools.run() directly. Commit
> 08bcf962c5f routed the existing eight call sites through
> <bintool>.run_cmd(), at Quentin's suggestion. Can you do the same
> here? - p11_kit.run_cmd('print-config'), openssl.run_cmd('version',
> '-m'), and the softhsm2_util.run_cmd()/pkcs11_tool.run_cmd() calls
> below.
Right. Will do this for the calls that remain.
> > + Uses '&' as separator if the URI already contains '?', else
'?'.
> > + Returns the URI unchanged when pin is None or empty.
> > + """
> > + if not pin:
> > + return uri
> > + sep = '&' if '?' in uri else '?'
> > + return f'{uri}{sep}pin-value={pin}'
>
> The PIN is concatenated raw, but RFC 7512 says the query component is
> percent-encoded.Wouldn't it be better to use urllib.parse.quote() on
> the PIN before appending? You could add a simple test for that.
Good catch. Will use urllib.parse.quote() and add a unit test with a PIN
containing reserved characters.
> > + - cert-ca: Common Name (CN) embedded in the certificate. Used
when
> > + generating a generic x509 certificate.
> > + - cert-revision-int: Integer certificate revision number. Used
when
> > + generating a generic x509 certificate. Defaults to 0.
> > + - sw-rev: Software revision number embedded in the certificate
by
> > + the sysfw/rom variants used by the TI K3 secure boot
subclasses.
> > + Defaults to 1.
>
> Ideally this would go into a separate preparatory commit.
Right. Will split in two then.
Note: Quentin separately asked to drop the x509-key-uri entry arg and let
keyfile accept either a path or a URI. v5 will do that, which trims patch
2/2 further. Since that's a non-trivial restructuring of the entry-arg
surface, I'll drop your Reviewed-by from v5 so you can re-evaluate, and
re-add it once you're happy.
Best regards,
Sergio Prado