RFR: 8353113: Peer supported certificate signature algorithms are not being checked with default SunX509 key manager [v4]

Sean Mullan mullan at openjdk.org
Thu Jun 5 17:47:54 UTC 2025


On Wed, 14 May 2025 18:16:13 GMT, Artur Barashev <abarashev at 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


More information about the net-dev mailing list