Validate Certificate Identity provided in Cross Cluster API Key Certificate by jfreden · Pull Request #136299 · elastic/elasticsearch

jfreden · 2025-10-09T14:35:34Z

This is a follow up to #134137, #134893, #135674 and #134604.

This PR pulls the DN out of the provided signature leaf certificate and uses it to match against the configured certificate_identity pattern on the cross cluster api key.

This PR does not deal with audit logging, that will be done in a followup PR.

gmjehovich · 2025-10-09T23:21:05Z

.../org/elasticsearch/xpack/remotecluster/RemoteClusterSecurityCrossClusterApiKeySigningIT.java

+            assertCrossClusterSearchSuccessfulWithResult();
+
+            // Change the CA to the default trust store to make sure untrusted signature fails auth even if it's not required
+            updateClusterSettingsFulfillingCluster(Settings.builder().putNull("cluster.remote.signing.certificate_authorities").build());


I am curious, why fail if a certificate is invalid but not required? Is it to protect against misconfiguration?

gmjehovich · 2025-10-09T23:33:01Z

...ain/java/org/elasticsearch/xpack/security/authc/CrossClusterAccessAuthenticationService.java

+            // Always validate a signature if provided
+            if (signature != null && verifySignature(signature, crossClusterAccessHeaders, listener) == false) {
+                // TODO audit logging
+                return;


With signature verification happening before API key validation, if the certificate is invalid but the API key is also expired, the user sees 'invalid certificate' and never learns about the expired key. Is this the intended priority?

I do think this way makes sense; if trust can't be established, then there isn't much point in validating the API key itself

The order right now is:

Validate that the API key is valid, this happens first because of authenticateHeaders

Validate that the provided certificate identity is ok also happens in authenticateHeaders

Validate signature

With the current implementation (1) and (2) would actually not generate an audit log from what I can see. Since (1) is the current implementation I think it's a bug.

Is this the intended priority?

My thinking is that validating the signature on every request, even failing ones is potentially expensive, so doing it in auth headers is probably not right. The point of auth headers is to short circuit the auth flow.

Doing the signature verification first in the auth flow decouples it from the rest of the api key service, if we don't do it there we require the api key credentials to contain all the signable payload + signature info and to pass it along to the api key service where we do validation. Not sure if that's a good enough reason, but I don't think it matters if we do it first or last, so keeping decoupling and doing it early is probably acceptable.

What do you think?

Realizing now I responded to you on Slack but not here. This argument makes sense to me, I don't ultimately see a huge difference between checking it first or last, and architecturally it's easier to do here than trying to couple it into the rest of the auth flow.

gmjehovich · 2025-10-09T23:37:42Z

.../org/elasticsearch/xpack/remotecluster/RemoteClusterSecurityCrossClusterApiKeySigningIT.java

+            assertCrossClusterSearchSuccessfulWithResult();
+
+            // Change the CA to the default trust store to make sure untrusted signature fails auth even if it's not required
+            updateClusterSettingsFulfillingCluster(Settings.builder().putNull("cluster.remote.signing.certificate_authorities").build());


I am curious, why fail if a certificate is invalid but not required? Is it to catch misconfigurations early?

Yes, if a signature is provided I think we should try to validate it and fail if it's not correct since it's unexpected. Better to be less lenient in this case I think.

It also gives customers a way to progressively switch over to signing. If they configure signing on the origin cluster, and it doesn't break, then they can rely on the fact that it's probably working correctly. Then they can turn on enforcement on the linked cluster.

And if that first step does break, then they can just turn off signing on the origin, without needing to reconfigure both sides.

gmjehovich · 2025-10-09T23:40:14Z

Thanks for working on this! The approach looks good. Moving cert validation into the authentication flow should fix the double audit logging I was seeing. I just left some questions to help with my understanding

jfreden · 2025-10-14T14:41:45Z

...plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/ApiKeyServiceTests.java

        }
    }

-    private ApiKeyService createApiKeyService(Settings baseSettings, FeatureService customFeatureService) {


This was unused so I removed it.

gmjehovich

I looked some into the test failures from the CI. The serverless ones seem unrelated, but the failing test on the ci test is interesting.

It looks like that case in testInvalidHeaders never triggers the callback passed to the call to authenticationService.authenticate as [this line] (

elasticsearch/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/CrossClusterAccessAuthenticationService.java

Line 127 in 87edbc9

    
           final CrossClusterAccessSubjectInfo subjectInfo = crossClusterAccessHeaders.getCleanAndValidatedSubjectInfo();

) with the call to crossClusterAccessHeaders.getCleanAndValidatedSubjectInfo() should be triggering that exception I think

x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/ApiKeyService.java

.../org/elasticsearch/xpack/remotecluster/RemoteClusterSecurityCrossClusterApiKeySigningIT.java

gmjehovich · 2025-10-14T23:48:48Z

...ain/java/org/elasticsearch/xpack/security/authc/CrossClusterAccessAuthenticationService.java

+            // Always validate a signature if provided
+            if (signature != null && verifySignature(signature, crossClusterAccessHeaders, listener) == false) {
+                // TODO audit logging
+                return;


Realizing now I responded to you on Slack but not here. This argument makes sense to me, I don't ultimately see a huge difference between checking it first or last, and architecturally it's easier to do here than trying to couple it into the rest of the auth flow.

x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/ApiKeyService.java

elasticsearchmachine · 2025-10-15T09:04:34Z

Hi @jfreden, I've created a changelog YAML for you.

elasticsearchmachine · 2025-10-15T09:05:35Z

Pinging @elastic/es-security (Team:Security)

jfreden · 2025-10-15T14:40:03Z

Ended up adding a pattern cache. Still need to add some testing for it but the basics are there. Will continue tomorrow.

ebarlas · 2025-10-20T03:22:05Z

x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/ApiKeyService.java

    private final boolean enabled;
    private final Settings settings;
    private final InactiveApiKeysRemover inactiveApiKeysRemover;
+    private final Cache<String, Pattern> certificateIdentityPatternCache;


What was the motivation for certificateIdentityPatternCache? Is regex compilation measurably slow relative to the surrounding changes (e.g. signature verification)?

I was on the fence about this initially and didn't add a cache.

We accept arbitrary patterns, so measuring if it's slow relative the surrounding changes is difficult. Another consideration is that the Pattern object can be large in memory, so a cache could make things worse. My guess is that most of the time the patterns won't be that expensive and like you point out, the pattern compilation is probably not that slow.

What convinced me to eventually add it was that we check the pattern twice per request, so if it's a complex pattern it could be expensive. This is also something that's very easy to cache and if we add a signature cache in the future it could be the next bottleneck.

ebarlas · 2025-10-20T03:51:47Z

...main/java/org/elasticsearch/xpack/security/transport/CrossClusterApiKeySignatureManager.java

@@ -192,9 +192,6 @@ public boolean verify(X509CertificateSignature signature, String... headers) thr
            }
            authTrustManager.checkClientTrusted(signature.certificates(), signature.certificates()[0].getPublicKey().getAlgorithm());



Small suggestion: consider using leaf rather than signature.certificates()[0] since the intermediate var is defined above in method scope. Relatedly, it might be worthwhile to introduce a leafCerificate method (or similar name) on X509CertificateSignature.

That's a great idea! I've added a leafCerificate() method and updated to use the leaf variable instead.

jfreden · 2025-10-21T09:46:23Z

...main/java/org/elasticsearch/xpack/security/transport/CrossClusterApiKeySignatureManager.java

                );
            }
-            authTrustManager.checkClientTrusted(signature.certificates(), signature.certificates()[0].getPublicKey().getAlgorithm());
+            for (var certificate : signature.certificates()) {


Added this to make sure we don't accept expired certs.

From my testing it looks like if I add an expired cert to the trust store, we trust it even if it's expired unless we do this explicit check.

jfreden · 2025-10-23T11:10:15Z

CI failing is unrelated, see: https://github.com/elastic/elasticsearch-serverless/issues/4737

…ficate (elastic#136299) * Validate certificate identity from cross cluster creds Co-authored-by: elasticsearchmachine <infra-root+elasticsearchmachine@elastic.co> Co-authored-by: Tim Vernum <tim@adjective.org>

This adds documentation for the RCS Strong Verification feature added in elastic/elasticsearch#136299, elastic/elasticsearch#134137, elastic/elasticsearch#134893, elastic/elasticsearch#135674 and elastic/elasticsearch#134604. Related settings docs PR: elastic/elasticsearch#137822 --------- Co-authored-by: shainaraskas <58563081+shainaraskas@users.noreply.github.com> Co-authored-by: Edu González de la Herrán <25320357+eedugon@users.noreply.github.com>

elasticsearchmachine added the v9.3.0 label Oct 9, 2025

gmjehovich reviewed Oct 9, 2025

View reviewed changes

jfreden force-pushed the validate_signature_identity branch from 79d328b to d1293d5 Compare October 10, 2025 11:46

jfreden changed the title ~~Validate signature identity~~ Oct 10, 2025

jfreden force-pushed the validate_signature_identity branch 3 times, most recently from 93eef5b to cc2e580 Compare October 14, 2025 14:40

jfreden commented Oct 14, 2025

View reviewed changes

gmjehovich reviewed Oct 15, 2025

View reviewed changes

jfreden force-pushed the validate_signature_identity branch from 87edbc9 to 2618ac9 Compare October 15, 2025 08:02

Validate certificate identity from cross cluster creds

99fac10

jfreden force-pushed the validate_signature_identity branch from 8cce705 to 99fac10 Compare October 15, 2025 08:57

jfreden commented Oct 15, 2025

View reviewed changes

x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/ApiKeyService.java Outdated Show resolved Hide resolved

jfreden added :Security/Security Security issues without another label >enhancement labels Oct 15, 2025

Update docs/changelog/136299.yaml

0cc2ea3

jfreden changed the title ~~Validate certificate identity from cross cluster creds~~ Oct 15, 2025

jfreden marked this pull request as ready for review October 15, 2025 09:05

jfreden requested a review from tvernum October 15, 2025 09:05

elasticsearchmachine added the Team:Security Meta label for security team label Oct 15, 2025

elasticsearchmachine and others added 2 commits October 15, 2025 09:11

[CI] Auto commit changes from spotless

69c9a17

fixup! Pattern match

eb7a3f7

jfreden requested a review from a team October 15, 2025 14:27

Add pattern cache

d63b4ec

gmjehovich mentioned this pull request Oct 16, 2025

Certificate audit logging #136660

Closed

jfreden force-pushed the validate_signature_identity branch from f39c655 to ed4093a Compare October 17, 2025 11:29

fixup! Import

094c15f

jfreden added the cloud-deploy Publish cloud docker image for Cloud-First-Testing label Oct 17, 2025

[CI] Auto commit changes from spotless

a71ca11

ebarlas reviewed Oct 20, 2025

View reviewed changes

jfreden and others added 6 commits October 21, 2025 09:41

fixup! Code review - add test

466badf

[CI] Auto commit changes from spotless

48e5321

fixup! Code review - add leafCertificate method

f40b0c3

Add code + test to make sure provided certs are not expired

8a16d08

[CI] Auto commit changes from spotless

039e8ba

Merge branch 'main' into validate_signature_identity

5b52cbe

jfreden commented Oct 21, 2025

View reviewed changes

fixup! Remove duplicate create call

aec1249

jfreden requested a review from tvernum October 21, 2025 14:38

fixup! Validate exipry trust anchor

566770e

jfreden force-pushed the validate_signature_identity branch from f39ddba to 566770e Compare October 22, 2025 13:38

jfreden and others added 3 commits October 22, 2025 15:55

Merge branch 'main' into validate_signature_identity

a758ba0

fixup! Remove trust anchor validit check

897f87a

Merge branch 'main' into validate_signature_identity

f88aecb

jfreden added the auto-merge-without-approval Automatically merge pull request when CI checks pass (NB doesn't wait for reviews!) label Oct 23, 2025

Merge branch 'main' into validate_signature_identity

329ede5

jfreden merged commit f44b8b2 into elastic:main Oct 23, 2025
40 of 41 checks passed

jfreden deleted the validate_signature_identity branch October 23, 2025 11:10

This was referenced Nov 10, 2025

Add RCS Strong Verification Documentation #137822

Merged

Add RCS Strong Verification Documentation elastic/docs-content#3864

Closed

Add RCS Strong Verification Documentation elastic/docs-content#3865

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Validate Certificate Identity provided in Cross Cluster API Key Certificate#136299

Validate Certificate Identity provided in Cross Cluster API Key Certificate#136299
jfreden merged 28 commits intoelastic:mainfrom
jfreden:validate_signature_identity

jfreden commented Oct 9, 2025 •

edited

Loading

gmjehovich Oct 9, 2025

gmjehovich Oct 9, 2025

jfreden Oct 10, 2025

jfreden Oct 10, 2025

gmjehovich Oct 14, 2025

gmjehovich Oct 9, 2025

jfreden Oct 10, 2025

tvernum Oct 17, 2025 •

edited

Loading

gmjehovich commented Oct 9, 2025

jfreden Oct 14, 2025

gmjehovich left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

gmjehovich Oct 14, 2025

Uh oh!

elasticsearchmachine commented Oct 15, 2025

elasticsearchmachine commented Oct 15, 2025

jfreden commented Oct 15, 2025

ebarlas Oct 20, 2025

jfreden Oct 21, 2025

ebarlas Oct 20, 2025

jfreden Oct 21, 2025

jfreden Oct 21, 2025

jfreden Oct 21, 2025 •

edited

Loading

jfreden commented Oct 23, 2025

Uh oh!

Labels

5 participants

		@@ -192,9 +192,6 @@ public boolean verify(X509CertificateSignature signature, String... headers) thr
		}
		authTrustManager.checkClientTrusted(signature.certificates(), signature.certificates()[0].getPublicKey().getAlgorithm());

Conversation

jfreden commented Oct 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

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

Choose a reason for hiding this comment

tvernum Oct 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

gmjehovich commented Oct 9, 2025

Choose a reason for hiding this comment

gmjehovich left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

elasticsearchmachine commented Oct 15, 2025

elasticsearchmachine commented Oct 15, 2025

jfreden commented Oct 15, 2025

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

jfreden Oct 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

jfreden commented Oct 23, 2025

Uh oh!

Labels

5 participants

jfreden commented Oct 9, 2025 •

edited

Loading

tvernum Oct 17, 2025 •

edited

Loading

jfreden Oct 21, 2025 •

edited

Loading