[PATCH v5 2/2] binman: x509_cert: support PKCS#11 URI in keyfile for HSM signing

[Sergio Prado]

Hi Quentin,

Em qui., 18 de jun. de 2026 às 14:42, Quentin Schulz <
quentin.schulz at cherry.de> escreveu:
>
> I think this could be split in multiple commits. I could identify the
> following unrelated changes:
[...]
> Do you agree with this split?

Yes, that's a fair split, will do in v6. And you're right on 1): the plain
PKCS#11 URI case already works today without any code change, since binman
passes keyfile through to 'openssl -key' unchanged and the provider
configured via OPENSSL_CONF resolves it (with the PIN coming from
openssl.cnf or a pin-value attribute in the URI). So the first patch of the
split will be docs plus a test exercising exactly that.

> Should we rather move this logic to ReadNode() instead then? Hopefully
> that is run only once? What made you chose GetCertificate() for this
> logic? Maybe I'm missing some clue.

I wanted to avoid mutating entry state given the double GetCertificate()
call, hence the local variable. But ReadNode() runs once, so moving the
logic there is simpler; will do in v6. I'll also make the helper skip
appending when the URI already carries a pin-value attribute, so it becomes
idempotent either way.

> This isn't specific to HSMI/PKCS11 though, is it? So it's just a way to
> override the keyfile entry argument by an environment variable. Yes, the
> typical usecase is to use it for an HSM but nothing forces you to use it
> for an HSM?

Correct, it's a generic override. I'll reword the documentation and rename
the variable to BINMAN_X509_KEYFILE in v6 so the name matches the entry
argument it overrides (it can carry a plain key file path just as well as a
URI). Shout if you prefer the old name or a different one.

> Aren't we missing "local PEM key file" here?

Indeed, will restructure the list as the accepted forms of 'keyfile': local
key file (default), PKCS#11 URI via provider, engine-prefixed URI.

> Is it requiring OpenSSL 3.x though or did you only test on OpenSSL 3.x?
> Since engines were supported in OpenSSL 1.x, I would assume PKCS11
> signing also worked back then? Are you aware of a limitation? Also, does
> it NOT work with OpenSSL 4.x (which distro will slowly start to migrate
to)?

It is a hard requirement, and it follows from the design we settled on in
the v3 review: binman never passes -engine/-keyform itself (the v3
auto-detection
machinery that could have done so was dropped on your suggestion), so the
key must be resolvable through a bare 'openssl -key <uri>'. That only works
with the provider API or the org.openssl.engine: STORE scheme, both
introduced in OpenSSL 3.0. On 1.x there is no transparent path, hence
"OpenSSL 1.x is not supported."

As for OpenSSL 4.x: the ENGINE API was removed entirely in OpenSSL 4.0 [1],
so the engine-prefixed form is OpenSSL 3.x only; the provider path is the
way forward and expected to work unchanged on 4.x. I'll reword the docs to
state all of this explicitly.

[1] https://openssl-library.org/post/2025-12-18-remove-engines/
> Yeah, or just say "OpenSSL 1.x is not supported." and be done with it :)

Will do :)

> question: why suddenly the variable before `make`? All the others are
> after so it seems like an odd choice to me.

It was deliberate: PKCS11_PIN is an environment variable read by binman itself
(via os.environ), not a make variable interpreted by the build system. In
practice `make PKCS11_PIN=1234` also works since make exports command-line
variables to recipes. I'll add a sentence making that explicit.

> The third option is to have the pin in the URI directly (which is
> equally bad, but is possible no?).

Yes, that works today (it's the "no code change" case above); I'll document
it.

> I'm wondering if we should support replacing the PIN inside the URI if
> there's one in it and we also have PKCS11_PIN.

I'd rather define PKCS11_PIN strictly as a fallback: if the URI
already contains
a pin-value attribute, the URI wins and PKCS11_PIN is ignored (right now
the code would blindly append a second pin-value, which RFC 7512 doesn't
allow). Will implement, document and unit-test that precedence in v6.
Silently replacing would be surprising, IMO.

> Side question, what happens if we have a pin set in openssl.cnf and also
> via PKCS11_PIN/pin in the URI?

I'll test this with softhsm2 and document the observed precedence in v6. I
expect the URI pin-value to take precedence over pkcs11-module-token-pin,
but I want to confirm before writing it down.

> Idly wondering if we shouldn't do a simple try:except and let
> raise_on_error to True and skip in the except.

I have a mild preference for the explicit return-code check, since a missing
provider is an expected outcome rather than an error. But I don't feel
strongly, happy to change it if you do.

> Please add a test for when the pin is passed via the URI directly (as I
> assume this is supported) and not through the env variable.

Will do.

> We should add a keyfile property in the test device tree to make sure we
> override it with the entryarg.

Will do. I confirmed entry arguments take precedence over the device tree
property (Entry.GetArg() checks entry args first).

> It'd be nice to check the engine works as well with the prefix. [...]

Simon suggested the same compromise; I'll add a unit test that the PIN combiner
leaves the org.openssl.engine:pkcs11: prefix intact, plus a check that the
prefixed URI reaches the openssl invocation unchanged.

I am going on vacation and will be back in a few weeks, so I'll address all
of the above and send v6 once I return. In the meantime, if you have any
additional comments before I prepare v6, please let me know.

Thanks a lot for the thorough review!

Sergio Prado