Skip to content

Conversation

@RitvikKapila
Copy link
Contributor

Issue #, if available:

Description of changes:

  • Update smithy blob changes after bumping aws-smithy-types to 1.2.8
  • Add Raw ECDH and KMS ECDH examples

Squash/merge commit message, if applicable:

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

@RitvikKapila RitvikKapila marked this pull request as ready for review November 26, 2024 02:05
@RitvikKapila RitvikKapila requested a review from a team as a code owner November 26, 2024 02:05

// Following are the helper functions for running ECDH examples

pub(crate) fn x962_to_x509(
Copy link
Contributor

Choose a reason for hiding this comment

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

i think this exists in the mpl

Copy link
Contributor Author

@RitvikKapila RitvikKapila Nov 27, 2024

Choose a reason for hiding this comment

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

Yes but that is an extern and it takes in an internal dafny type ECDHCurveSpec which I don't want to expose to customers. So this is a function which closely mimics the function in the externs.

Copy link
Contributor

Choose a reason for hiding this comment

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

I would much rather us use the extern code because if the extern were to change then we would have to come and update this as well

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Once aws-lc-rs supports the various formats we need, we want to drop the dependence on aws-lc-sys.

If we keep the function public, we will have to support it forever. This will make the above update a blocking change. For Rust ECDH examples, I will create a function that mimics X962_to_X509 so that we can update it with the aws-lc-rs conversions once they are supported.

@RitvikKapila RitvikKapila marked this pull request as draft December 3, 2024 01:06
@RitvikKapila RitvikKapila marked this pull request as ready for review December 3, 2024 18:59
Copy link
Contributor

@josecorella josecorella left a comment

Choose a reason for hiding this comment

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

just have a question about the submodule bump, other than that lgtm

mpl
Copy link
Contributor

Choose a reason for hiding this comment

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

are we also supposed to bump the submodule?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, it is already at MPL HEAD

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for taking a look at this

@RitvikKapila RitvikKapila merged commit 2e76dd7 into rkapila/rust-reviewed Dec 4, 2024
61 checks passed
@RitvikKapila RitvikKapila deleted the rkapila/rust-ecdh-examples branch December 4, 2024 06: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