Skip to content

Commit

Permalink
8284694: Avoid evaluating SSLAlgorithmConstraints twice
Browse files Browse the repository at this point in the history
Reviewed-by: stuefe
Backport-of: d8446b4f60472b11e4cdaef97288fe143cca4511
  • Loading branch information
GoeLin committed Jun 29, 2022
1 parent 97178bf commit eb8789b
Show file tree
Hide file tree
Showing 7 changed files with 426 additions and 42 deletions.
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2018, 2019, 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 @@ -162,7 +162,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
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
Original file line number Diff line number Diff line change
Expand Up @@ -1554,14 +1554,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 @@ -1594,14 +1594,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
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2004, 2018, 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 @@ -197,15 +197,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 @@ -223,13 +223,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
Original file line number Diff line number Diff line change
Expand Up @@ -207,10 +207,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 @@ -261,10 +261,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

1 comment on commit eb8789b

@openjdk-notifier
Copy link

Choose a reason for hiding this comment

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

Please sign in to comment.