[Security][XDS] Support Verification with SPIFFE Bundle Maps by gtcooke94 · Pull Request #40321 · grpc/grpc

gtcooke94 · 2025-07-24T20:45:23Z

This PR adds APIs discussed in grpc/proposal#506 and https://github.com/grpc/proposal/blob/master/A87-mtls-spiffe-support.md to support verification with SPIFFE Bundle Map roots.

RELEASE NOTES:

Adds support for SPIFFE Bundle Maps in as roots of trust per gRFC A87 and gRFC L127

yashykt · 2025-07-28T18:40:16Z

src/core/credentials/transport/tls/spiffe_utils.cc

+      "No spiffe bundle found for trust domain %s", trust_domain));
+}
+
 }  // namespace grpc_core


please add new line

yashykt · 2025-07-28T18:40:32Z

test/cpp/end2end/BUILD

+        "//test/core/test_util:grpc_test_util",
+        "//test/cpp/util:test_util",
+    ],
+)


please add new line

yashykt · 2025-07-28T18:40:47Z

test/cpp/end2end/xds/xds_security_spiffe_end2end_test.cc

+  const auto result = RUN_ALL_TESTS();
+  grpc_shutdown();
+  return result;
+}


please add new line

matthewstevenson88

Thanks for the PR! I already did a review when this was on your personal branch (linked below for posterity). LGTM, modulo a few small nits I noticed while doing a final pass on the PR.

src/core/credentials/transport/tls/spiffe_utils.cc

src/core/credentials/transport/tls/spiffe_utils.h

src/core/tsi/ssl_transport_security_utils.h

test/cpp/end2end/xds/xds_end2end_test_lib.cc

Automated fix for refs/heads/spiffe_verification

matthewstevenson88

Per request, did a review pass of the newly-added tests. No major concerns, just some nits and clarification suggestions. Deferring to Yash to review the xds_security_spiffe_end2end_test.cc file, as I am not qualified to review for the XDS logic.

test/core/tsi/spiffe_ssl_transport_security_test.cc

test/cpp/end2end/spiffe_bundle_map_end2end_test.cc

test/core/tsi/spiffe_ssl_transport_security_test.cc

test/cpp/end2end/spiffe_bundle_map_end2end_test.cc

pawbhard

LGTM, from xds e2e test side .added some nit commrnts

pawbhard · 2025-08-09T04:55:47Z

test/cpp/end2end/xds/BUILD

+        "absl/log:check",
+        "gtest",
+    ],
+    # flaky = True,  # TODO(b/144705388)


nit: cleanup

pawbhard · 2025-08-09T04:57:00Z

test/cpp/end2end/xds/BUILD

+        "no_test_ios",
+        "no_windows",
+        "xds_end2end_test",
+    ],  # TODO(jtattermusch): fix test on windows


Does it need to fix on windows, or copy-paste ?
If yes add TODO on your name or existing person name

Copy paste, removing TODO

pawbhard · 2025-08-09T05:23:05Z

test/core/tsi/test_creds/spiffe_end2end/README.md

@@ -0,0 +1,38 @@
+All of the following files in this directory except `server_spiffebundle.json`
+and `client_spiffebundle.json` are generated with the `generate.sh` and
+`generate_intermediate.sh` script in this directory.


nit: Do we need to generate them again , if yes when ? Should we add that in this README?

Added, not sure the exact date, I believe the scripts usually do 10 years for cert expiry

pawbhard · 2025-08-09T05:35:37Z

test/cpp/end2end/xds/xds_security_spiffe_end2end_test.cc

