markt-asf commented on code in PR #1098:
URL: https://github.com/apache/tomcat/pull/1098#discussion_r4401972809
##########
java/org/apache/tomcat/util/net/jsse/JSSESSLContext.java:
##########
@@ -89,6 +90,22 @@ public X509Certificate[] getCertificateChain(String alias) {
return result;
}
+
+ @Override
+ public String getServedAlias(SSLHostConfigCertificate certificate) {
+ if (kms != null) {
+ for (KeyManager km : kms) {
+ if (km instanceof X509KeyManager x509KeyManager) {
+ String alias =
certificate.chooseServerAlias(x509KeyManager);
+ if (alias != null) {
+ return alias;
+ }
+ }
+ }
+ }
+ return SSLContext.super.getServedAlias(certificate);
+ }
+
Review Comment:
What about OpenSSL and Panama ? Do they need their own versions of this? Can
this method be pulled up to the super class?
##########
java/org/apache/tomcat/util/net/SSLHostConfigCertificate.java:
##########
@@ -244,6 +244,31 @@ public Type getType() {
}
+ /**
+ * Returns the alias the given key manager would choose to serve for this
certificate, or {@code null} if it has no
+ * suitable entry. When {@link #getType()} is {@link Type#UNDEFINED} every
key type is tried. This reflects the
+ * alias actually used in a handshake, which can differ from {@link
#getCertificateKeyAlias()} - for example after
+ * the SunMSCAPI Windows-MY numbered-duplicate resolution performed by
SSLUtilBase#getKeyManagers().
+ *
+ * @param keyManager the key manager that will serve this certificate
+ *
+ * @return the served alias, or {@code null} if the key manager has no
suitable entry
+ */
+ public String chooseServerAlias(X509KeyManager keyManager) {
+ Type[] candidateTypes = Type.UNDEFINED.equals(type) ? Type.values() :
new Type[] { type };
+ for (Type candidateType : candidateTypes) {
+ if (candidateType == Type.UNDEFINED) {
+ continue;
+ }
+ String alias =
keyManager.chooseServerAlias(candidateType.getKeyType(), null, null);
+ if (alias != null) {
+ return alias;
+ }
+ }
+ return null;
+ }
+
Review Comment:
Similar code in OpenSSLContext.findAlias() (and the panama version).
Refactor to reduce duplication?
##########
java/org/apache/tomcat/util/net/AbstractEndpoint.java:
##########
@@ -647,13 +648,19 @@ protected void logCertificate(SSLHostConfigCertificate
certificate) {
getLogCertificate().info(sm.getString("endpoint.tls.info", getName(),
sslHostConfig.getHostName(),
certificate.getType(), certificateInfo, trustStoreSource));
- SSLContext sslContext = certificate.getSslContext();
- if (getLogCertificate().isDebugEnabled() && sslContext != null) {
- String alias = certificate.getCertificateKeyAlias();
- if (alias == null) {
- alias = SSLUtilBase.DEFAULT_KEY_ALIAS;
+ // When a configured alias was resolved to a different alias that is
actually served (e.g. a SunMSCAPI
Review Comment:
that -> than
##########
test/org/apache/tomcat/util/net/TesterKeystoreGenerator.java:
##########
@@ -41,6 +43,8 @@
public final class TesterKeystoreGenerator {
Review Comment:
We should try and move away from this class and pre-generate all the test
keys/cert we need. See script in PMC private svn repo.
##########
java/org/apache/tomcat/util/net/SSLUtilBase.java:
##########
@@ -450,6 +482,99 @@ public KeyManager[] getKeyManagers() throws Exception {
}
+ /**
+ * Searches a SunMSCAPI Windows-MY key store for a key entry whose
certificate is valid for TLS server
+ * authentication, among the numbered duplicates of {@code baseAlias}.
When several certificates share a Windows
+ * "friendly name", that provider appends {@code " (1)"}, {@code " (2)"},
etc. to give them unique aliases, as the
+ * key store API requires.
+ *
+ * @param ks the SunMSCAPI Windows-MY key store to search
+ * @param baseAlias the configured alias. Not itself re-checked: the
caller has already determined that its
+ * certificate is not valid for TLS server
authentication.
+ *
+ * @return the alias of the first duplicate (in increasing numeric order
starting at 1) whose certificate is
+ * valid for TLS server authentication, or {@code null} if
there is no such duplicate
+ *
+ * @throws KeyStoreException if the key store has not been initialized
+ */
+ private static String findServerAuthAliasInWindowsMYFamily(KeyStore ks,
String baseAlias) throws KeyStoreException {
+ // Numbered duplicates cannot outnumber the aliases in the store, so
that count bounds the scan.
+ int limit = Collections.list(ks.aliases()).size();
+ for (int suffix = 1; suffix <= limit; suffix++) {
+ String candidateAlias = baseAlias + " (" + suffix + ")";
+ Certificate candidateCert = ks.getCertificate(candidateAlias);
+ if (candidateCert == null) {
+ // A missing number is a gap, not the end of the sequence:
keep scanning so a valid duplicate beyond it
+ // is still found.
Review Comment:
Is this the case? I'd expect the JRE to number them sequentially. Also
`limit` does not allow for any gaps. It looks like this could be simplified a
little.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
