Support SAN/dnsName for restricted trust

[jakelandis]

This commit extends the TLS restricted trust model to allow reading from
alternative fields from the X509 certificate. Prior to this commit the only
supported (hard coded) value that could be used with restricted trust
is the SAN/otherName/CN value. This commit introduces support to read
from other fields from the X509 certificate. This commit also introduces
support to read from SAN/dnsName if configured. Any fields read from the
certificate will be used to match against the restricted trust file and if any
of the values match to the restricted trust file, then restricted trust is allowed.
Only if none of the values match then the restricted trust denied.

SAN/otherName/CN is the default, and SAN/dnsName can be used in addition
or in place of SAN/otherName/CN. The possible configuration values are:
*.trust_restrictions.x509_fields: ["subjectAltName.otherName.commonName", "subjectAltName.dnsName"]

To help support testing, all of the existing certificates have been updated
to include a SAN/dnsName that matches the SAN/otherName/CN. This
allows the tests to randomize which field(s) are used to match for restricted trust.
This also has the side effect of making this commit larger than expected in
terms of lines of change. A readme has been included with copy-able commands
to recreate the certificates as needed.

Additionally, a CCS REST test has been introduced that uses the restricted trust.
To support this new CCS REST test the private keys for the test certificates are also
included in this commit as well as the gradle configuration needed to share those
certificates across projects.

[elasticsearchmachine]

Pinging @elastic/es-security (Team:Security)

[elasticsearchmachine]

Hi @jakelandis, I've created a changelog YAML for you.

[jakelandis]

Here is the readme to create these ... it is kinda buried in the noise.

[tvernum]

LGTM, with a couple of things we can follow up.

[tvernum]

Suggested change

	Set<String> otherNames = getSubjectAlternativeNames(certificate).stream()
	Set<String> otherNames = sans.stream()

[tvernum]

Suggested change

	List.of("subjectAltName.otherName.commonName"),
	List.of(RestrictedTrustConfig.SAN_OTHER_COMMON),

?

[tvernum]

The existing SSL config code mostly uses enums for these sorts of settings (e.g. clientAuth, verificationMode). Is it intentional that this would be different?

[justincr-elastic]

The dotted names make sense to me, just in case we get requests to parse CN from other parts of a cert in the future. For example:

1. subjectName.commonName
2. issuerName.commonName
3. subjectAltName.otherName.commonName
4. issuerAltName.otherName.commonName
5. subjectAltName.directoryName
6. issuerAltName.directoryName

[jakelandis]

The existing SSL config code mostly uses enums for these sorts of settings (e.g. clientAuth, verificationMode). Is it intentional that this would be different?

The settings infrastructure does not allows you to define a default that is not a list of strings. The generic listSettings only support a Setting<List<T>> fallbackSetting, which is very different from a List<T> defaultList (which is what we would need). I see there is support in the SslConfigurationLoader via the resolveListSetting abstraction. However, I found that abstraction very confusing since IMO it breaks the contract of how to define settings and where the defaults are derived. Rather than deriving the default from the setting itself , it is derived explicitly at run time via a specialized method and the default is defined in business class not the setting itself. The setting itself must be defined to have a default value of null which is arguably wrong since it violates the settings contract. Since the default value must be defined as null (when it is not null), you can not simply pass around the setting and expect to get back the correct default. Further, that abstraction is only in 8.x which would make the backporting even more difficult. I personally found adding the setting to be by far the most complicated and frustrating piece of this entire change mostly due to the abstraction and mismatched expectations of how you define a default and read a default value (it uniquely different from how any other settings work). I chose to better honor the setting contract and define the default as the default in the setting. Enums are nice but we need better support from the setting infrastructure to model the default as a list of enums (I could model the values as an enum as a wrapper around the strings, but that is kinda pointless). Long term, I would like to revisit this abstractions and make the other settings that default as non strings better support defaults as part of the setting definition itself.

[tvernum]

The code in ssl-config is complicated by the fact that libs can't depend on Setting (the Setting class is in core, and we can't have a dependency from ssl-config back to core).
So all the "Settings" in ssl-config have to be implemented in a very manual way which includes having to handle default values with explicit code instead of a default value. Technically it would have been possible to workaround it in different ways (essentially by duplicating half of theSetting class into ssl-config) but we made the choice to just handle null/empty values in SslConfigurationLoader. None of that has anything to do with enums - the code works the same way for string and enums, it's just a consequence of the fact that Setting & Settings have not been extracted into a standalone library.

