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

Persistent private keys for agreement #302

Merged
merged 10 commits into from
Jan 18, 2024
Merged

Conversation

justsmth
Copy link
Contributor

@justsmth justsmth commented Dec 20, 2023

Issues:

#300

Description of changes:

  • Allow PrivateKeys to persist across multiple agreements.
  • Support seralizing/deserializing agreement::PrivateKey

Call-outs:

  • Mostly a rename of EphemeralPrivateKey to PrivateKey. I made EphemeralPrivateKey a wrapper around PrivateKey and moved it to another file.

Testing:

  • Augmented existing tests for coverage.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license and the ISC license.

@hansonchar
Copy link
Contributor

Nice! Thanks for this PR.

@justsmth justsmth force-pushed the persistent-agreement branch 2 times, most recently from 50c4f85 to 5b52b3c Compare December 21, 2023 23:29
@codecov-commenter
Copy link

codecov-commenter commented Dec 21, 2023

Codecov Report

Attention: 26 lines in your changes are missing coverage. Please review.

Comparison is base (60def47) 95.73% compared to head (7c68274) 95.86%.

Files Patch % Lines
aws-lc-rs/src/agreement/ephemeral.rs 95.16% 16 Missing ⚠️
aws-lc-rs/src/agreement.rs 97.65% 6 Missing ⚠️
aws-lc-rs/src/ec.rs 98.07% 2 Missing ⚠️
aws-lc-rs/src/ed25519.rs 95.34% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #302      +/-   ##
==========================================
+ Coverage   95.73%   95.86%   +0.13%     
==========================================
  Files          59       60       +1     
  Lines        8091     8494     +403     
==========================================
+ Hits         7746     8143     +397     
- Misses        345      351       +6     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

aws-lc-rs/src/encoding.rs Outdated Show resolved Hide resolved
aws-lc-rs/src/agreement.rs Outdated Show resolved Hide resolved
aws-lc-rs/src/agreement/ephemeral.rs Outdated Show resolved Hide resolved
aws-lc-rs/src/agreement/ephemeral.rs Outdated Show resolved Hide resolved
skmcgrail
skmcgrail previously approved these changes Jan 2, 2024
aws-lc-rs/src/encoding.rs Outdated Show resolved Hide resolved
aws-lc-rs/src/agreement.rs Show resolved Hide resolved
aws-lc-rs/src/ec.rs Show resolved Hide resolved
aws-lc-rs/src/ec/key_pair.rs Show resolved Hide resolved
aws-lc-rs/src/agreement.rs Show resolved Hide resolved
skmcgrail
skmcgrail previously approved these changes Jan 11, 2024
aws-lc-rs/src/agreement.rs Show resolved Hide resolved
aws-lc-rs/src/agreement.rs Show resolved Hide resolved
aws-lc-rs/src/agreement.rs Show resolved Hide resolved
aws-lc-rs/src/agreement.rs Show resolved Hide resolved
aws-lc-rs/src/agreement.rs Show resolved Hide resolved
aws-lc-rs/src/agreement/ephemeral.rs Show resolved Hide resolved
aws-lc-rs/src/agreement/ephemeral.rs Show resolved Hide resolved
aws-lc-rs/src/agreement/ephemeral.rs Show resolved Hide resolved
skmcgrail
skmcgrail previously approved these changes Jan 17, 2024
@@ -307,13 +307,11 @@ impl PrivateKey {
)
})?
} else {
let ec_group = unsafe { ec_group_from_nid(alg.id.nid())? };
let private_bn = DetachableLcPtr::try_from(key_bytes)?;
let ec_group: LcPtr<EC_GROUP> = ec_group_from_nid(alg.id.nid())?;
Copy link
Member

Choose a reason for hiding this comment

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

Minor but ec_group_from_nid doesn't technically need this since you aren't trying to convert to the type like the line right below it. Not really picky, just wasn't sure if you were doing it for clarity or had expected different.

@justsmth justsmth merged commit 3448b87 into aws:main Jan 18, 2024
102 of 108 checks passed
@justsmth justsmth deleted the persistent-agreement branch January 18, 2024 19:40
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.

5 participants