WIP: Add PKCS12 LocalKeyId attribute to bags (openssl and firefox compatibility) - Githubissues

mirleft / ocaml-x509

X509 (RFC5280) handling in OCaml

BSD 2-Clause "Simplified" License

52 stars 34 forks source link

WIP: Add PKCS12 LocalKeyId attribute to bags (openssl and firefox compatibility) #163

Closed NightBlues closed 1 year ago

NightBlues commented 1 year ago

Hi, @hannesm! I've tried to upgrade my project to latest version of x509 lib and found that pkcs12 doesn't work for me:( In PR https://github.com/mirleft/ocaml-x509/pull/138 I've suggested to add attribute LocalKeyId to SafeBags because openssl adds it automatically and firefox fails to import p12 container without it. As I understand, LocalKeyId is required to map private key with certificate it corresponds to (because there may be lots of keys and certs in container). I would like to add some unittests for it, but x509.PKCS12 module doesn't export api for accessing bags attributes. firefox_fails_to_import localkeyid_in_cert

hannesm commented 1 year ago

as alternative, we could always embed the local_key_id (and compute the fingerprint within the P12.create. What do you think?

hannesm commented 1 year ago

and thanks for bringing this up again -- I don't remember what I did back then and why I removed the local key id stuff from your PR(s).... is the friendlyname something you need as well?

NightBlues commented 1 year ago

we could always embed the local_key_id (and compute the fingerprint within the P12.create I think its good idea because it follows behavior of other non-OCaml tools, but have ability to control content of pkcs12 container (which has a bit complicated structure) through ?attributes is good choice too. I can't remember why I've added friendlyname - may be because it was in some examples in RFCs

hannesm commented 1 year ago

@NightBlues would you be willing to revise this PR to "always embed the local_key_id" (i.e. compute the hash in P12.create)?

Let's do the "control content of pkcs12 container through ?attributes" when the demand arrives (no need for premature code writing).

NightBlues commented 1 year ago

There are a few questions here:

my openssl version (old enough) currently use SHA1, but its not "secure", may be newer versions use other hashing algorithms
there are many certificates given to create function - how to determine which of them belong to the private_key (some heuristic may help to guess: it should be CA:false certificate, but not sure)
as I understand hashing is not the only way - we can even use uuid or something else as local_key_id unique enough to pair private key with certificate

hannesm commented 1 year ago

we can use a uuid, we can as well use sha256. since x509 does not depend on uuidm, I'd be in favour to use sha256.

how to find the certificate? well, in the certificate you've a public_key, and the public key of the private key can as well be extracted, and compared:

let find_cert certs priv =
  let pub = X509.Public_key.fingerprint (X509.Private_key.public priv) in
  List.find_opt (fun cert -> Cstruct.equal (X509.Public_key.fingerprint (X509.Certificate.public_key cert)) pub) certs

EDIT: use fingerprint instead of id. Now this fingerprint can be used as the value of local_key_id. Also, not sure whether it's worth to check there's only a single certificate using this public key.

hannesm commented 1 year ago

I took the liberty to implement this myself in 52820ea -- I also tested this with my firefox successfully (thanks for raising the issue, and your description of how to reproduce).