markt-asf commented on PR #1098: URL: https://github.com/apache/tomcat/pull/1098#issuecomment-6627895866
My manual review didn't flag any concerns. I think this is a good fix to the issue reported on the users list. I a ran a Co-Pilot review of the PR. Of the things if reported, the following appear to be valid. 1. Most (all modern?) TLS clients do not accept ANY_EXTENDED_KEY_USAGE_OID. CAB forum does not allow it. Nor do they accept no EKU. I think the filtering can be simplified to must include SERVER_AUTH_EXTENDED_KEY_USAGE_OID. 2. `SSLUtilBase:426-434` A missing alias is treated the same way as a present but unsuitable certificate. A missing `tomcat` alias should not trigger a search for a `tomcat (1)` alias as that could be an entirely different key/cert pair. 3. FIPS doesn't wrap key managers so `SSLContext#getServedAlias()` could return the wrong value when FIPS is being used. This could cause the wrong cert to be loaded and/or monitored for expiration when FIPS is used. Not sure how easy it is to detect FIPS usage at this point. 4. Similar to 3. Need to document that these checks don't apply to FIPS configurations. 5. The [NewSunX509](https://github.com/openjdk/jdk/blob/master/src/java.base/share/classes/sun/security/ssl/X509KeyManagerImpl.java) key manager returns synthetic aliases of the form `<uid>.<keystore index>.<alias>` which the logging at `AbstractEndpoint.java:652–659` will report. This looks OK to me. I think it was flagged as it is another place where a FIPS configuration could report the wrong information. I think 1 and 2 are simple fixes. I'm not so sure about 3-5. We do have users that use FIPS so I this these do need to be addressed. -- 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]
