Closed Villemoes closed 1 month ago
@tdewey-rpi How does this fit-in with the PKCS#11 options in sb-provisioner ?
Very similar to the approach taken by rpi-sb-provisioner - with one key difference: rather than relying on a grep
to infer if the user wants to sign with PKCS#11, we instead expose a separate variable that takes priority and is explicitly a PKCS#11 object: https://github.com/raspberrypi/rpi-sb-provisioner/blob/170825abddbacbc56cf858096b4e5ae316d4e41a/service/rpi-sb-provisioner.sh#L50
Well, that's also doable, but I really prefer if the private key is as much of an "opaque token" as possible, for two reasons: First, in our Yocto setup, I've made it very easy to use some development key, which is indeed just a .pem file, by setting RPI_SIGNING_KEY
in local.conf
; that means random developers don't always need to have access to the HSM or signing service (of course, the artifacts then only work on development boards provisioned with the dev key). Of course, I could modify the bitbake logic which calls into these rpi helper scripts and lift the "does it start with pkcs11:" logic there, but I think it's neater if the tools just DTRT.
Second, when the key is passed down through several layers of scripts, which I just now realized I'll need to do (the update-pieeprom.sh
script in the "parent" repo will need to be taught about this as well), it's much easier if such a script just accepts a -k <key>
and passes that value on, instead of teaching every layer to accept a "pkcs11 uri xor .pem file" and teaching it how to invoke the next lower-layer script in the appropriate way.
I see that the sb-provisioner code first checks for a pkcs11 "thingy" before checking for a local .pem file; whereas this PR seems to be checking for a local file first, before checking for pkcs11 ?
I see that the sb-provisioner code first checks for a pkcs11 "thingy" before checking for a local .pem file; whereas this PR seems to be checking for a local file first, before checking for pkcs11 ?
This is more an artefact of the fundamental design choice - strongly explicit vs conveniently implicit. This PR would expect one value for KEY
that is pem||pkcs#11string
, while sb-provisioner makes you explicitly set that you want to use PKCS#11.
I don't believe one approach is superior to the other - they simply represent different schools of thought on a specific problem.
@Villemoes I understand and accept your design pressures, however I would strongly caution you to make it extremely clear to the development user which credential they're using to sign their payloads.
We have an additional design pressure in that our primary target is working with Raspberry Pi OS (and derivatives), and there is a healthy curiosity in the community around this process, and driving these scripts directly. As a result, I prefer to design in favour of every script being defensive unless it is explicitly an internal-only component of a larger package (which must be defensive). Keeping the checks in these scripts, therefore, I support.
I'm prepared to merge this but I'm not going to rule out requiring explicit arguments for PKCS#11 in the future
Well, that's also doable, but I really prefer if the private key is as much of an "opaque token" as possible, for two reasons: First, in our Yocto setup, I've made it very easy to use some development key, which is indeed just a .pem file, by setting RPI_SIGNING_KEY
in local.conf
; that means random developers don't always need to have access to the HSM or signing service (of course, the artifacts then only work on development boards provisioned with the dev key). Of course, I could modify the bitbake logic which calls into these rpi helper scripts and lift the "does it start with pkcs11:" logic there, but I think it's neater if the tools just DTRT.
Second, when the key is passed down through several layers of scripts, which I just now realized I'll need to do (the update-pieeprom.sh
script in the "parent" repo will need to be taught about this as well), it's much easier if such a script just accepts a -k <key>
and passes that value on, instead of teaching every layer to accept a "pkcs11 uri xor .pem file" and teaching it how to invoke the next lower-layer script in the appropriate way.
I see that the sb-provisioner code first checks for a pkcs11 "thingy" before checking for a local .pem file; whereas this PR seems to be checking for a local file first, before checking for pkcs11 ?
This is more an artefact of the fundamental design choice - strongly explicit vs conveniently implicit. This PR would expect one value for
KEY
that ispem||pkcs#11string
, while sb-provisioner makes you explicitly set that you want to use PKCS#11.
Yes. As to the "order" I'm checking in, I don't believe it would ever matter in practice, nobody creates a file containing a private key starting with the string "pkcs11:". It could be interchanged to first check for that particular prefix, and if somebody does have a file named like that, it could be passed via the same trick that allows you to delete a file called -f
, namely ./pkcs11:...
. But it really doesn't matter.
I don't believe one approach is superior to the other - they simply represent different schools of thought on a specific problem.
@Villemoes I understand and accept your design pressures, however I would strongly caution you to make it extremely clear to the development user which credential they're using to sign their payloads.
Not a problem at all; in our case, signing using the signing service requires a release manager to upfront authorize the use of the production keys (and for a given, finite, number of times, and within a time window).
We have an additional design pressure in that our primary target is working with Raspberry Pi OS (and derivatives), and there is a healthy curiosity in the community around this process, and driving these scripts directly. As a result, I prefer to design in favour of every script being defensive unless it is explicitly an internal-only component of a larger package (which must be defensive). Keeping the checks in these scripts, therefore, I support.
I must admit I don't quite understand what you're saying or asking me to do. What does "defensive" mean here? I still check for the existence of a file, if it is assumed to be a file. And in case the HSM/pkcs11 backend somehow rejects the signing attempt, certainly the openssl call fails and thus the whole script.
I must admit I don't quite understand what you're saying or asking me to do. What does "defensive" mean here? I still check for the existence of a file, if it is assumed to be a file. And in case the HSM/pkcs11 backend somehow rejects the signing attempt, certainly the openssl call fails and thus the whole script.
Defensive in terms of user expectations - where having an implicit or hidden mechanism can be hard to discover, and hard to reason about without knowing where to find the mechanism.
I'm not asking for anything additional.
I'm prepared to merge this but I'm not going to rule out requiring explicit arguments for PKCS#11 in the future
Thanks. But in the meantime I've realized how much more work there is to properly support my use case. It's not just this script, but also the update-pieeprom.sh script and the rpi-sign-bootcode python script.
The latter is more or less impossible to rework directly, as it doesn't use libssl at all but does the rsa crypto in pure python, so I've reverse engineered it and rewritten in shell calling openssl, adding the option of passing the public key separately. I don't know if you'd want to take that reimplementation (it doesn't do the -c 2711
case, as nothing seems to call it with that option).
update-pieeprom.sh I haven't gotten to yet, but it seems simple enough once the other tools have been updated, but that includes rpi-sign-bootcode.
I'm happy if this gets merged, and I will anyway rewrite or update the other scripts at least privately, and I can post the results somewhere for people to look at.
Merging - we use a slightly different approach for PKCS#11 internally where entire logic including names of keys is encapsulated by OPENSSL command. This removes the option for the creator of the build to pass alternate/wrong parameters since they can't drive the HSM directly
In production setups, it is quite normal that the private key does not exist as a file in the file system, but is kept inside some HSM, remote signing service or similar, and only accessed via some pkcs#11 interface; moreover, by design, the private key cannot be extracted from the HSM or signing service.
In such a case, the user will have set OPENSSL_CONF to some configuration file setting up the appropriate engine, and the "key" is simply the pkcs#11 URI, e.g. "pkcs11:model=foo;object=bar".
In order to support this use case, automatically infer the appropriate options to pass to openssl-dgst if "${KEY}" begins with "pkcs11:". Doing this at the top level avoids duplicating the logic in both writeSig and verifySig. While here, this also adds a sanity check that -v can only be used while also providing a (public) key to check against.
This drops the -keyform argument in the non-pkcs#11 case, as openssl automatically infers the type, and this then in fact allows one to use a private key in e.g. DER format.