On Wed, 14 May 2025 18:16:13 GMT, Artur Barashev <abaras...@openjdk.org> wrote:
>> When the deafult SunX509KeyManagerImpl is being used we are in violation of >> TLSv1.3 RFC spec because we ignore peer supported certificate signatures >> sent to us in "signature_algorithms"/"signature_algorithms_cert" extensions: >> https://datatracker.ietf.org/doc/html/rfc8446#section-4.4.2.2 >> https://datatracker.ietf.org/doc/html/rfc8446#section-4.4.2.3 >> >> X509KeyManagerImpl on the other hand includes the algorithms sent by the >> peer in "signature_algorithms_cert" extension (or in "signature_algorithms" >> extension when "signature_algorithms_cert" extension isn't present) in the >> algorithm constraints being checked. > > Artur Barashev has updated the pull request incrementally with one additional > commit since the last revision: > > Make the test run on TLSv1.3 Some initial comments. src/java.base/share/classes/sun/security/ssl/SunX509KeyManagerImpl.java line 264: > 262: * the peer (if any). > 263: * > 264: * @since 1.5 I would remove the `@since 1.5` from these methods. It isn't relevant anymore since this is an internal class and that version is no longer supported. That version info is in the `X509ExtendedKeyManager` API which is sufficient. src/java.base/share/classes/sun/security/ssl/SunX509KeyManagerImpl.java line 395: > 393: if (SSLLogger.isOn && SSLLogger.isOn("keymanager")) { > 394: SSLLogger.fine("Ignore alias " + alias + > 395: ": certificate list does not conform to " + suggest saying "certificate chain" not "certificate list". test/lib/jdk/test/lib/security/CertificateBuilder.java line 244: > 242: * @throws IOException if an encoding error occurs. > 243: */ > 244: public CertificateBuilder addSubjectAltNameIPExt(List<String> > IPAddresses) variables usually start with lower case letter, so suggest `ipAddresses` ------------- PR Review: https://git.openjdk.org/jdk/pull/25016#pullrequestreview-2900744862 PR Review Comment: https://git.openjdk.org/jdk/pull/25016#discussion_r2129536065 PR Review Comment: https://git.openjdk.org/jdk/pull/25016#discussion_r2129556680 PR Review Comment: https://git.openjdk.org/jdk/pull/25016#discussion_r2129090967
