RFR: 8349583: Add mechanism to disable signature schemes based on their TLS scope [v19]

Artur Barashev abarashev at openjdk.org
Thu Mar 27 19:41:26 UTC 2025


On Thu, 27 Mar 2025 18:46:02 GMT, Sean Mullan <mullan at openjdk.org> wrote:

>> Artur Barashev has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Fix java.security syntax. Remove whitespace.
>
> test/jdk/sun/security/ssl/SignatureScheme/AbstractCheckSignatureSchemes.java line 77:
> 
>> 75:     }
>> 76: 
>> 77:     protected String getProtocol() {
> 
> I'd be more inclined to make this abstract and force subclasses to override it.

Will do. I also though about it but since the original `SigSchemePropOrdering` test had "TLSv1.2" I kept it as default.

> test/jdk/sun/security/ssl/SignatureScheme/DisableSignatureSchemePerScopeTLS12.java line 52:
> 
>> 50:     protected static final String DISABLED_CONSTRAINTS =
>> 51:             HANDSHAKE_DISABLED_SIG + " usage HandShakesignature, "
>> 52:             + CERTIFICATE_DISABLED_SIG + " usage certificateSignature";
> 
> Nit: s/certificateSignature/CertificateSignature/

Actually this is done on purpose to check case-insensitive matching, I use this approach in other tests as well.

> test/jdk/sun/security/ssl/SignatureScheme/DisableSignatureSchemePerScopeTLS13.java line 42:
> 
>> 40: 
>> 41: public class DisableSignatureSchemePerScopeTLS13
>> 42:         extends DisableSignatureSchemePerScopeTLS12 {
> 
> It's a little odd this extends *TLS12 - did you consider extending `AbstractCheckSignatureSchemes` or was that too complicated?

I've done it for code reusability, we'll have to copy/paste a few methods otherwise.

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/23681#discussion_r2017493320
PR Review Comment: https://git.openjdk.org/jdk/pull/23681#discussion_r2017495722
PR Review Comment: https://git.openjdk.org/jdk/pull/23681#discussion_r2017498123


More information about the security-dev mailing list