Parse byte array of otherName SANs to string of ASN1 object and OID#1369
Merged
hardik-k-shah merged 1 commit intoopensearch-project:mainfrom Sep 9, 2021
Merged
Conversation
hardik-k-shah
previously approved these changes
Aug 26, 2021
| for (List<?> altName : altNames) { | ||
| Integer type = (Integer) altName.get(0); | ||
| // otherName requires parsing to string | ||
| if (type == 0) { |
Member
There was a problem hiding this comment.
I assume that only type 0 is of array of [oid, value] while all other types (like type 2 or 7) are string.
Contributor
Author
There was a problem hiding this comment.
Correct, the other types can be represented as strings directly using toString(), but type 0 (OtherName) is a byte array:
https://docs.oracle.com/javase/7/docs/api/java/security/cert/X509Certificate.html#getSubjectAlternativeNames()
"... No standard string format is defined for otherNames, X.400 names, EDI party names, or any other type of names. They are returned as byte arrays containing the ASN.1 DER encoded form of the name."
palashhedau
previously approved these changes
Sep 2, 2021
Member
|
@jksmth Can you please sign your commit, so we can merge it? |
Signed-off-by: Jake Smith <jakemgsmith@gmail.com>
c667666
29daf6f to
c667666
Compare
Contributor
Author
|
@hardik-k-shah commit has been signed |
hardik-k-shah
approved these changes
Sep 9, 2021
palashhedau
approved these changes
Sep 9, 2021
lbreinig
pushed a commit
to lbreinig/security
that referenced
this pull request
Dec 23, 2021
…pensearch-project#1369) Signed-off-by: Jake Smith <jakemgsmith@gmail.com>
wuychn
pushed a commit
to ochprince/security
that referenced
this pull request
Mar 16, 2023
…pensearch-project#1369) Signed-off-by: Jake Smith <jakemgsmith@gmail.com>
3 tasks
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Signed-off-by: Jake Smith jakemgsmith@gmail.com
opensearch-security pull request intake form
Please provide as much details as possible to get feedback/acceptance on your PR quickly
This PR replaces #943
Category: Bug fix
__Github Issue SSL certificate hot reload with OtherName in SAN #843
Description of changes:
Checks for type 0 (otherName) Subject Alternate Names (SANs) when validating certificate DNs, if found the byte array is read and if is a valid ASN1 object the OID and value is decoded to provide a string representation of the otherName. This is achieved with the use of bouncycastle lib.
Why these changes are required?
Per SSL certificate hot reload with OtherName in SAN #843
cert.getSubjectAlternativeNames().toString()does not lookup type 0 (otherName) SANs correctly and returns a string representing the byte array which changes on each iteration. This causes hot reload to fail if the certificate includes an otherName in the SAN as the string of the SANs returned bycert.getSubjectAlternativeNames().toString()will never match the current and new certificate in thehasValidDNs()method.What is the old behavior before changes and new behavior after changes?
Old behaviour
Getting currently loaded certificates with
GET /_opendistro/_security/api/ssl/certswould return:Note
[0, [B@6ac191ff]in transport certificate san. Repeated calls to GET /_opendistro/_security/api/ssl/certs shows that the string representation of the OtherName byte array (B@xxxxxxxx) changes each time.New behaviour
Getting currently loaded certificates with
GET /_opendistro/_security/api/ssl/certswill now return:Note
[0, [2.5.4.3, elastic-test-es-node-0]]in transport certificate san is now decoded to a list toString() of [oid, value]Testing done:
Verified by manually testing and unit tests. New test certificates have been created to include an otherName in the SAN.
TO-DOs, if any: (Please describe pending items and provide Github issues# for each of them)
Is it backport from main branch? (If yes, please add backport PR # and commits #)
By making a contribution to this project, I certify that:
(a) The contribution was created in whole or in part by me and I
have the right to submit it under the open source license
indicated in the file; or
(b) The contribution is based upon previous work that, to the best
of my knowledge, is covered under an appropriate open source
license and I have the right under that license to submit that
work with modifications, whether created in whole or in part
by me, under the same open source license (unless I am
permitted to submit under a different license), as indicated
in the file; or
(c) The contribution was provided directly to me by some other
person who certified (a), (b) or (c) and I have not modified
it.
(d) I understand and agree that this project and the contribution
are public and that a record of the contribution (including all
personal information I submit with it, including my sign-off) is
maintained indefinitely and may be redistributed consistent with
this project or the open source license(s) involved.
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.