+  }
+
+  grpc_core::UniqueTypeName type() const override {
+    static grpc_core::UniqueTypeName::Factory kFactory("fake");


nit : use GRPC_UNIQUE_TYPE_NAME_HERE()

pawbhard · 2025-08-09T05:36:12Z

test/cpp/end2end/xds/xds_security_spiffe_end2end_test.cc

+
+ private:
+  int CompareImpl(const grpc_tls_certificate_provider* other) const override {
+    // TODO(yashykt): Maybe do something better here.


Maybe add TODO to yourself or existing gRPC member .
Same for rest of the PR

These were copy paste TODOs, just removed them

pawbhard · 2025-08-09T05:38:57Z

test/cpp/end2end/xds/xds_security_spiffe_end2end_test.cc

+    identity_pair_ = ReadTlsIdentityPair(kClientKeyPath, kClientCertPath);
+
+    // TODO(yashykt): Use different client certs here instead of reusing
+    // server certs after https://github.com/grpc/grpc/pull/24876 is merged


PR is already merged.

) This PR adds APIs discussed in grpc/proposal#506 and https://github.com/grpc/proposal/blob/master/A87-mtls-spiffe-support.md to support verification with SPIFFE Bundle Map roots. RELEASE NOTES: * Adds support for SPIFFE Bundle Maps in as roots of trust per [gRFC A87](https://github.com/grpc/proposal/blob/master/A87-mtls-spiffe-support.md) and [gRFC L127](grpc/proposal#506) Closes grpc#40321 COPYBARA_INTEGRATE_REVIEW=grpc#40321 from gtcooke94:spiffe_verification 6ddc24e PiperOrigin-RevId: 793688101

…rpc#40321)" This reverts commit 65cdfba.

Roll forward #40321 with fixes relating to OpenSSL 1.0.2 This roll forward fixes two things broken by the original commit: * OpenSSL 1.0.2 compatibility - `X509_up_ref` is not in 1.0.2, so `CRYPTO_add` is used along with compiler directives. * The macOS tests flatten trust bundles, and two files in different directories were both named `ca.pem` in the new `spiffe_bundle_map_end2end_test.cc`. One was from the existing test that this new test file was modeled after and was not needed, so it was removed from the BUILD file resolving the double naming conflict. Closes #40476 COPYBARA_INTEGRATE_REVIEW=#40476 from gtcooke94:spiffe_roll_forward e30b7e4 PiperOrigin-RevId: 796537764

Roll forward grpc#40321 with fixes relating to OpenSSL 1.0.2 This roll forward fixes two things broken by the original commit: * OpenSSL 1.0.2 compatibility - `X509_up_ref` is not in 1.0.2, so `CRYPTO_add` is used along with compiler directives. * The macOS tests flatten trust bundles, and two files in different directories were both named `ca.pem` in the new `spiffe_bundle_map_end2end_test.cc`. One was from the existing test that this new test file was modeled after and was not needed, so it was removed from the BUILD file resolving the double naming conflict. Closes grpc#40476 COPYBARA_INTEGRATE_REVIEW=grpc#40476 from gtcooke94:spiffe_roll_forward e30b7e4 PiperOrigin-RevId: 796537764

) This PR adds APIs discussed in grpc/proposal#506 and https://github.com/grpc/proposal/blob/master/A87-mtls-spiffe-support.md to support verification with SPIFFE Bundle Map roots. RELEASE NOTES: * Adds support for SPIFFE Bundle Maps in as roots of trust per [gRFC A87](https://github.com/grpc/proposal/blob/master/A87-mtls-spiffe-support.md) and [gRFC L127](grpc/proposal#506) Closes grpc#40321 COPYBARA_INTEGRATE_REVIEW=grpc#40321 from gtcooke94:spiffe_verification 6ddc24e PiperOrigin-RevId: 793688101

Roll forward grpc#40321 with fixes relating to OpenSSL 1.0.2 This roll forward fixes two things broken by the original commit: * OpenSSL 1.0.2 compatibility - `X509_up_ref` is not in 1.0.2, so `CRYPTO_add` is used along with compiler directives. * The macOS tests flatten trust bundles, and two files in different directories were both named `ca.pem` in the new `spiffe_bundle_map_end2end_test.cc`. One was from the existing test that this new test file was modeled after and was not needed, so it was removed from the BUILD file resolving the double naming conflict. Closes grpc#40476 COPYBARA_INTEGRATE_REVIEW=grpc#40476 from gtcooke94:spiffe_roll_forward e30b7e4 PiperOrigin-RevId: 796537764

) This PR adds APIs discussed in grpc/proposal#506 and https://github.com/grpc/proposal/blob/master/A87-mtls-spiffe-support.md to support verification with SPIFFE Bundle Map roots. RELEASE NOTES: * Adds support for SPIFFE Bundle Maps in as roots of trust per [gRFC A87](https://github.com/grpc/proposal/blob/master/A87-mtls-spiffe-support.md) and [gRFC L127](grpc/proposal#506) Closes grpc#40321 COPYBARA_INTEGRATE_REVIEW=grpc#40321 from gtcooke94:spiffe_verification 6ddc24e PiperOrigin-RevId: 793688101

