On Fri, 8 Nov 2024 07:39:37 GMT, Lothar Kimmeringer <[email protected]> wrote:
>> src/java.base/share/classes/sun/security/util/AbstractAlgorithmConstraints.java
>> line 127:
>>
>>> 125: return patternCache.computeIfAbsent(
>>> 126: pattern,
>>> 127: p -> Pattern.compile(p.replace("*", ".*")))
>>
>> Do we care if one uses other regex matching characters as part of the
>> pattern input, e.g. should `TLS_[a-zA-Z0-9_]+` be a valid input that
>> disables some algorithms?
>>
>> Should the matching be case insensitive?
>
> I've asked myself the same thing and I think that - if that's not supposed to
> be allowed - the following should solve that:
>
>
> p -> Pattern.compile("^\\Q" + p.replace("*", "\\E.*\\Q") + "\\E$")
- I think we shouldn't care if someone wants to use other regex syntax
matching, maybe someone will find it useful. We just not going to document this
to avoid any confusion, most people will just use `*`. They should be able to
use other regex (with `*` in place of `.*`) as long as at least one `*` is
present and cipher suite starts with "TLS_". Filtering the pattern to disallow
this will result in one extra regex matching operation while we try to keep
things fast.
- About just ignoring other regex characters with `Pattern.compile("^\\Q" +
p.replace("*", "\\E.*\\Q") + "\\E$")`: I'm not sure if silently ignoring those
characters is what we want, we should either disallow them (throw an exception)
or allow those characters.
- I think the other design change we should consider is to use full regex
without any `*` replacement. Most people who would be setting those values
should be familiar with regex.
- Yet another option is to implement custom `*` matching method and not to use
regex at all.
- About case-insensitive matching: actually that was the original version of
this code, but other implementations treat cipher suites in case-sensitive
manner, so I modified it to be case-sensitive.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/21841#discussion_r1834533349
