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

feat: Add support for AWS-LC build in ESDK #750

Merged
merged 13 commits into from
Sep 6, 2022

Conversation

samuel40791765
Copy link
Contributor

@samuel40791765 samuel40791765 commented Jun 6, 2022

Issue #, if available:
Resolves CryptoAlg-1133

Description of changes:
After meeting up and discussing with the ESDK team, we're ready to continue pushing forward with this PR. The PR has been verified to still work with ESDK as of right now, and we will continue pushing forward with the changes within it.

  1. Two additional header file references in source/cipher_openssl.c
    source/cipher_openssl.c uses ASN1 and OBJ specific functions (BN_to_ASN1_INTEGER, d2i_ASN1_INTEGER, OBJ_txt2nid etc.).
    These functions are defined in AWS-LC’s asn1.h & obj.h header file. Adding the two header file references successfully resolves all of the missing symbols. ESDK had gotten around asn1.h because the existing OpenSSL header ec.h in the code file was indirectly calling upon asn1.h. obj.h is AWS-LC specific.

  2. OpenSSL_free
    CESDK calls i2o_ECPublicKey which internally allocates memory using OPENSS_malloc. This behavior applies to both implementation of i2o_ECPublicKey in OpenSSL and AWS-LC. The expected way to deal with memory allocated with OPENSSL_malloc is to free it with OPENSSL_free. However, when the OPENSSL_free change is applied to OpenSSL builds for ESDK, the CBMC proof for aws_cryptosdk_sig_get_pubkey will fail. This might be due to the CMBC proof overrides not recognizing the OpenSSL_free function.
    The only change that needs to be done here is to change free to OpenSSL_free, to deal with memory that has been passed into i2o_ECPublicKey.

  3. HKDF
    AWS-LC does not support HKDF construction with the EVP_PKEY* methods in OpenSSL 1.1.1.
    We're backporting to ESDK's HKDF self implementation methods aws_cryptosdk_hkdf_extract
    and aws_cryptosdk_hkdf_expand when building with AWS-LC for now.
    AWS-LC does have support for HKDF (HKDF_extract & HKDF_expand) and the semantics of them
    are nearly identical to ESDK's self implementation. We can look to add support if there are customer
    requirements to use AWS-LC's HKDF crypto implementation when building with ESDK.

  4. AWS-LC CI support in ESDK
    A docker file with test scripts to support AWS-LC has been added. The scripts aim to work similarly to ESDK's CI docker file build process (codebuild/ubuntu-latest-x64.Dockerfile). The CI should be able to use the same buildbatch and build specs defined under codebuild/ubuntu-latest-x64.

The contents of this PR were mostly taken and adjusted from #716.

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

Check any applicable:

  • Were any files moved? Moving files changes their URL, which breaks all hyperlinks to the files.

@samuel40791765 samuel40791765 requested a review from a team as a code owner June 6, 2022 18:49
@alex-chew alex-chew self-requested a review June 6, 2022 21:37
source/hkdf.c Outdated Show resolved Hide resolved
codebuild/bin/install-shared-deps-awslc.sh Outdated Show resolved Hide resolved
codebuild/bin/install-shared-deps-awslc.sh Outdated Show resolved Hide resolved

function download_awslc() {
AWSLC_GIT_URL='https://github.com/awslabs/aws-lc.git'
AWSLC_GIT_REF='main'
Copy link
Contributor

Choose a reason for hiding this comment

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

We should pin a particular AWS-LC version as we do for the OpenSSL Dockerfiles. I tried changing this to the latest relevant tag I saw (AWS-LC-FIPS-1.0.2), but the Docker image failed to build because (I assume) that tag is a bit out of date. Can you tag the working commit on main and pin that tag here? Then I can build the image and set up a CI build that runs within it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry for the confusion, the AWS-LC-FIPS-1.0.2 branch points to a FIPS version release, which points to a branch of an older commit of the repo that's still in the process of getting FIPS validated.
I've set the docker image tag to v1.1.0, which points to a newer commit should work. Just so you know, we'll also be setting up an encryption-sdk CI in AWS-LC, so we can verify that all future versions of AWS-LC will build with ESDK :)

@alex-chew alex-chew changed the title Add support for AWS-LC build in ESDK feat: Add support for AWS-LC build in ESDK Jul 12, 2022
@ajewellamz ajewellamz temporarily deployed to continuous-integration August 30, 2022 21:41 Inactive
@ajewellamz ajewellamz temporarily deployed to continuous-integration August 30, 2022 21:41 Inactive
ajewellamz
ajewellamz previously approved these changes Aug 30, 2022
@alex-chew alex-chew temporarily deployed to continuous-integration August 30, 2022 22:37 Inactive
@alex-chew alex-chew temporarily deployed to continuous-integration August 30, 2022 22:37 Inactive
@alex-chew alex-chew temporarily deployed to continuous-integration August 31, 2022 16:27 Inactive
@alex-chew alex-chew temporarily deployed to continuous-integration August 31, 2022 16:27 Inactive
@samuel40791765 samuel40791765 temporarily deployed to continuous-integration September 2, 2022 18:46 Inactive
@samuel40791765 samuel40791765 temporarily deployed to continuous-integration September 2, 2022 18:46 Inactive
@samuel40791765 samuel40791765 temporarily deployed to continuous-integration September 2, 2022 18:52 Inactive
@samuel40791765 samuel40791765 temporarily deployed to continuous-integration September 2, 2022 18:52 Inactive
@samuel40791765 samuel40791765 temporarily deployed to continuous-integration September 2, 2022 18:57 Inactive
@samuel40791765 samuel40791765 temporarily deployed to continuous-integration September 2, 2022 18:58 Inactive
@samuel40791765 samuel40791765 force-pushed the aws-lc-support branch 3 times, most recently from 16ae1ae to a97252f Compare September 2, 2022 19:19
@samuel40791765 samuel40791765 temporarily deployed to continuous-integration September 2, 2022 19:26 Inactive
@samuel40791765 samuel40791765 temporarily deployed to continuous-integration September 2, 2022 19:26 Inactive
@samuel40791765 samuel40791765 temporarily deployed to continuous-integration September 2, 2022 20:49 Inactive
@samuel40791765 samuel40791765 temporarily deployed to continuous-integration September 2, 2022 20:49 Inactive
@samuel40791765 samuel40791765 force-pushed the aws-lc-support branch 2 times, most recently from 53c089a to a97252f Compare September 2, 2022 21:03
@samuel40791765 samuel40791765 temporarily deployed to continuous-integration September 2, 2022 22:40 Inactive
@samuel40791765 samuel40791765 temporarily deployed to continuous-integration September 2, 2022 22:41 Inactive
@ajewellamz ajewellamz merged commit c40940e into aws:master Sep 6, 2022
@ajewellamz ajewellamz deleted the aws-lc-support branch September 6, 2022 15:00
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.

3 participants