Roll forward grpc#40321 with fixes relating to OpenSSL 1.0.2 This roll forward fixes two things broken by the original commit: * OpenSSL 1.0.2 compatibility - `X509_up_ref` is not in 1.0.2, so `CRYPTO_add` is used along with compiler directives. * The macOS tests flatten trust bundles, and two files in different directories were both named `ca.pem` in the new `spiffe_bundle_map_end2end_test.cc`. One was from the existing test that this new test file was modeled after and was not needed, so it was removed from the BUILD file resolving the double naming conflict. Closes grpc#40476 COPYBARA_INTEGRATE_REVIEW=grpc#40476 from gtcooke94:spiffe_roll_forward e30b7e4 PiperOrigin-RevId: 796537764

reset merge base

8f11c34

github-actions bot added lang/c++ lang/core labels Jul 24, 2025

gtcooke94 added area/security area/api release notes: no Indicates if PR should not be in release notes and removed lang/c++ lang/core labels Jul 24, 2025

grpc-checks bot added per-call-memory/neutral per-channel-memory/neutral bloat/medium labels Jul 24, 2025

gtcooke94 requested a review from yashykt July 28, 2025 18:37

yashykt reviewed Jul 28, 2025

View reviewed changes

fix broken test

5194d7d

github-actions bot added lang/c++ lang/core labels Jul 28, 2025

gtcooke94 added 5 commits July 28, 2025 18:56

change existing API

a401d06

cleanup, add newlines

42ae149

fix wrong api call

82bc3b2

other api usage

c1c634b

swap to auto in loop

b4eee8b

grpc-checks bot added bloat/low and removed bloat/medium labels Jul 28, 2025

gtcooke94 requested a review from yashykt July 28, 2025 21:08

fix usings

097078f

grpc-checks bot added bloat/medium and removed bloat/low labels Jul 29, 2025

add copyright

004cd59

matthewstevenson88 approved these changes Jul 29, 2025

View reviewed changes

address PR comments

4a566ea

gtcooke94 and others added 4 commits July 30, 2025 16:08

Automated change: Fix sanity tests

5f9e816

Merge pull request #166 from gtcooke94/create-pull-request/patch-262c452

15a6cad

Automated fix for refs/heads/spiffe_verification

shellcheck

ff06c27

use regex for failure message mapping

7727852

matthewstevenson88 reviewed Jul 31, 2025

View reviewed changes

gtcooke94 added 7 commits July 31, 2025 17:38

clang tidy

e343f19

pr comments on spiffe_ssl_transport_security_test

6ceea98

comments on spiffe_bundle_map_end2end_test

f398a95

remove unneeded var

2673dc1

only test the regex on featured impls

b580e71

x509v3 for general name

494c28d

rename root_cert_

6ddc24e

gtcooke94 added the kokoro:force-run label Aug 6, 2025

grpc-kokoro removed the kokoro:force-run label Aug 6, 2025

pawbhard approved these changes Aug 9, 2025

View reviewed changes

gtcooke94 added 2 commits August 11, 2025 15:57

remove TODOs, PR nits

2564319

Add README comment

f4a36d8

copybara-service bot closed this in 65cdfba Aug 11, 2025

gtcooke94 added a commit to gtcooke94/grpc that referenced this pull request Aug 14, 2025

Revert "[Security][XDS] Support Verification with SPIFFE Bundle Maps (g…

392219a

…rpc#40321)" This reverts commit 65cdfba.

gtcooke94 mentioned this pull request Aug 18, 2025

[Security] Spiffe Verification roll forward #40476

Closed

anniefrchz mentioned this pull request Aug 22, 2025

ALTS Integrity-only Negotiation anniefrchz/grpc#24

Closed

Conversation

gtcooke94 commented Jul 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

matthewstevenson88 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

matthewstevenson88 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

pawbhard left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

gtcooke94 commented Jul 24, 2025 •

edited

Loading