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