Setting always treats default values as String so that things like _cluster/settings?include_defaults=true can work (which it doesn't for SSL settings because we can't use Setting, but the SSL settings are filtered anyway, so it wouldn't show anything even if we did register a default). The idea is that every default value for a setting can be represented in the same format that an admin would use to define that setting. So, even if the setting is an integer (see intSetting) the default value is a String which will then be parsed back into an Integer when read.

That doesn't prevent using an enum for a Setting type - see for example LdapUserSearchSessionFactorySettings.SEARCH_SCOPE or LicenseService.ALLOWED_LICENSE_TYPES_SETTING, it just means that the default values need to be converted into a String (or List<String>).
The benefit is that all the code that checks whether a particular x509_field is configured deals with real enum objects rather than strings pretending to be enums - all the case conversion / parsing code lives in the Setting and anything that uses the Setting value can do a simple contains (or for single-value settings, switch).

I'm not especially fussed about whether we change it here, but I do think that it is an overall benefit if Setting objects that have an enumerable list of allowed strings are implemented using real enum objects.
Perhaps we should add enumSetting and enumListSetting to Setting to make it easier to model it that way.

[jakelandis]

I have updated the code to better conform to the existing settings and use an enumeration. I had to exposed the default value as a final static public variable used in both the buisness class and the setting itself which alieviates my concerns about the differences in the default w.r.t. to the contract. (FWIW, the settings code requires a non-null default value for list settings). Change is over at #91983.

[tvernum]

I don't think we have any tests that verify that (for example) the SAN_DNS names aren't used if the field is set to SAN_OTHER_COMMON (the default.

That is, with a cert that was generated from:

 - ip: 50.100.150.200 - dns: search.example.com - cn: instance03.cluster02.elasticsearch 

We should accept the cert with either:

• trust.subject_name=*.cluster02.elasticsearch, x509_fields=[ "subjectAltName.otherName.commonName" ] (that is, the default value for x509_fields)
• trust.subject_name=*.example.com, x509_fields=[ "subjectAltName.dnsName" ]

But none of these:

• trust.subject_name=*.cluster02.elasticsearch, x509_fields=[ "subjectAltName.dnsName" ]
• trust.subject_name=*.example.com, x509_fields=[ "subjectAltName.otherName.commonName" ]
• trust.subject_name=*.150.200, x509_fields=[ "subjectAltName.otherName.commonName", "subjectAltName.dnsName" ]

I can see that the code will work correctly in these cases, but I can't see tests for it.

[jakelandis]

The randomly picked x509_fields here should provide coverage that SAN_DNS names aren't used if the field is set to SAN_OTHER_COMMON (and vice-versa). Since values are identical on the certificate, there is a chance that we could be reading from the wrong value if something else is buggy (but the other tests should cover that scenario). However it's pretty easy to add an explicit test for this case and will do so on a follow up PR.

[tvernum]

@elasticmachine run elasticsearch-ci/bwc please

Caused by: org.gradle.internal.resolve.ArtifactResolveException: Could not download ml-cpp-8.6.0-SNAPSHOT-deps.zip (org.elasticsearch.ml:ml-cpp:8.6.0-SNAPSHOT

[tvernum]

@elasticmachine run elasticsearch-ci/bwc another time please.

[justincr-elastic]

Missing dnsName wildcard test coverage? Examples:

1. dnsName="*.localdomain" with trust_restrictions=[ "*.localdomain" ]
2. dnsName="*.example.com" with trust_restrictions=[ "*.com" ]
3. dnsName="*.example.com" with trust_restrictions=[ "*.example.*" ]

[justincr-elastic]

Missing dnsName ACE encoding test for I18N mentioned in RFC 5280 Section 7.2.

 IA5String is limited to the set of ASCII characters. To accommodate internationalized domain names in the current structure, conforming implementations MUST convert internationalized domain names to the ASCII Compatible Encoding (ACE) format as specified in Section 4 of RFC 3490 before storage in the dNSName field. 

If design intent is to defer I18N dnsName support, maybe add a test to verify ACE encoded values are rejected or ignored.

[justincr-elastic]

Is design intent for trust_restrictions to do greedy matching or lazy matching?

SAN dnsName matching is lazy (i.e. one subdomain) per HTTP/TLS RFC 2818 Section 3.1.

 Matching is performed using the matching rules specified by RFC2459. If more than one identity of a given type is present in the certificate (e.g., more than one dNSName name, a match in any one of the set is considered acceptable.) Names may contain the wildcard character * which is considered to match any single domain name component or component fragment. E.g., *.a.com matches foo.a.com but not bar.foo.a.com. f*.com matches foo.com but not bar.com. 

It looks like SAN dnsName logic reuses same code as SAN otherName. I can't tell if SAN otherName design intent was to do lazy or greedy matching based on Elastic Cloud doc examples. Would it be a good idea to add some more tests to make design intent more clear?

[jakelandis]

The match aligns with our other usage of how match strings. We support wildcard expressions and lucene regex's. Both are string matches, not aware of domain names. The docs need to be updated (they need it before and after this PR) and will be handled in a different PR (likely in a different repository).

[jakelandis]

The wild test coverage is provided by the existing test methods (specifically here). The certificates were updated to include the same value for both commonName and dnsName so that we can randomize which one we use for the match.

[justincr-elastic]

LGTM. Missing dnsName wildcard test coverage?

[jakelandis]

Follow up PR created : #91983