Decode Subject Alternative Names (SAN) for X.509 Certificates #610
Conversation
| @@ -355,7 +355,17 @@ static List<SubjectName> getSubjectAltNames(final X509Certificate cert, final in | |||
| if (o instanceof String) { | |||
| result.add(new SubjectName((String) o, type)); | |||
| } else if (o instanceof byte[]) { | |||
| // TODO ASN.1 DER encoded form | |||
| final byte[] bytes = (byte[]) o; | |||
There was a problem hiding this comment.
@arturobernalg We have got to be super careful here as this method has security implications. It is better to be too strict and too lax here. I trust you know what you are doing.
There was a problem hiding this comment.
@arturobernalg We have got to be super careful here as this method has security implications. It is better to be too strict and too lax here. I trust you know what you are doing.
@ok2c I've ensured strict validation in the updated method: DNS names now reject unexpected byte[], IP addresses only allow 4 or 16 bytes with exceptions for invalid lengths, and unrecognized types log warnings while falling back.
| throw new IllegalArgumentException("Invalid byte length for IP address: " + bytes.length); | ||
| } | ||
| } else if (type == SubjectName.DNS) { | ||
| throw new IllegalArgumentException("Unexpected byte[] for DNS SAN type"); |
There was a problem hiding this comment.
@arturobernalg IllegalArgumentException looks really wrong here. Can we just ignore those bits that we are not able to properly recognize and handle?
| if (LOG.isWarnEnabled()) { | ||
| LOG.warn("Unrecognized SAN type: {}, data: {}", type, TextUtils.toHexString(bytes)); | ||
| } | ||
| decodedValue = TextUtils.toHexString(bytes); // Fallback to hex string |
There was a problem hiding this comment.
@arturobernalg I would not do that. Before you know there will be so called security professionals with all sorts of hand-crafted certificates, claiming vulnerabilities in our code and demanding we issue CVE with them as a finder.
There was a problem hiding this comment.
@ok2c please take another look
Supports DNS names as plain text, IPv4 and IPv6 addresses in binary form, and falls back to hex encoding for unknown types.
This implementation adds decoding for Subject Alternative Names in X.509 certificates, including: