Only set the keymaterial once and correctly handle errors during keymaterial setting on the client-side as well (#10613)

Motivation:

We should stop as soon as we were able to set the key material on the server side as otherwise we may select keymaterial that "belongs" to a less prefered cipher. Beside this it also is just useless work.
We also need to propagate the exception when it happens during key material selection on the client side so openssl will produce the right alert.

Modifications:

- Stop once we were able to select a key material on the server side
- Ensure we not call choose*Alias more often then needed
- Propagate exceptions during selection of the keymaterial on the client side.

Result:

Less overhead and more correct behaviour
This commit is contained in:
Norman Maurer 2020-09-29 09:12:34 +02:00 committed by GitHub
parent bc00be7e98
commit 5a9256a183
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 20 additions and 21 deletions

View File

@ -23,10 +23,8 @@ import javax.security.auth.x500.X500Principal;
import java.security.PrivateKey;
import java.security.cert.X509Certificate;
import java.util.Arrays;
import java.util.Collections;
import java.util.HashMap;
import java.util.HashSet;
import java.util.LinkedHashSet;
import java.util.Map;
import java.util.Set;
@ -73,28 +71,23 @@ final class OpenSslKeyMaterialManager {
throw new SSLHandshakeException("Unable to find key material");
}
boolean matched = false;
// authMethods may contain duplicates but call chooseServerAlias(...) may be expensive. So let's ensure
// authMethods may contain duplicates or may result in the same type
// but call chooseServerAlias(...) may be expensive. So let's ensure
// we filter out duplicates.
Set<String> authMethodsSet = new LinkedHashSet<String>(authMethods.length);
Collections.addAll(authMethodsSet, authMethods);
Set<String> aliases = new HashSet<String>(authMethodsSet.size());
for (String authMethod : authMethodsSet) {
Set<String> typeSet = new HashSet<String>(KEY_TYPES.size());
for (String authMethod : authMethods) {
String type = KEY_TYPES.get(authMethod);
if (type != null) {
if (type != null && typeSet.add(type)) {
String alias = chooseServerAlias(engine, type);
if (alias != null && aliases.add(alias)) {
if (!setKeyMaterial(engine, alias)) {
return;
}
matched = true;
if (alias != null) {
// We found a match... let's set the key material and return.
setKeyMaterial(engine, alias);
return;
}
}
}
if (!matched) {
throw new SSLHandshakeException("Unable to find key material for auth method(s): "
+ Arrays.toString(authMethods));
}
throw new SSLHandshakeException("Unable to find key material for auth method(s): "
+ Arrays.toString(authMethods));
}
void setKeyMaterialClientSide(ReferenceCountedOpenSslEngine engine, String[] keyTypes,
@ -108,11 +101,14 @@ final class OpenSslKeyMaterialManager {
}
}
private boolean setKeyMaterial(ReferenceCountedOpenSslEngine engine, String alias) throws SSLException {
private void setKeyMaterial(ReferenceCountedOpenSslEngine engine, String alias) throws SSLException {
OpenSslKeyMaterial keyMaterial = null;
try {
keyMaterial = provider.chooseKeyMaterial(engine.alloc, alias);
return keyMaterial == null || engine.setKeyMaterial(keyMaterial);
if (keyMaterial == null) {
return;
}
engine.setKeyMaterial(keyMaterial);
} catch (SSLException e) {
throw e;
} catch (Exception e) {

View File

@ -292,8 +292,11 @@ public final class ReferenceCountedOpenSslClientContext extends ReferenceCounted
}
keyManagerHolder.setKeyMaterialClientSide(engine, keyTypes, issuers);
} catch (Throwable cause) {
logger.debug("request of key failed", cause);
engine.initHandshakeException(cause);
if (cause instanceof Exception) {
throw (Exception) cause;
}
throw new SSLException(cause);
}
}