Skip to content

Conversation

plotnick
Copy link
Contributor

We previously used the x509-parser crate to parse X.509 certificates; this migrates to the RustCrypto x509-cert crate instead. The motivation for this change is that Permission Slip uses the latter crate (since it needs full ser/de for certificates), and now that it supports owned types, we can pass around e.g., deserialized objects of type Certificate instead of DER serialized Vec<u8>. A forthcoming PR will migrate Permission Slip to this new interface, and we'll need a similar one for hubtools. Draft status on this PR can be removed once those are up and ready to merge along with this one, but reviews would be welcome in the meantime.

The RustCrypto x509-cert crate does lack a few convenience methods that x509-parser had (e.g., public_key, verify_signature), but they were easily replaced with trivial functions. The one behavioral difference I found was that x509-cert returns an error if the buffer containing a DER message is larger than the message itself, but x509-parser does not (it just ignores the excess bytes); this matters to lpc55_sign verify-signed-image because the certificates in the image table are padded to 4-byte boundaries. The work-around is to peek at the DER header, compute the actual message length, and pass a slice with just the message bytes.

As a user convenience, lpc55_sign also now auto-detects PEM encoded certificates and decodes them transparently. In particular, this means you don't have to DER encode the output certificates from the OKS ceremony.

@plotnick plotnick requested review from mkeeter and mx-shift April 19, 2023 18:38
@mkeeter
Copy link
Contributor

mkeeter commented Apr 19, 2023

This looks reasonable, although I'd also like to have draft PRs staged for hubtools / Hubris / Humility – just to make sure there are no surprises in how we integrated this crate.

I can help with that if you're busy; let me know if that would be useful!

@plotnick
Copy link
Contributor Author

... I'd also like to have draft PRs staged for hubtools / Hubris / Humility ... I can help with that if you're busy; let me know if that would be useful!

It would, thank you for offering! In particular, I'm not set up to do either Hubris or Humility development right now, so if you could help with those, that'd be great. I'll take hubtools, as I'd like to get back to integrating that into permslip.

@plotnick plotnick marked this pull request as ready for review April 20, 2023 21:42
@mkeeter
Copy link
Contributor

mkeeter commented Apr 21, 2023

@plotnick I think this is good to go. The Hubris PR (oxidecomputer/hubris#1302) seems fine (thanks to @kc8apf for some fixes), and we're trying to remove the Humility dependency altogether (oxidecomputer/humility#366).

@plotnick plotnick merged commit a5cd40d into master Apr 21, 2023
@plotnick plotnick deleted the rust-crypto branch April 21, 2023 15:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants