Skip to content

Fix/replace openssl shellout with aws lc rs#923

Merged
jmt-lab merged 3 commits into
awslabs:developfrom
jeremydosborn:fix/replace-openssl-shellout-with-aws-lc-rs
Mar 23, 2026
Merged

Fix/replace openssl shellout with aws lc rs#923
jmt-lab merged 3 commits into
awslabs:developfrom
jeremydosborn:fix/replace-openssl-shellout-with-aws-lc-rs

Conversation

@jeremydosborn

@jeremydosborn jeremydosborn commented Mar 13, 2026

Copy link
Copy Markdown
Contributor

Issue: #627

gen-rsa-key was shelling out to the system openssl binary to generate RSA keys. When #825 migrated the crypto backend from ring to aws-lc-rs, this shell-out was not addressed — RSA key generation via aws-lc-rs may not have been available at the time.

This PR replaces the std::process::Command openssl shell-out with aws-lc-rs, which is already a dependency.

Changes:

  • gen_rsa_key now uses aws_lc_rs::rsa::PrivateDecryptingKey for key generation
  • --bits restricted to 2048, 3072, 4096 — other values return a clear error
  • --exp retained for CLI compatibility but ignored if non-65537, with a warning
  • Removes dead error variants CommandExec, CommandStatus, CommandUtf8
  • Removes OpenSSL installation requirement from README
  • Adds base64 = "0.22" as a new dependency for DER-to-PEM encoding
  • Adds two new tests: gen_rsa_key_and_sign_root and gen_rsa_key_unsupported_size_fails

Testing: cargo test passes (full workspace).

Note: the snakeoil test fixtures in tests/data/ could be regenerated using the new code path to provide additional coverage, if the maintainers wish to do so in a follow-up.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@webern

webern commented Mar 13, 2026

Copy link
Copy Markdown
Contributor

Yay 🎉

@jmt-lab jmt-lab left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@bcressey

Copy link
Copy Markdown
Contributor

Code changes look fine to me. Can you run cargo fmt? CI is failing for that reason.

@jeremydosborn

Copy link
Copy Markdown
Contributor Author

Done

@yeazelm yeazelm left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the contribution @jeremydosborn !

@jeremydosborn

Copy link
Copy Markdown
Contributor Author

Of course! Thanks everyone.

@sam-berning sam-berning left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@jeremydosborn jeremydosborn mentioned this pull request Mar 22, 2026
@jmt-lab jmt-lab merged commit 3dbc927 into awslabs:develop Mar 23, 2026
9 checks passed
@jeremydosborn jeremydosborn deleted the fix/replace-openssl-shellout-with-aws-lc-rs branch April 13, 2026 14:20
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.

6 participants