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

openssl: interop with AWS-LC #10320

Closed
wants to merge 1 commit into from
Closed

Conversation

jeking3
Copy link
Contributor

@jeking3 jeking3 commented Jan 19, 2023

This pull request adds support for building against AWS-LC.
AWS-LC is a BoringSSL/OpenSSL derivative. For more information see https://github.com/awslabs/aws-lc/
This is my first PR into curl - please let me know if I missed anything or changed something I was not supposed to.

Changes

  • Configure changes to detect AWS-LC
  • CMakeLists.txt changes to detect AWS-LC (very minor)
  • Compile-time branches needed to support AWS-LC
  • Correctly set OSSL_VERSION and report AWS-LC release number
  • GitHub Actions script to build with autoconf and cmake against AWS-LC

Testing

@jeking3
Copy link
Contributor Author

jeking3 commented Jan 19, 2023

I did a force push to fix a LONGLINE formatting error coming from checksrc.pl during the build in CI.

@jeking3
Copy link
Contributor Author

jeking3 commented Jan 19, 2023

The 6 macOS build failures appear to be pre-existing to this PR and should be resolved by #10288.
The curl/check curl-novalgrind-ngtcp2-with-openssl failure appears to be pre-existing to this PR.

@jay jay added TLS feature-window A merge of this requires an open feature window labels Jan 19, 2023
@jay
Copy link
Member

jay commented Jan 19, 2023

This looks fine to me but we are in a feature freeze and not adding new symbols, so this will have to wait until the next feature window opens.

@jeking3
Copy link
Contributor Author

jeking3 commented Jan 19, 2023

What does this mean in terms of timing before it is able to get into a release? The next one I see on the calendar you provided is in March. I was hoping to be able to get this into 7.88.0 to simplify some near-term plans. Given it was submitted on the day of the freeze and is very low risk, would you consider an exception?

@cmeister2
Copy link
Contributor

would you consider an exception?

Given as Daniel is on vacation, I doubt anyone is going to grant an exception to the feature window in his absence.

@jeking3
Copy link
Contributor Author

jeking3 commented Jan 19, 2023

It looks like I didn't quite get all of the compile-time branches, like OSSL_PACKAGE definition and version printout, and a curl-openssl.m4 section for detecting AWS-LC.

@jeking3
Copy link
Contributor Author

jeking3 commented Jan 19, 2023

Regarding the feature window, that's fine, we'll let this bake appropriately then, and it will be better when it goes in.

@jeking3 jeking3 force-pushed the awslc-compat branch 2 times, most recently from d79cbb4 to 95d9f49 Compare January 19, 2023 19:49
@jeking3
Copy link
Contributor Author

jeking3 commented Jan 19, 2023

That line length linter sure is pedantic!

@jeking3 jeking3 force-pushed the awslc-compat branch 2 times, most recently from b90c19d to 210b166 Compare January 23, 2023 14:35
@jeking3
Copy link
Contributor Author

jeking3 commented Jan 23, 2023

Rebased to pick up CI improvements.

@jeking3
Copy link
Contributor Author

jeking3 commented Feb 3, 2023

Rebased to resolve conflicts in READMEs. I see we're down to two failing CI jobs (unrelated to these changes), so it looks like some of those were fixed (or removed) as well.

* Configure changes to detect AWS-LC
* CMakeLists.txt changes to detect AWS-LC
* Compile-time branches needed to support AWS-LC
* Correctly set OSSL_VERSION and report AWS-LC release number
* GitHub Actions script to build with autoconf and cmake against AWS-LC

AWS-LC is a BoringSSL/OpenSSL derivative
For more information see https://github.com/awslabs/aws-lc/
@jeking3
Copy link
Contributor Author

jeking3 commented Feb 22, 2023

Rebased on master to resolve conflict and pick up CI fixes.

Copy link
Member

@MarcelRaad MarcelRaad left a comment

Choose a reason for hiding this comment

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

Looks good to me and the CI failures look unrelated.

@@ -1015,6 +1015,7 @@ CURLSSH_AUTH_KEYBOARD 7.16.1
CURLSSH_AUTH_NONE 7.16.1
CURLSSH_AUTH_PASSWORD 7.16.1
CURLSSH_AUTH_PUBLICKEY 7.16.1
CURLSSLBACKEND_AWSLC 7.88.0
Copy link
Member

Choose a reason for hiding this comment

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

This should probably become 8.1.0 as that's the next feature window version.

AWS-LC
AWSLC
aws-lc
awslc
Copy link
Member

Choose a reason for hiding this comment

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

Do we really need to spell it this many different ways? Is there not an official way we could stick to?

@bagder
Copy link
Member

bagder commented Mar 30, 2023

Thanks!

@bagder bagder closed this in 34ef4fa Mar 30, 2023
bch pushed a commit to bch/curl that referenced this pull request Jul 19, 2023
* Configure changes to detect AWS-LC
* CMakeLists.txt changes to detect AWS-LC
* Compile-time branches needed to support AWS-LC
* Correctly set OSSL_VERSION and report AWS-LC release number
* GitHub Actions script to build with autoconf and cmake against AWS-LC

AWS-LC is a BoringSSL/OpenSSL derivative
For more information see https://github.com/awslabs/aws-lc/

Closes curl#10320
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature-window A merge of this requires an open feature window TLS
Development

Successfully merging this pull request may close these issues.

None yet

5 participants