Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

store keys as pem instead of s-expression #70

Merged
merged 1 commit into from
Mar 1, 2024

Conversation

hannesm
Copy link
Contributor

@hannesm hannesm commented Feb 28, 2024

Dear valued opam-health-check authors and maintainers,

thanks to @reynir mirage/mirage-crypto#208 (comment) I got aware that your application uses the "convert a RSA key to s-expression" from mirage-crypto-pk. Now, this is a feature we'd like to remove from the next version.

What this PR provides is a possible migration. I'm not sure how opam-health-check is deployed and how the key material is generated / copied / provided.

What this PR does:

  • on the key generation and writing of key files, it uses PEM (PKCS8) instead of s-expressions
  • on the key reading (decoding) side, it first tries to decode as s-expression and if that fails as PEM

Now, depending on the deployment and desired use, of course there can as well be a utility that goes through the key directory (on the server) and convert all from s-expression to pem. But are keys actually copied around to client machines and used there? If yes, these of course should as well be updated / converted.

Please let me know what you think about a migration. I start this PR and discussion early that you have the least trouble once the next mirage-crypto release is done.

Thanks for reading.

@mtelvers
Copy link
Member

Thank you for opening this PR. The distribution of these keys is very limited; we could regenerate new keys in PEM format or do a one-time conversion of the keys from s-expression to PEM. Opam-health-check can be updated to expect PEM format keys only.

let () =
  let key = Mirage_crypto_pk.Rsa.priv_of_sexp (Sexplib.Sexp.load_sexp Sys.argv.(1)) in
  let key_pem = X509.Private_key.encode_pem (`RSA key) in
  let pem = Cstruct.to_string key_pem in
  let oc = open_out Sys.argv.(1) in
  Printf.fprintf oc "%s" pem;
  close_out oc;

@hannesm
Copy link
Contributor Author

hannesm commented Feb 28, 2024

Thanks @mtelvers, indeed your migration code should mostly work -- I'd open a second file for writing and then move the second to the first to avoid truncation / power failure inconsistency / thinking about failure mode when the new stuff to be written is smaller than the old stuff that was in the file...

@mtelvers mtelvers merged commit 52d209d into ocurrent:master Mar 1, 2024
1 of 2 checks passed
@hannesm hannesm deleted the ohc-key-store-pem branch March 3, 2024 22:16
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