-
Couldn't load subscription status.
- Fork 337
[FEATURE] Add support for X509v3 extensions for authentication #5701
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
laminelam
wants to merge
2
commits into
opensearch-project:main
Choose a base branch
from
laminelam:feature/cert_auth_san_1012
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+356
−39
Open
Changes from all commits
Commits
Show all changes
2 commits
Select commit
Hold shift + click to select a range
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -27,10 +27,19 @@ | |
| package org.opensearch.security.http; | ||
|
|
||
| import java.nio.file.Path; | ||
| import java.security.cert.CertificateParsingException; | ||
| import java.security.cert.X509Certificate; | ||
| import java.util.ArrayList; | ||
| import java.util.Collection; | ||
| import java.util.Collections; | ||
| import java.util.EnumSet; | ||
| import java.util.List; | ||
| import java.util.Map; | ||
| import java.util.Objects; | ||
| import java.util.Optional; | ||
| import java.util.regex.Matcher; | ||
| import java.util.regex.Pattern; | ||
| import java.util.stream.Collectors; | ||
| import javax.naming.InvalidNameException; | ||
| import javax.naming.ldap.LdapName; | ||
| import javax.naming.ldap.Rdn; | ||
|
|
@@ -54,66 +63,126 @@ public class HTTPClientCertAuthenticator implements HTTPAuthenticator { | |
| public static final String OPENDISTRO_SECURITY_SSL_SKIP_USERS = "skip_users"; | ||
| protected final Settings settings; | ||
| private final WildcardMatcher skipUsersMatcher; | ||
| private final ParsedAttribute parsedUsernameAttr; | ||
| private final ParsedAttribute parsedRolesAttr; | ||
|
|
||
| public HTTPClientCertAuthenticator(final Settings settings, final Path configPath) { | ||
| this.settings = settings; | ||
| this.skipUsersMatcher = WildcardMatcher.from(settings.getAsList(OPENDISTRO_SECURITY_SSL_SKIP_USERS)); | ||
| private enum AttributeType { | ||
| DN, | ||
| SAN | ||
| } | ||
|
|
||
| @Override | ||
| public AuthCredentials extractCredentials(final SecurityRequest request, final ThreadContext threadContext) { | ||
| private record ParsedSAN(int type, Pattern pattern) { | ||
| } | ||
|
|
||
| final String principal = threadContext.getTransient(ConfigConstants.OPENDISTRO_SECURITY_SSL_PRINCIPAL); | ||
| private record ParsedAttribute(AttributeType type, String dnAttr, ParsedSAN san) { | ||
|
|
||
| static ParsedAttribute dn(String attr) { | ||
| return new ParsedAttribute(AttributeType.DN, attr, null); | ||
| } | ||
|
|
||
| static ParsedAttribute san(ParsedSAN san) { | ||
| return new ParsedAttribute(AttributeType.SAN, null, san); | ||
| } | ||
| } | ||
|
|
||
| private ParsedAttribute parseAttributeSetting(String raw) { | ||
| if (Strings.isNullOrEmpty(raw)) return null; // “not configured” | ||
|
|
||
| if (!Strings.isNullOrEmpty(principal)) { | ||
| // Accept forms: | ||
| // "cn" -> DN:cn | ||
| // "dn:cn" -> DN:cn | ||
| // "san:EMAIL" -> SAN type EMAIL, no regex (match all of that SAN) | ||
| // "san:EMAIL:re" -> SAN type EMAIL, regex | ||
| final String s = raw.trim(); | ||
|
|
||
| final String usernameAttribute = settings.get("username_attribute"); | ||
| final String rolesAttribute = settings.get("roles_attribute"); | ||
| if (s.regionMatches(true, 0, "san:", 0, 4)) { | ||
| final String rest = s.substring(4); // after "san:" | ||
| final int firstColon = rest.indexOf(':'); | ||
| final String sanField = (firstColon >= 0) ? rest.substring(0, firstColon) : rest; | ||
| final String regex = (firstColon >= 0) ? rest.substring(firstColon + 1) : null; | ||
|
|
||
| if (skipUsersMatcher.test(principal)) { | ||
| log.debug("Skipped user client cert authentication of user {} as its in skip_users list ", principal); | ||
| Integer sanTypeInt; | ||
| try { | ||
| sanTypeInt = SANType.valueOf(sanField.toUpperCase(java.util.Locale.ROOT)).getValue(); | ||
| } catch (IllegalArgumentException e) { | ||
| log.warn("Unsupported SAN type '{}' in attribute '{}'", sanField, raw); | ||
| return null; | ||
| } | ||
|
|
||
| try { | ||
| final LdapName rfc2253dn = new LdapName(principal); | ||
| String username = principal.trim(); | ||
| String[] backendRoles = null; | ||
|
|
||
| if (usernameAttribute != null && usernameAttribute.length() > 0) { | ||
| final List<String> usernames = getDnAttribute(rfc2253dn, usernameAttribute); | ||
| if (usernames.isEmpty() == false) { | ||
| username = usernames.get(0); | ||
| } | ||
| Pattern pattern = null; | ||
| if (!Strings.isNullOrEmpty(regex)) { | ||
| try { | ||
| pattern = Pattern.compile(regex, Pattern.CASE_INSENSITIVE | Pattern.UNICODE_CASE); | ||
| } catch (Exception e) { | ||
| log.warn("Invalid regex in attribute '{}': {}", raw, e.toString()); | ||
| return null; | ||
| } | ||
| } | ||
|
|
||
| if (rolesAttribute != null && rolesAttribute.length() > 0) { | ||
| final List<String> roles = getDnAttribute(rfc2253dn, rolesAttribute); | ||
| if (roles.isEmpty() == false) { | ||
| backendRoles = roles.toArray(new String[0]); | ||
| } | ||
| } | ||
| return ParsedAttribute.san(new ParsedSAN(sanTypeInt, pattern)); | ||
| } | ||
|
|
||
| return new AuthCredentials(username, backendRoles).markComplete(); | ||
| } catch (InvalidNameException e) { | ||
| log.error("Client cert had no properly formed DN (was: {})", principal); | ||
| return null; | ||
| } | ||
| // DN form: either "dn:cn" or just "cn" | ||
| final String dnAttr = s.regionMatches(true, 0, "dn:", 0, 3) ? s.substring(3) : s; | ||
| return ParsedAttribute.dn(dnAttr); | ||
| } | ||
|
|
||
| } else { | ||
| public HTTPClientCertAuthenticator(final Settings settings, final Path configPath) { | ||
| this.settings = settings; | ||
| this.skipUsersMatcher = WildcardMatcher.from(settings.getAsList(OPENDISTRO_SECURITY_SSL_SKIP_USERS)); | ||
| this.parsedUsernameAttr = parseAttributeSetting(settings.get("username_attribute")); | ||
| this.parsedRolesAttr = parseAttributeSetting(settings.get("roles_attribute")); | ||
| } | ||
|
|
||
| @Override | ||
| public AuthCredentials extractCredentials(final SecurityRequest request, final ThreadContext threadContext) { | ||
|
|
||
| final String principal = threadContext.getTransient(ConfigConstants.OPENDISTRO_SECURITY_SSL_PRINCIPAL); | ||
|
|
||
| if (Strings.isNullOrEmpty(principal)) { | ||
| log.trace("No CLIENT CERT, send 401"); | ||
| return null; | ||
| } | ||
| if (skipUsersMatcher.test(principal)) { | ||
| log.debug("Skipped user client cert authentication of user {} as its in skip_users list ", principal); | ||
| return null; | ||
| } | ||
|
|
||
| try { | ||
| final String username = extractUsername(threadContext, principal); | ||
| final String[] roles = extractRoles(threadContext, principal); | ||
| return new AuthCredentials(username, roles).markComplete(); | ||
| } catch (InvalidNameException e) { | ||
| log.error("Client cert had no properly formed DN"); | ||
| log.debug("Client cert had no properly formed DN (was: {})", principal); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: this would be repetitive with logs at debug level. Considering only logging the DN with this log line. i.e. |
||
| return null; | ||
| } | ||
| } | ||
|
|
||
| @Override | ||
| public Optional<SecurityResponse> reRequestAuthentication(final SecurityRequest response, AuthCredentials creds) { | ||
| return Optional.empty(); | ||
| private String extractUsername(ThreadContext ctx, String principal) throws InvalidNameException { | ||
| if (parsedUsernameAttr == null || (parsedUsernameAttr.type == AttributeType.DN && parsedUsernameAttr.dnAttr == null)) { | ||
| return principal; | ||
| } | ||
| List<String> usernames; | ||
| if (parsedUsernameAttr.type == AttributeType.DN) { | ||
| usernames = getDnAttribute(new LdapName(principal), parsedUsernameAttr.dnAttr); | ||
| } else { | ||
| usernames = extractFromSAN(ctx, parsedUsernameAttr.san); | ||
| } | ||
| return usernames == null || usernames.isEmpty() ? principal : usernames.get(0); | ||
| } | ||
|
|
||
| @Override | ||
| public String getType() { | ||
| return "clientcert"; | ||
| private String[] extractRoles(ThreadContext ctx, String principal) throws InvalidNameException { | ||
| if (parsedRolesAttr == null || (parsedRolesAttr.type == AttributeType.DN && parsedRolesAttr.dnAttr == null)) { | ||
| return null; | ||
| } | ||
| List<String> roles; | ||
| if (parsedRolesAttr.type == AttributeType.DN) { | ||
| roles = getDnAttribute(new LdapName(principal), parsedRolesAttr.dnAttr); | ||
| } else { | ||
| roles = extractFromSAN(ctx, parsedRolesAttr.san); | ||
| } | ||
| return roles == null || roles.isEmpty() ? null : roles.toArray(new String[0]); | ||
| } | ||
|
|
||
| private List<String> getDnAttribute(LdapName rfc2253dn, String attribute) { | ||
|
|
@@ -129,4 +198,101 @@ private List<String> getDnAttribute(LdapName rfc2253dn, String attribute) { | |
|
|
||
| return Collections.unmodifiableList(attrValues); | ||
| } | ||
|
|
||
| private static final int MAX_SAN_MATCHES = 16; | ||
| private static final int MAX_SAN_VALUE_LEN = 8192; | ||
|
|
||
| private List<String> extractFromSAN(ThreadContext ctx, ParsedSAN psan) { | ||
| if (psan == null) return Collections.emptyList(); | ||
|
|
||
| final X509Certificate[] peerCertificates = ctx.getTransient(ConfigConstants.OPENDISTRO_SECURITY_SSL_PEER_CERTIFICATES); | ||
|
|
||
| if (peerCertificates == null || peerCertificates.length == 0) { | ||
| return Collections.emptyList(); | ||
| } | ||
|
|
||
| try { | ||
| Collection<List<?>> altNames = peerCertificates[0].getSubjectAlternativeNames(); | ||
| if (altNames == null) return Collections.emptyList(); | ||
|
|
||
| return altNames.stream() | ||
| .filter(entry -> entry != null && entry.size() >= 2) | ||
| .filter(entry -> entry.get(0) instanceof Integer i && i.intValue() == psan.type()) | ||
| .map(entry -> sanValueToString(psan.type, entry.get(1))) | ||
| .map(v -> { | ||
| if (Strings.isNullOrEmpty(v)) return null; | ||
| if (psan.pattern() == null) return v; // no regex -> keep full | ||
| // bound input length before regex | ||
| String s = v.length() > MAX_SAN_VALUE_LEN ? v.substring(0, MAX_SAN_VALUE_LEN) : v; | ||
| Matcher m = psan.pattern().matcher(s); | ||
| if (!m.matches()) return null; | ||
| return (m.groupCount() >= 1) ? m.group(1) : s; // first capture group, else full | ||
| }) | ||
| .filter(Objects::nonNull) | ||
| .limit(MAX_SAN_MATCHES) | ||
| .collect(Collectors.toList()); | ||
| } catch (CertificateParsingException e) { | ||
| log.error("Error parsing X509 certificate", e); | ||
| return Collections.emptyList(); | ||
| } | ||
| } | ||
|
|
||
| // sometimes IP address is of type of byte[] | ||
| private static String sanValueToString(int type, Object value) { | ||
| if (value == null) return null; | ||
| if (value instanceof String) return (String) value; | ||
| if (type == SANType.IP_ADDRESS.value && value instanceof byte[]) { | ||
| byte[] addr = (byte[]) value; | ||
| try { | ||
| return java.net.InetAddress.getByAddress(addr).getHostAddress(); | ||
| } catch (java.net.UnknownHostException e) { | ||
| return null; | ||
| } | ||
| } | ||
| return null; | ||
| } | ||
|
|
||
| @Override | ||
| public Optional<SecurityResponse> reRequestAuthentication(final SecurityRequest response, AuthCredentials creds) { | ||
| return Optional.empty(); | ||
| } | ||
|
|
||
| @Override | ||
| public String getType() { | ||
| return "clientcert"; | ||
| } | ||
|
|
||
| /** | ||
| * Enumeration of supported SAN (Subject Alternative Name) types as defined in RFC 5280. | ||
| * https://datatracker.ietf.org/doc/html/rfc5280#section-4.2.1.6 | ||
| */ | ||
| private enum SANType { | ||
| OTHER_NAME(0), // OtherName | ||
| EMAIL(1), // rfc822Name | ||
| DNS(2), // dNSName | ||
| X400_ADDRESS(3), // x400Address | ||
| DIRECTORY_NAME(4), // directoryName | ||
| EDI_PARTY_NAME(5), // ediPartyName | ||
| URI(6), // uniformResourceIdentifier | ||
| IP_ADDRESS(7), // iPAddress | ||
| REGISTERED_ID(8); // registeredID | ||
|
|
||
| private static final Map<Integer, SANType> lookup = EnumSet.allOf(SANType.class) | ||
| .stream() | ||
| .collect(Collectors.toMap(SANType::getValue, sanType -> sanType)); | ||
|
|
||
| private final int value; | ||
|
|
||
| SANType(int value) { | ||
| this.value = value; | ||
| } | ||
|
|
||
| public int getValue() { | ||
| return value; | ||
| } | ||
|
|
||
| public static SANType fromValue(int value) { | ||
| return lookup.get(value); | ||
| } | ||
| } | ||
| } | ||
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe (at least per RFC [1]) that the pattern dialect being used is not compatible with Java pattern spec: like
*is equivalent of Java's.*, or I am missing something here? thank you[1] https://www.rfc-editor.org/rfc/rfc6125#section-6.4.3
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right now I treat the value as a Java regex and it should clearly be stated in the documentation.
As for the RFC, the wildcard spec is only for the case of SAN:DNS hostnames (e.g., *.example.com glob). We can still support both regex and glob but I'am afraid this would make things more complex and creates confusion for the user. I see three options:
A) Keep regex only and document clearly.
B) For DNS, support RFC-6125 glob by default with an explicit regex: opt-in.
C) Allow both for all SAN types via small prefixes (?glob) / (?regex) with auto-detect for simple globs.
What are your thoughts?
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Certainly don't intent to complicate things, basically my question would be - when if ever the pattern will be (and should be) Java compatible? (yes, we could document etc, but from practical standpoint). This looks to me as restriction that is artificial (due to the fact we use Java standard library), may be we could completely disregards the pattern handing?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The reason of using java regex is that we need to support groups capturing. Why? Because we want to be able to extract specific information to use it as a username or back-end role. For example, let's say the cert passes such info in the following format:
roles_attribute:SAN:URN:googleservice/groupid/123/deptWe want our regex to not only much this format but also extract the group id =123 and use in roles.
To be honest I was very hesitant to add regex support, the first version of this PR didn't have it. So I am willing to remove it for now and restrict the validation to exact much, and may in a futur PR extend it to support regex.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Certainly +1 to that, thank you @laminelam