Skip to content

Update bundled JDK to 25 #134910

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.

Sign up for GitHub

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

Merged
merged 15 commits into from
Sep 19, 2025
+34 −14
Merged

Update bundled JDK to 25 #134910

Show file tree
Hide file tree
Changes from 11 commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
797d367
Update bundled JDK to 25
mosche Sep 17, 2025
fbe6358
Fix UriUtilsTests
mark-vieira Sep 17, 2025
1fc595d
Use static import
mark-vieira Sep 17, 2025
ead33a0
Merge branch 'main' into bundled_jdk25
mark-vieira Sep 17, 2025
2005459
Merge branch 'main' into bundled_jdk25
mosche Sep 18, 2025
c20b698
Fix ESQL operator overflow warning assertions on JDK 25
mosche Sep 18, 2025
128908c
Fix more uri parsing tests
mark-vieira Sep 18, 2025
5564e20
Merge branch 'main' into fork/mosche/bundled_jdk25
mark-vieira Sep 18, 2025
341cca1
Update x-pack/plugin/esql/build.gradle
mosche Sep 18, 2025
00d885e
Fix another url test
mark-vieira Sep 18, 2025
c9eb22d
Fix FIPS failures?
mark-vieira Sep 18, 2025
1f6a3c8
Fix another one
mark-vieira Sep 18, 2025
1811da4
Merge branch 'main' into fork/mosche/bundled_jdk25
mark-vieira Sep 18, 2025
63fce46
fix fips exceptions
mosche Sep 19, 2025
e703280
Merge branch 'main' into bundled_jdk25
mosche Sep 19, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions build-tools-internal/src/main/groovy/elasticsearch.fips.gradle
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,7 @@ if (buildParams.inFipsJvm) {
task.systemProperty('java.security.policy', String.format(Locale.ROOT, "=%s", fipsPolicy))
task.systemProperty('javax.net.ssl.trustStore', fipsTrustStore)
task.systemProperty('org.bouncycastle.fips.approved_only', 'true')
task.systemProperty('jdk.includeInExceptions', 'hostInfo')
Copy link
Contributor

Choose a reason for hiding this comment

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

@rjernst this fixes a category of failures on FIPS, mostly around URI parsing, where the returned exception messages lack specifics that might leak sensitive information (i.e. "illegal character at index n"). This behavior seems to be enabled in "FIPS mode", but can be overriden via a system property (see code change). Is this actually going to be problematic? Do we need to set this system property by default in ES so that we get the same behavior even when running on FIPS, since we do our own redaction of URIs?

Copy link
Contributor

Choose a reason for hiding this comment

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

Here's an example failure: https://gradle-enterprise.elastic.co/s/z75k5utsjqnxu/tests/task/:x-pack:plugin:sql:sql-client:test/details/org.elasticsearch.xpack.sql.client.UriUtilsTests/testUriRedactionInvalidFragment?top-execution=1

Essentially, in the FIPS mode we cannot parse where the problematic characters are from the thrown Exception (the JDK omits this for security reasons) so we return a generic <REDACTED> instead. It's worth noting that we only hit this a) in FIPS mode (or at least as we configure it for testing) and b) when we are parsing an invalid URI. So presumably this is so we don't leak credentials with logs and error messages. I think the end result is when running on FIPS if folks provide bad URIs in configuration or other requests, they might get a slightly less useful error message unless we pass the system property above. Thoughts?

Copy link
Member

Choose a reason for hiding this comment

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

Perhaps @tvernum can weigh in here. I think it would be ok to be consistent at the expense of less useful error messages, but I could go either way.

Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry, I don't quite follow how "fips mode" changes the behaviour (that is, what about our fips tests triggers the change)

From the JDK 25 docs it sounds like the JDK never includes this information without the system property, but your description seem to imply otherwise.

Copy link
Contributor

Choose a reason for hiding this comment

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

OK, I think I worked it out.

The set of possibly values for the jdk.includeInExceptions property has expanded between JDK24 and JDK25

  • In JDK 24, the options were hostInfo and jar and the default was empty.
  • In JDK 25, the options are hostInfo, hostInfoExclSocket, jar and userInfo and the "default" is hostInfoExclSocket but the default is actually configured in conf/security/java.properties

Our FIPS tests run with a custom security config (that's how you enable FIPS, so we need to copy that new default into our FIPS setup

diff --git i/build-tools-internal/src/main/resources/fips_java.security w/build-tools-internal/src/main/resources/fips_java.security
index 1018b1b96ac..6bed997bfe8 100644
--- i/build-tools-internal/src/main/resources/fips_java.security
+++ w/build-tools-internal/src/main/resources/fips_java.security
@@ -49,3 +49,7 @@ jdk.xml.dsig.secureValidationPolicy=\
     noRetrievalMethodLoops
 jceks.key.serialFilter = java.base/java.lang.Enum;java.base/java.security.KeyRep;\
   java.base/java.security.KeyRep$Type;java.base/javax.crypto.spec.SecretKeySpec;!*
+
+# Needed since JDK25 to match the default config in the out-of-the-box java.security
+jdk.includeInExceptions=hostInfoExclSocket
+
diff --git i/build-tools-internal/src/main/resources/fips_java_oracle.security w/build-tools-internal/src/main/resources/fips_java_oracle.security
index 875151a058e..c50f85202a1 100644
--- i/build-tools-internal/src/main/resources/fips_java_oracle.security
+++ w/build-tools-internal/src/main/resources/fips_java_oracle.security
@@ -50,3 +50,7 @@ jdk.xml.dsig.secureValidationPolicy=\
     noRetrievalMethodLoops
 jceks.key.serialFilter = java.base/java.lang.Enum;java.base/java.security.KeyRep;\
   java.base/java.security.KeyRep$Type;java.base/javax.crypto.spec.SecretKeySpec;!*
+
+# Needed since JDK25 to match the default config in the out-of-the-box java.security
+jdk.includeInExceptions=hostInfoExclSocket
+

(From my testing that doesn't break older JDKs, and it shouldn't, they just ignore unrecognized options)

It's possible we need to also add it to the versions in test/test-clusters/src/main/resources/fips/ as well.

But if we do that, then we don't need this system-property change.

Copy link
Contributor

Choose a reason for hiding this comment

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

For reference this is the effective set of differences for java.security between Oracle's OpenJDK v24 and v25

$ diff <(grep '^[^#]' ~/.gradle/jdks/oracle_corporation-24-aarch64-os_x.2/jdk-24.jdk/Contents/Home/conf/security/java.security) <(grep '^[^#]' ~/.gradle/jdks/oracle_corporation-25-aarch64-os_x.2/jdk-25.jdk/Contents/Home/conf/security/java.security)
40c40,41
<     ECDH, TLS_RSA_*
---
>     ECDH, TLS_RSA_*, rsa_pkcs1_sha1 usage HandshakeSignature, \
>     ecdsa_sha1 usage HandshakeSignature, dsa_sha1 usage HandshakeSignature
64a66
> jdk.includeInExceptions=hostInfoExclSocket
66c68
< jdk.security.caDistrustPolicies=SYMANTEC_TLS,ENTRUST_TLS
---
> jdk.security.caDistrustPolicies=SYMANTEC_TLS,ENTRUST_TLS,CAMERFIRMA_TLS
68a71
> jdk.epkcs8.defaultAlgorithm=PBEWithHmacSHA256AndAES_128

I don't think any other other changes are going to be a problem between FIPS/non-FIPS.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks @tvernum for having a look 👍 I'll push the suggested changes

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@tvernum Actually, wondering, are you ok doing this as a follow up? I don't want to risk further delays on this PR and would rather isolate the change in a follow up if you're ok.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok, as discussed I've included above change, see 63fce46

}
}
}
Expand Down
2 changes: 1 addition & 1 deletion build-tools-internal/version.properties
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ elasticsearch = 9.2.0
lucene = 10.2.2

bundled_jdk_vendor = openjdk
bundled_jdk = 24.0.2+12@fdc5d0102fe0414db21410ad5834341f
bundled_jdk = 25+36@bd75d5f9689641da8e1daabeccb5528b
# optional dependencies
spatial4j = 0.7
jts = 1.15.0
Expand Down
3 changes: 2 additions & 1 deletion client/rest/src/test/java/org/elasticsearch/client/RestClientTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@

import static java.util.Collections.singletonList;
import static org.hamcrest.Matchers.instanceOf;
import static org.hamcrest.Matchers.startsWith;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertSame;
Expand Down Expand Up @@ -105,7 +106,7 @@ public void onSuccess(Response response) {
public void onFailure(Exception exception) {
try {
assertThat(exception, instanceOf(IllegalArgumentException.class));
assertEquals("Expected scheme name at index 0: ::http:///", exception.getMessage());
assertThat(exception.getMessage(), startsWith(("Expected scheme name")));
} finally {
latch.countDown();
}
Expand Down
5 changes: 5 additions & 0 deletions x-pack/plugin/esql/build.gradle
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,11 @@ interface Injected {
}

tasks.named("test").configure {
// prevent empty stacktraces on JDK 25 for intrinsic Math.*Exact invocations
// https://bugs.openjdk.org/browse/JDK-8367990
// https://github.com/elastic/elasticsearch/issues/135009
jvmArgs '-XX:-OmitStackTraceInFastThrow'

def injected = project.objects.newInstance(Injected)
// Define the folder to delete and recreate
def tempDir = file("build/testrun/test/temp/esql")
Expand Down
3 changes: 2 additions & 1 deletion ...est/java/org/elasticsearch/xpack/inference/services/custom/CustomRequestManagerTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@

import static org.elasticsearch.xpack.inference.Utils.inferenceUtilityExecutors;
import static org.hamcrest.Matchers.is;
import static org.hamcrest.Matchers.startsWith;
import static org.mockito.Mockito.mock;

public class CustomRequestManagerTests extends ESTestCase {
Expand Down Expand Up @@ -81,6 +82,6 @@ public void testCreateRequest_ThrowsException_ForInvalidUrl() {
var exception = expectThrows(ElasticsearchStatusException.class, () -> listener.actionGet(TimeValue.timeValueSeconds(30)));

assertThat(exception.getMessage(), is("Failed to construct the custom service request"));
assertThat(exception.getCause().getMessage(), is("Failed to build URI, error: Illegal character in path at index 0: ^"));
assertThat(exception.getCause().getMessage(), startsWith("Failed to build URI, error: Illegal character in path"));
}
}
3 changes: 2 additions & 1 deletion ...st/java/org/elasticsearch/xpack/inference/services/custom/request/CustomRequestTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@

import static org.hamcrest.Matchers.instanceOf;
import static org.hamcrest.Matchers.is;
import static org.hamcrest.Matchers.startsWith;

public class CustomRequestTests extends ESTestCase {

Expand Down Expand Up @@ -380,7 +381,7 @@ public void testCreateRequest_ThrowsException_ForInvalidUrl() {
IllegalStateException.class,
() -> new CustomRequest(RerankParameters.of(new QueryAndDocsInputs("query string", List.of("abc", "123"))), model)
);
assertThat(exception.getMessage(), is("Failed to build URI, error: Illegal character in path at index 0: ^"));
assertThat(exception.getMessage(), startsWith("Failed to build URI, error: Illegal character in path"));
}

private static String convertToString(InputStream inputStream) throws IOException {
Expand Down
8 changes: 2 additions & 6 deletions ...lugin/sql/jdbc/src/test/java/org/elasticsearch/xpack/sql/jdbc/JdbcConfigurationTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
import org.elasticsearch.test.ESTestCase;
import org.elasticsearch.xpack.sql.client.SslConfig;
import org.elasticsearch.xpack.sql.client.SuppressForbidden;
import org.hamcrest.Matchers;

import java.net.URI;
import java.net.URISyntaxException;
Expand Down Expand Up @@ -43,12 +44,7 @@ private JdbcConfiguration ci(String url) throws SQLException {

public void testInvalidUrl() {
JdbcSQLException e = expectThrows(JdbcSQLException.class, () -> ci("jdbc:es://localhost9200/?ssl=#5#"));
assertEquals(
"Invalid URL: Invalid connection configuration: Illegal character in fragment at index 28: "
+ "http://localhost9200/?ssl=#5#; format should be "
+ "[jdbc:[es|elasticsearch]://[[http|https]://]?[host[:port]]?/[prefix]?[\\?[option=value]&]*]",
e.getMessage()
);
assertThat(e.getMessage(), Matchers.startsWith("Invalid URL: Invalid connection configuration: Illegal character in fragment"));
}

public void testJustThePrefix() throws Exception {
Expand Down
8 changes: 5 additions & 3 deletions ...plugin/sql/sql-client/src/test/java/org/elasticsearch/xpack/sql/client/UriUtilsTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
import static org.elasticsearch.xpack.sql.client.UriUtils.appendSegmentToPath;
import static org.elasticsearch.xpack.sql.client.UriUtils.parseURI;
import static org.elasticsearch.xpack.sql.client.UriUtils.removeQuery;
import static org.hamcrest.Matchers.matchesPattern;

public class UriUtilsTests extends ESTestCase {

Expand Down Expand Up @@ -89,9 +90,10 @@ public void testUnsupportedProtocol() throws Exception {
}

public void testMalformedWhiteSpace() throws Exception {
assertEquals(
"Invalid connection configuration: Illegal character in authority at index 7: http:// ",
expectThrows(IllegalArgumentException.class, () -> parseURI(" ", DEFAULT_URI)).getMessage()
assertThat(
expectThrows(IllegalArgumentException.class, () -> parseURI(" ", DEFAULT_URI)).getMessage(),
// Use a lenient regex here since later JDKs trim exception message whitespace
matchesPattern("^Invalid connection configuration: Illegal character in authority at index 7: http:// ?")
);
}

Expand Down