Skip to content

Commit

Permalink
8284694: Avoid evaluating SSLAlgorithmConstraints twice
Browse files Browse the repository at this point in the history 
Reviewed-by: redestad, xuelei, coffeys
  • Loading branch information
@djelinski
djelinski committed Apr 20, 2022
1 parent cb16e41 commit d8446b4
Show file tree
Hide file tree
Showing 7 changed files with 426 additions and 42 deletions.
4 changes: 2 additions & 2 deletions src/java.base/share/classes/sun/security/ssl/HandshakeContext.java
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2018, 2021, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2018, 2022, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
Expand Down Expand Up @@ -164,7 +164,7 @@ protected HandshakeContext(SSLContextImpl sslContext,
this.conContext = conContext;
this.sslConfig = (SSLConfiguration)conContext.sslConfig.clone();

this.algorithmConstraints = new SSLAlgorithmConstraints(
this.algorithmConstraints = SSLAlgorithmConstraints.wrap(
sslConfig.userSpecifiedAlgorithmConstraints);
this.activeProtocols = getActiveProtocols(sslConfig.enabledProtocols,
sslConfig.enabledCipherSuites, algorithmConstraints);
Expand Down
100 changes: 76 additions & 24 deletions src/java.base/share/classes/sun/security/ssl/SSLAlgorithmConstraints.java
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2010, 2018, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2010, 2022, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
Expand Down Expand Up @@ -57,46 +57,98 @@ final class SSLAlgorithmConstraints implements AlgorithmConstraints {

// the default algorithm constraints
static final AlgorithmConstraints DEFAULT =
new SSLAlgorithmConstraints(null);
new SSLAlgorithmConstraints(null, true);

// the default SSL only algorithm constraints
static final AlgorithmConstraints DEFAULT_SSL_ONLY =
new SSLAlgorithmConstraints((SSLSocket)null, false);
new SSLAlgorithmConstraints(null, false);

SSLAlgorithmConstraints(AlgorithmConstraints userSpecifiedConstraints) {
this.userSpecifiedConstraints = userSpecifiedConstraints;
this.peerSpecifiedConstraints = null;
this.enabledX509DisabledAlgConstraints = true;
private SSLAlgorithmConstraints(AlgorithmConstraints userSpecifiedConstraints,
boolean enabledX509DisabledAlgConstraints) {
this(userSpecifiedConstraints, null, enabledX509DisabledAlgConstraints);
}

SSLAlgorithmConstraints(SSLSocket socket,
private SSLAlgorithmConstraints(
AlgorithmConstraints userSpecifiedConstraints,
SupportedSignatureAlgorithmConstraints peerSpecifiedConstraints,
boolean withDefaultCertPathConstraints) {
this.userSpecifiedConstraints = getUserSpecifiedConstraints(socket);
this.peerSpecifiedConstraints = null;
this.userSpecifiedConstraints = userSpecifiedConstraints;
this.peerSpecifiedConstraints = peerSpecifiedConstraints;
this.enabledX509DisabledAlgConstraints = withDefaultCertPathConstraints;
}

SSLAlgorithmConstraints(SSLEngine engine,
/**
* Returns a SSLAlgorithmConstraints instance that checks the provided
* {@code userSpecifiedConstraints} in addition to standard checks.
* Returns a singleton instance if parameter is null or DEFAULT.
* @param userSpecifiedConstraints additional constraints to check
* @return a SSLAlgorithmConstraints instance
*/
static AlgorithmConstraints wrap(AlgorithmConstraints userSpecifiedConstraints) {
return wrap(userSpecifiedConstraints, true);
}

private static AlgorithmConstraints wrap(
AlgorithmConstraints userSpecifiedConstraints,
boolean withDefaultCertPathConstraints) {
this.userSpecifiedConstraints = getUserSpecifiedConstraints(engine);
this.peerSpecifiedConstraints = null;
this.enabledX509DisabledAlgConstraints = withDefaultCertPathConstraints;
if (nullIfDefault(userSpecifiedConstraints) == null) {
return withDefaultCertPathConstraints ? DEFAULT : DEFAULT_SSL_ONLY;
}
return new SSLAlgorithmConstraints(userSpecifiedConstraints,
withDefaultCertPathConstraints);
}

/**
* Returns a SSLAlgorithmConstraints instance that checks the constraints
* configured for the given {@code socket} in addition to standard checks.
* Returns a singleton instance if the constraints are null or DEFAULT.
* @param socket socket with configured constraints
* @return a SSLAlgorithmConstraints instance
*/
static AlgorithmConstraints forSocket(SSLSocket socket,
boolean withDefaultCertPathConstraints) {
AlgorithmConstraints userSpecifiedConstraints =
getUserSpecifiedConstraints(socket);
return wrap(userSpecifiedConstraints, withDefaultCertPathConstraints);
}

SSLAlgorithmConstraints(SSLSocket socket, String[] supportedAlgorithms,
static SSLAlgorithmConstraints forSocket(
SSLSocket socket,
String[] supportedAlgorithms,
boolean withDefaultCertPathConstraints) {
this.userSpecifiedConstraints = getUserSpecifiedConstraints(socket);
this.peerSpecifiedConstraints =
new SupportedSignatureAlgorithmConstraints(supportedAlgorithms);
this.enabledX509DisabledAlgConstraints = withDefaultCertPathConstraints;
return new SSLAlgorithmConstraints(
nullIfDefault(getUserSpecifiedConstraints(socket)),
new SupportedSignatureAlgorithmConstraints(supportedAlgorithms),
withDefaultCertPathConstraints);
}

/**
* Returns a SSLAlgorithmConstraints instance that checks the constraints
* configured for the given {@code engine} in addition to standard checks.
* Returns a singleton instance if the constraints are null or DEFAULT.
* @param engine engine with configured constraints
* @return a SSLAlgorithmConstraints instance
*/
static AlgorithmConstraints forEngine(SSLEngine engine,
boolean withDefaultCertPathConstraints) {
AlgorithmConstraints userSpecifiedConstraints =
getUserSpecifiedConstraints(engine);
return wrap(userSpecifiedConstraints, withDefaultCertPathConstraints);
}

SSLAlgorithmConstraints(SSLEngine engine, String[] supportedAlgorithms,
static SSLAlgorithmConstraints forEngine(
SSLEngine engine,
String[] supportedAlgorithms,
boolean withDefaultCertPathConstraints) {
this.userSpecifiedConstraints = getUserSpecifiedConstraints(engine);
this.peerSpecifiedConstraints =
new SupportedSignatureAlgorithmConstraints(supportedAlgorithms);
this.enabledX509DisabledAlgConstraints = withDefaultCertPathConstraints;
return new SSLAlgorithmConstraints(
nullIfDefault(getUserSpecifiedConstraints(engine)),
new SupportedSignatureAlgorithmConstraints(supportedAlgorithms),
withDefaultCertPathConstraints);
}

private static AlgorithmConstraints nullIfDefault(
AlgorithmConstraints constraints) {
return constraints == DEFAULT ? null : constraints;
}

private static AlgorithmConstraints getUserSpecifiedConstraints(
Expand Down
12 changes: 6 additions & 6 deletions src/java.base/share/classes/sun/security/ssl/SSLContextImpl.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1485,14 +1485,14 @@ private void checkAdditionalTrust(X509Certificate[] chain,
String[] peerSupportedSignAlgs =
extSession.getLocalSupportedSignatureAlgorithms();

constraints = new SSLAlgorithmConstraints(
constraints = SSLAlgorithmConstraints.forSocket(
sslSocket, peerSupportedSignAlgs, true);
} else {
constraints =
new SSLAlgorithmConstraints(sslSocket, true);
SSLAlgorithmConstraints.forSocket(sslSocket, true);
}
} else {
constraints = new SSLAlgorithmConstraints(sslSocket, true);
constraints = SSLAlgorithmConstraints.forSocket(sslSocket, true);
}

checkAlgorithmConstraints(chain, constraints, checkClientTrusted);
Expand Down Expand Up @@ -1525,14 +1525,14 @@ private void checkAdditionalTrust(X509Certificate[] chain,
String[] peerSupportedSignAlgs =
extSession.getLocalSupportedSignatureAlgorithms();

constraints = new SSLAlgorithmConstraints(
constraints = SSLAlgorithmConstraints.forEngine(
engine, peerSupportedSignAlgs, true);
} else {
constraints =
new SSLAlgorithmConstraints(engine, true);
SSLAlgorithmConstraints.forEngine(engine, true);
}
} else {
constraints = new SSLAlgorithmConstraints(engine, true);
constraints = SSLAlgorithmConstraints.forEngine(engine, true);
}

checkAlgorithmConstraints(chain, constraints, checkClientTrusted);
Expand Down
12 changes: 6 additions & 6 deletions src/java.base/share/classes/sun/security/ssl/X509KeyManagerImpl.java
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2004, 2021, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2004, 2022, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
Expand Down Expand Up @@ -199,15 +199,15 @@ private AlgorithmConstraints getAlgorithmConstraints(Socket socket) {
extSession.getPeerSupportedSignatureAlgorithms();
}

return new SSLAlgorithmConstraints(
return SSLAlgorithmConstraints.forSocket(
sslSocket, peerSupportedSignAlgs, true);
}
}

return new SSLAlgorithmConstraints(sslSocket, true);
return SSLAlgorithmConstraints.forSocket(sslSocket, true);
}

return new SSLAlgorithmConstraints((SSLSocket)null, true);
return SSLAlgorithmConstraints.DEFAULT;
}

// Gets algorithm constraints of the engine.
Expand All @@ -225,13 +225,13 @@ private AlgorithmConstraints getAlgorithmConstraints(SSLEngine engine) {
extSession.getPeerSupportedSignatureAlgorithms();
}

return new SSLAlgorithmConstraints(
return SSLAlgorithmConstraints.forEngine(
engine, peerSupportedSignAlgs, true);
}
}
}

return new SSLAlgorithmConstraints(engine, true);
return SSLAlgorithmConstraints.forEngine(engine, true);
}

// we construct the alias we return to JSSE as seen in the code below
Expand Down
8 changes: 4 additions & 4 deletions src/java.base/share/classes/sun/security/ssl/X509TrustManagerImpl.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -216,10 +216,10 @@ private void checkTrusted(X509Certificate[] chain,
String[] localSupportedSignAlgs =
extSession.getLocalSupportedSignatureAlgorithms();

constraints = new SSLAlgorithmConstraints(
constraints = SSLAlgorithmConstraints.forSocket(
sslSocket, localSupportedSignAlgs, false);
} else {
constraints = new SSLAlgorithmConstraints(sslSocket, false);
constraints = SSLAlgorithmConstraints.forSocket(sslSocket, false);
}

// Grab any stapled OCSP responses for use in validation
Expand Down Expand Up @@ -270,10 +270,10 @@ private void checkTrusted(X509Certificate[] chain,
String[] localSupportedSignAlgs =
extSession.getLocalSupportedSignatureAlgorithms();

constraints = new SSLAlgorithmConstraints(
constraints = SSLAlgorithmConstraints.forEngine(
engine, localSupportedSignAlgs, false);
} else {
constraints = new SSLAlgorithmConstraints(engine, false);
constraints = SSLAlgorithmConstraints.forEngine(engine, false);
}

// Grab any stapled OCSP responses for use in validation
Expand Down
Loading

3 comments on commit d8446b4

@openjdk-notifier
Copy link

Choose a reason for hiding this comment

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

Review

Issues

@GoeLin
Copy link
Member

@GoeLin GoeLin commented on d8446b4 Jun 8, 2022

Choose a reason for hiding this comment

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

/backport jdk17u-dev

@openjdk
Copy link

@openjdk openjdk bot commented on d8446b4 Jun 8, 2022

Choose a reason for hiding this comment

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

@GoeLin the backport was successfully created on the branch GoeLin-backport-d8446b4f in my personal fork of openjdk/jdk17u-dev. To create a pull request with this backport targeting openjdk/jdk17u-dev:master, just click the following link:

➡️ Create pull request

The title of the pull request is automatically filled in correctly and below you find a suggestion for the pull request body:

Hi all,

This pull request contains a backport of commit d8446b4f from the openjdk/jdk repository.

The commit being backported was authored by Daniel Jeliński on 20 Apr 2022 and was reviewed by Claes Redestad, Xue-Lei Andrew Fan and Sean Coffey.

Thanks!

If you need to update the source branch of the pull then run the following commands in a local clone of your personal fork of openjdk/jdk17u-dev:

$ git fetch https://github.com/openjdk-bots/jdk17u-dev GoeLin-backport-d8446b4f:GoeLin-backport-d8446b4f
$ git checkout GoeLin-backport-d8446b4f
# make changes
$ git add paths/to/changed/files
$ git commit --message 'Describe additional changes made'
$ git push https://github.com/openjdk-bots/jdk17u-dev GoeLin-backport-d8446b4f

Please sign in to comment.