Build Envoy with aws_lc on Power (ppc64le) #38403

Jenkins-J · 2025-02-11T15:49:24Z

Commit Message: Build Envoy with aws_lc instead of boringssl when building for the ppc64le architecture.
Additional Description: Envoy uses boringssl as the default ssl implementation. As boringssl does not support the ppc64le architecture, a different ssl implementation is necessary to build Envoy for ppc64le.
Risk Level: High
Testing:
Docs Changes:
Release Notes:
Platform Specific Features: Boringssl no longer supports the ppc64le architecture. Here, aws_lc is used as a "drop-in" replacement for boringssl when building Envoy for ppc64le. Aws_lc is conditionally built only when Envoy is built for ppc64le with FIPS support.
[Optional Runtime guard:]
[Optional Fixes #Issue]
[Optional Fixes commit #PR or SHA]
[Optional Deprecated:]
[Optional API Considerations:]

repokitteh-read-only · 2025-02-11T15:49:32Z

CC @envoyproxy/dependency-shepherds: Your approval is needed for changes made to (bazel/.*repos.*\.bzl)|(bazel/dependency_imports\.bzl)|(api/bazel/.*\.bzl)|(.*/requirements\.txt)|(.*\.patch).
envoyproxy/dependency-shepherds assignee is @moderation

🐱

Caused by: #38403 was opened by Jenkins-J.

see: more, trace.

mattklein123 · 2025-02-12T15:13:12Z

This seems OK to me but will defer to @ggreenway

phlax · 2025-02-12T15:38:10Z

needs main merge

ggreenway

I think we need to document somewhere that any builds using aws-lc or on PPC are not covered by the envoy security policy, and that PPC is currently best-effort and not maintained by the Envoy maintainers.

Both of those caveats are necessary because we don't have any CI coverage for this build variant. If that changes at some point in the future that can be re-evaluated.

ggreenway · 2025-02-12T17:02:33Z

source/extensions/transport_sockets/tls/cert_validator/spiffe/spiffe_validator.cc

@@ -116,9 +116,15 @@ absl::Status SPIFFEValidator::addClientValidationContext(SSL_CTX* ctx, bool) {
    X509_NAME* name = X509_get_subject_name(ca.get());

    // Check for duplicates.
+    #ifdef OPENSSL_IS_AWSLC
+    if (sk_X509_NAME_find_awslc(list.get(), nullptr, name)) {


Can you explain why this, and only this, function needs a _awslc suffix? Is there a defined policy in aws-lc for when a different function name is used?

Thinking through this a little more, I think it would be easier to read and reason about if you made a header for api compatibility that you include in needed files, and the entire content is inside #ifdef OPENSSL_IS_AWSLC, and you either #define sk_X509_NAME_find sk_X509_NAME_find_awslc, or define a function named sk_X509_NAME_find in the Envoy namespace so that it's a closer match than the global namespace version, which does the needed API translation. I'd prefer that to sprinkling preprocessor directives around the code.

Can you explain why this, and only this, function needs a _awslc suffix? Is there a defined policy in aws-lc for when a different function name is used?

Yes, the function sk_X509_NAME_find_awslc is consistent with the boringssl implementation while the sk_X509_NAME_find function in aws-lc has a different number of parameters in order to be openssl compatible.

Thinking through this a little more, I think it would be easier to read and reason about if you made a header for api compatibility that you include in needed files, and the entire content is inside #ifdef OPENSSL_IS_AWSLC, and you either #define sk_X509_NAME_find sk_X509_NAME_find_awslc, or define a function named sk_X509_NAME_find in the Envoy namespace so that it's a closer match than the global namespace version, which does the needed API translation. I'd prefer that to sprinkling preprocessor directives around the code.

I'll make that fix.

Jenkins-J · 2025-02-14T15:34:52Z

@ggreenway I've made the change to move the preprocessor conditions to a header file. Let me know if there is a better location for the header file or a better place to include it in the build process.

ggreenway

This looks fine from the point-of-view of the TLS code; this is pretty unobtrusive.

I need someone else to review the bazel changes. Either @phlax or @keith probably.

Is it intended that use of aws-lc will always be FIPS compliant?

Please document somewhere that any builds using aws-lc or on PPC are not covered by the envoy security policy, and that PPC is currently best-effort and not maintained by the Envoy maintainers.

If in the future the boringssl and aws-lc APIs diverge, Envoy will follow boringssl, even if it breaks aws-lc.

source/common/tls/aws_lc_compat.h

keith

on the bazel side i think this looks good! but i would love it if there were links to upstream patches for all the new patches here so we can work to eliminate them. they all seem super reasonable so hopefully they can land upstream

keith · 2025-02-14T20:31:31Z

bazel/v8_ppc64le.patch

+ config_setting(
+     name = "platform_cpu_ppc64le",
+-    constraint_values = ["@platforms//cpu:ppc"],
+    constraint_values = ["@platforms//cpu:ppc64le"],


is there an upstream patch for this? seems like a good change

There is no upstream patch for this change yet.

keith · 2025-02-14T20:31:39Z

bazel/rules_rust_ppc64le.patch

+     "aarch64-pc-windows-msvc": "rust_windows_aarch64",
+     "aarch64-unknown-linux-gnu": "rust_linux_aarch64",
+     "s390x-unknown-linux-gnu": "rust_linux_s390x",
+    "powerpc64le-unknown-linux-gnu": "rust_linux_powerpc64le",


is there an upstream patch for all this?

I have submitted a PR for this one: bazelbuild/rules_rust#3259

keith · 2025-02-14T20:31:48Z

bazel/rules_java_ppc64le.patch

+     struct(
+         name = "remotejdk21_linux_ppc64le",
+-        target_compatible_with = ["@platforms//os:linux", "@platforms//cpu:ppc"],
+        target_compatible_with = ["@platforms//os:linux", "@platforms//cpu:ppc64le"],


upstream patch?

~~There is no upstream patch for this change yet.~~
I have submitted a PR for this change: bazelbuild/rules_java#274

keith · 2025-02-14T20:32:04Z

bazel/protobuf.patch

@@ -64,6 +72,7 @@ index 32b26cbdc..e28b8e387 100644
 +    actual = select({
 +        ":linux-aarch_64": "@com_google_protobuf_protoc_linux_aarch_64//:protoc",
 +        ":linux-x86_64": "@com_google_protobuf_protoc_linux_x86_64//:protoc",
+        ":linux-ppcle_64": "@com_google_protobuf_protoc_linux_ppcle_64//:protoc",


upstream patch?

This is one patch file that existed before my changes (I just added the linux-ppcle_64 case). I have not created an upstream patch for this one either.

this is our patch to try and prevent building of protoc

phlax · 2025-02-18T12:59:04Z

needs main merge and DCO fix

@keith will land when you are happy

/wait

Build Envoy with aws_lc instead of boringssl when building for the ppc64le architecture. Signed-off-by: James Jenkins <James.Jenkins@ibm.com>

Signed-off-by: James Jenkins <James.Jenkins@ibm.com>

Remove conditional compilation gaurds for aws-lc, adding the new header file to do the proper translation. Signed-off-by: James Jenkins <James.Jenkins@ibm.com>

Signed-off-by: James Jenkins <James.Jenkins@ibm.com>

Jenkins-J · 2025-02-18T14:56:36Z

needs main merge and DCO fix

@keith will land when you are happy

/wait

Rebased changes on the main branch and signed all commits.

phlax · 2025-02-19T13:52:11Z

@Jenkins-J the format and deps fails are real

Jenkins-J · 2025-02-19T14:35:02Z

@Jenkins-J the format and deps fails are real

Thank you, I will start fixing those now.

phlax · 2025-02-19T18:58:15Z

/retest flakey tsan/quic

Jenkins-J · 2025-02-19T21:36:42Z

@phlax There remains a "spelling" error in the aws_lc_compat.h file because the check identifies "ppc64le" as a spelling error. Is the preferred way to resolve this by adding "ppc64le" to the dictionary file or should I just remove the term from the comment?

ggreenway · 2025-02-19T22:20:36Z

Put backticks around it and the word won't be spell checked. Or possibly double backticks.

Signed-off-by: James Jenkins <James.Jenkins@ibm.com>

ggreenway · 2025-02-20T16:27:34Z

Please do not force-push/rebase. It makes PRs difficult to review.

envoy/CONTRIBUTING.md

Line 136 in 6317db1

    
           * Once your PR is under review, *please do not rebase it*. If you rebase, you will need to force push to

Jenkins-J · 2025-02-20T16:32:16Z

@ggreenway I'm so sorry, I did not see that before.

Jenkins-J · 2025-02-21T16:28:08Z

I have made all of the formatting and dependency corrections. There is one failing check but it appears to be unrelated to the changes in this PR. Do those checks need to be run again?

phlax · 2025-02-21T16:30:17Z

yep - looks unrelated - you can

/retest

... yourself if you need to (but please check it looks unrelated first)

phlax · 2025-02-21T16:31:36Z

argh - didnt work

/retest

phlax

lgtm, thanks @Jenkins-J

feedback addressed

Jenkins-J requested review from botengyao and tyxia as code owners February 11, 2025 15:49

repokitteh-read-only bot added the deps Approval required for changes to Envoy's external dependencies label Feb 11, 2025

repokitteh-read-only bot assigned moderation Feb 11, 2025

clnperez mentioned this pull request Feb 11, 2025

ppc64le CI build always fails #25090

Open

mattklein123 assigned ggreenway Feb 12, 2025

ggreenway requested changes Feb 12, 2025

View reviewed changes

ggreenway added the waiting label Feb 13, 2025

repokitteh-read-only bot removed the waiting label Feb 14, 2025

ggreenway previously requested changes Feb 14, 2025

View reviewed changes

source/common/tls/aws_lc_compat.h Show resolved Hide resolved

ggreenway assigned keith and phlax Feb 14, 2025

keith reviewed Feb 14, 2025

View reviewed changes

repokitteh-read-only bot added the waiting label Feb 18, 2025

Jenkins-J added 6 commits February 18, 2025 09:29

Build Envoy with aws_lc on Power

366f5ca

Build Envoy with aws_lc instead of boringssl when building for the ppc64le architecture. Signed-off-by: James Jenkins <James.Jenkins@ibm.com>

Create header file for aws-lc compatibility

c717e81

Signed-off-by: James Jenkins <James.Jenkins@ibm.com>

Remove conditional comilation for aws-lc

9ec3f44

Remove conditional compilation gaurds for aws-lc, adding the new header file to do the proper translation. Signed-off-by: James Jenkins <James.Jenkins@ibm.com>

Add aws_lc_compat header to BUILD file

649b661

Signed-off-by: James Jenkins <James.Jenkins@ibm.com>

Document usage of aws_lc_compat header

e9ee9b9

Signed-off-by: James Jenkins <James.Jenkins@ibm.com>

Document security implications of ppc64le builds

b224c51

Signed-off-by: James Jenkins <James.Jenkins@ibm.com>

Jenkins-J force-pushed the aws-lc-fips-1.33.0-pr branch from 5bace09 to b224c51 Compare February 18, 2025 14:54

repokitteh-read-only bot removed the waiting label Feb 18, 2025

keith previously approved these changes Feb 18, 2025

View reviewed changes

Jenkins-J dismissed keith’s stale review via bd7025b February 19, 2025 16:53

Jenkins-J force-pushed the aws-lc-fips-1.33.0-pr branch from bd7025b to 0ecbfb9 Compare February 19, 2025 19:09

Fix formatting and deps errors

25d2f46

Signed-off-by: James Jenkins <James.Jenkins@ibm.com>

Jenkins-J force-pushed the aws-lc-fips-1.33.0-pr branch from 0ecbfb9 to 25d2f46 Compare February 20, 2025 14:44

phlax approved these changes Feb 21, 2025

View reviewed changes

repokitteh-read-only bot removed the deps Approval required for changes to Envoy's external dependencies label Feb 21, 2025

phlax enabled auto-merge (squash) February 21, 2025 16:35

phlax merged commit 60aed1c into envoyproxy:main Feb 21, 2025
26 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Build Envoy with aws_lc on Power (ppc64le) #38403

Build Envoy with aws_lc on Power (ppc64le) #38403

Jenkins-J commented Feb 11, 2025

repokitteh-read-only bot commented Feb 11, 2025

mattklein123 commented Feb 12, 2025

phlax commented Feb 12, 2025

ggreenway left a comment

ggreenway Feb 12, 2025

ggreenway Feb 12, 2025

Jenkins-J Feb 12, 2025

Jenkins-J Feb 12, 2025

Jenkins-J commented Feb 14, 2025

ggreenway left a comment

keith left a comment

keith Feb 14, 2025

Jenkins-J Feb 14, 2025

keith Feb 14, 2025

Jenkins-J Feb 14, 2025

keith Feb 14, 2025

Jenkins-J Feb 14, 2025 •

edited

Loading

keith Feb 14, 2025

Jenkins-J Feb 14, 2025 •

edited

Loading

phlax Feb 19, 2025

phlax commented Feb 18, 2025

Jenkins-J commented Feb 18, 2025

phlax commented Feb 19, 2025

Jenkins-J commented Feb 19, 2025

phlax commented Feb 19, 2025

Jenkins-J commented Feb 19, 2025

ggreenway commented Feb 19, 2025

ggreenway commented Feb 20, 2025

Jenkins-J commented Feb 20, 2025

Jenkins-J commented Feb 21, 2025

phlax commented Feb 21, 2025

phlax commented Feb 21, 2025

phlax left a comment

Build Envoy with aws_lc on Power (ppc64le) #38403

Build Envoy with aws_lc on Power (ppc64le) #38403

Conversation

Jenkins-J commented Feb 11, 2025

repokitteh-read-only bot commented Feb 11, 2025

mattklein123 commented Feb 12, 2025

phlax commented Feb 12, 2025

ggreenway left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Jenkins-J commented Feb 14, 2025

ggreenway left a comment

Choose a reason for hiding this comment

keith left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Jenkins-J Feb 14, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Jenkins-J Feb 14, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

phlax commented Feb 18, 2025

Jenkins-J commented Feb 18, 2025

phlax commented Feb 19, 2025

Jenkins-J commented Feb 19, 2025

phlax commented Feb 19, 2025

Jenkins-J commented Feb 19, 2025

ggreenway commented Feb 19, 2025

ggreenway commented Feb 20, 2025

Jenkins-J commented Feb 20, 2025

Jenkins-J commented Feb 21, 2025

phlax commented Feb 21, 2025

phlax commented Feb 21, 2025

phlax left a comment

Choose a reason for hiding this comment

Jenkins-J Feb 14, 2025 •

edited

Loading

Jenkins-J Feb 14, 2025 •

edited

Loading