Principal Extraction from Certificate RDN Attribute Value in PKI Realm. #137230
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| pr: 137230 | ||
| summary: Principal Extraction from Certificate RDN Attribute Value in PKI Realm | ||
| area: Security | ||
| type: bug | ||
| issues: [] |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,62 @@ | ||
| /* | ||
| * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one | ||
| * or more contributor license agreements. Licensed under the Elastic License | ||
| * 2.0; you may not use this file except in compliance with the Elastic License | ||
| * 2.0. | ||
| */ | ||
|
|
||
| package org.elasticsearch.xpack.security.authc.pki; | ||
|
|
||
| import org.elasticsearch.common.ssl.DerParser; | ||
|
|
||
| import java.io.IOException; | ||
|
|
||
| /** | ||
| * Utility class to extract RDN field values from X500 principal DER encoding. | ||
| */ | ||
| public class RdnFieldExtractor { | ||
|
Contributor
There was a problem hiding this comment. When I proposed that we process RDNs, I didn't think we were going to need to write our own parser (but the alternative isn't really obvious) We have multiple DN/RDN implementations already, they're not a perfect fit for what we want, but I think some of them can be made to work
I think we can avoid this custom parsing code entirely if we use either
We use If I had to pick, I guess I'd use cryptacular, since it does exactly what we need, and we can swap it out if we ever need to. That said, it's not the worst thing to use our own parser (it's surprisingly short since we have a working DER parser), but we lose schema support, and I really don't think we can force OIDs onto our users. So, even if we stick with our own parsing we'll need to pull in a schema from one of those libraries so we can work with standard attribute names.
Contributor
Author
There was a problem hiding this comment. I agree that requiring schema definition awareness weakens the case for
Can we determine how the username pattern setting is used today? That might help inform the OID decision.
Contributor
Author
There was a problem hiding this comment. The hurdle that I ran into earlier with This is what led me to a direct interpretation of the DER bytes, which bypasses nuances with text formatting. String s = "OID.1.3.6.1.4.1.50000.1.1=EMP-2024-42, CN=Jane Developer, OU=Engineering, O=Acme Corp";
X500Principal principal = new X500Principal(s);
System.out.println(principal.getName(X500Principal.RFC2253)); |
||
|
|
||
| public static String extract(byte[] encoded, String oid) { | ||
| try { | ||
| return doExtract(encoded, oid); | ||
| } catch (IOException | IllegalStateException e) { | ||
| return null; // invalid encoding | ||
| } | ||
| } | ||
|
|
||
| private static String doExtract(byte[] encoded, String oid) throws IOException { | ||
|
Contributor
There was a problem hiding this comment. Trying to understand what's going on here. Each
Contributor
Author
There was a problem hiding this comment. Yes, that's right. My understanding is that "constructed" is a tag bit that denotes a sequence or set.
Contributor
There was a problem hiding this comment. You can read chapter 12 of the ASN.1 book if you're particularly interested in becoming an expert in ASN.1, but yes, "constructed" is akin to "structured", that is a non-primitive ( a sequence, set or choice ) |
||
| DerParser parser = new DerParser(encoded); | ||
|
|
||
| DerParser.Asn1Object dnSequence = parser.readAsn1Object(DerParser.Type.SEQUENCE); | ||
| DerParser sequenceParser = dnSequence.getParser(); | ||
|
|
||
| String value = null; | ||
|
|
||
| while (true) { | ||
| try { | ||
| DerParser.Asn1Object rdnSet = sequenceParser.readAsn1Object(DerParser.Type.SET); // throws IOException on EOF | ||
| DerParser setParser = rdnSet.getParser(); | ||
|
|
||
| while (true) { | ||
| try { | ||
| DerParser.Asn1Object attrSeq = setParser.readAsn1Object(DerParser.Type.SEQUENCE); // throws IOException on EOF | ||
| DerParser attrParser = attrSeq.getParser(); | ||
|
|
||
| String attrOid = attrParser.readAsn1Object().getOid(); | ||
| DerParser.Asn1Object attrValue = attrParser.readAsn1Object(); | ||
| if (oid.equals(attrOid)) { | ||
| value = attrValue.getString(); // retain last (most-significant) occurrence | ||
| } | ||
| } catch (IOException e) { | ||
| break; // RDN SET EOF | ||
| } | ||
| } | ||
| } catch (IOException e) { | ||
| break; // DN SEQUENCE EOF | ||
| } | ||
| } | ||
|
|
||
| return value; | ||
| } | ||
|
|
||
| } | ||
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.
Maybe this should be a warning since it's probably due to a misconfiguration most of the time?
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.
This was also a case of remaining consistent with surrounding code.
getPrincipalFromSubjectDNusesdebuglogging.