From 2c9df274e914a9f3547fdee6a4cd9b68c0f7ca0c Mon Sep 17 00:00:00 2001 From: Andrew Ayer Date: Thu, 28 Apr 2016 21:26:59 -0700 Subject: [PATCH] Gracefully handle all manner of poorly encoded identifiers Also add preliminary support for IP address identifiers. --- cmd/ctwatch/main.go | 48 ++++------- helpers.go | 56 +++++++------ identifiers.go | 198 ++++++++++++++++++++++++++++++++++++++++++++ x509.go | 47 ++++++----- 4 files changed, 269 insertions(+), 80 deletions(-) create mode 100644 identifiers.go diff --git a/cmd/ctwatch/main.go b/cmd/ctwatch/main.go index 504c7e7..fe2d9d2 100644 --- a/cmd/ctwatch/main.go +++ b/cmd/ctwatch/main.go @@ -26,19 +26,6 @@ var stateDir = flag.String("state_dir", DefaultStateDir(), "Directory for storin var watchDomains []string var watchDomainSuffixes []string -func addWatchDomain (domain string) { - domain = strings.ToLower(domain) - - watchDomains = append(watchDomains, domain) - watchDomainSuffixes = append(watchDomainSuffixes, "." + domain) - - if dot := strings.IndexRune(domain, '.'); dot != -1 { - // also look for wildcard names that could match - // TODO: support exotic wildcards (wildcards besides "*.") in case there are CAs that issue them (there are) and clients that support them (less clear) - watchDomains = append(watchDomains, "*" + domain[dot:]) - } -} - func setWatchDomains (domains []string) error { for _, domain := range domains { if domain == "." { // "." as in root zone (matches everything) @@ -46,22 +33,19 @@ func setWatchDomains (domains []string) error { watchDomainSuffixes = []string{""} break } else { - addWatchDomain(domain) - - asciiDomain, err := idna.ToASCII(domain) + asciiDomain, err := idna.ToASCII(strings.ToLower(domain)) if err != nil { return fmt.Errorf("Invalid domain `%s': %s", domain, err) } - if asciiDomain != domain { - addWatchDomain(asciiDomain) - } - unicodeDomain, err := idna.ToUnicode(domain) - if err != nil { - return fmt.Errorf("Invalid domain `%s': %s", domain, err) - } - if unicodeDomain != domain { - addWatchDomain(unicodeDomain) + watchDomains = append(watchDomains, asciiDomain) + watchDomainSuffixes = append(watchDomainSuffixes, "." + asciiDomain) + + if dot := strings.IndexRune(asciiDomain, '.'); dot != -1 { + // also look for wildcard names that could match + // TODO: support exotic wildcards (wildcards besides "*.") in case there are CAs that issue them (there are) and clients that support them (less clear) + watchDomains = append(watchDomains, "*" + asciiDomain[dot:]) + // TODO: optionally match ?. and . also } } } @@ -69,14 +53,13 @@ func setWatchDomains (domains []string) error { } func dnsNameMatches (dnsName string) bool { - dnsNameLower := strings.ToLower(dnsName) for _, domain := range watchDomains { - if dnsNameLower == domain { + if dnsName == domain { return true } } for _, domainSuffix := range watchDomainSuffixes { - if strings.HasSuffix(dnsNameLower, domainSuffix) { + if strings.HasSuffix(dnsName, domainSuffix) { return true } } @@ -104,13 +87,10 @@ func processEntry (scanner *ctwatch.Scanner, entry *ct.LogEntry) { // If there's any sort of parse error related to the identifiers, report // the certificate because we can't say for sure it doesn't match a domain - // we care about (fail safe behavior). Treat common names as DNS names - // because many TLS clients do. + // we care about (fail safe behavior). if info.ParseError != nil || - info.CertInfo.CommonNamesParseError != nil || - info.CertInfo.DNSNamesParseError != nil || - anyDnsNameMatches(info.CertInfo.CommonNames) || - anyDnsNameMatches(info.CertInfo.DNSNames) { + info.CertInfo.IdentifiersParseError != nil || + anyDnsNameMatches(info.CertInfo.Identifiers.DNSNames) { cmd.LogEntry(&info) } } diff --git a/helpers.go b/helpers.go index 796d49f..042f860 100644 --- a/helpers.go +++ b/helpers.go @@ -102,10 +102,8 @@ type EntryInfo struct { type CertInfo struct { TBS *TBSCertificate - CommonNames []string - CommonNamesParseError error - DNSNames []string - DNSNamesParseError error + Identifiers *Identifiers + IdentifiersParseError error Subject RDNSequence SubjectParseError error Issuer RDNSequence @@ -121,8 +119,7 @@ type CertInfo struct { func MakeCertInfoFromTBS (tbs *TBSCertificate) *CertInfo { info := &CertInfo{TBS: tbs} - info.CommonNames, info.CommonNamesParseError = tbs.ParseCommonNames() - info.DNSNames, info.DNSNamesParseError = tbs.ParseDNSNames() + info.Identifiers, info.IdentifiersParseError = tbs.ParseIdentifiers() info.Subject, info.SubjectParseError = tbs.ParseSubject() info.Issuer, info.IssuerParseError = tbs.ParseIssuer() info.SerialNumber, info.SerialNumberParseError = tbs.ParseSerialNumber() @@ -161,17 +158,24 @@ func MakeCertInfoFromLogEntry (entry *ct.LogEntry) (*CertInfo, error) { } } -func (info *CertInfo) commonNamesString () string { - if info.CommonNamesParseError == nil { - return strings.Join(info.CommonNames, ", ") +func (info *CertInfo) dnsNamesString (sep string) string { + if info.IdentifiersParseError == nil { + return strings.Join(info.Identifiers.DNSNames, sep) } else { return "" } } -func (info *CertInfo) dnsNamesString () string { - if info.DNSNamesParseError == nil { - return strings.Join(info.DNSNames, ", ") +func (info *CertInfo) ipAddrsString (sep string) string { + if info.IdentifiersParseError == nil { + str := "" + for _, ipAddr := range info.Identifiers.IPAddrs { + if str != "" { + str += sep + } + str += ipAddr.String() + } + return str } else { return "" } @@ -206,16 +210,11 @@ func (info *CertInfo) Environ () []string { env = append(env, "PUBKEY_HASH=" + info.PubkeyHash()) - if info.CommonNamesParseError != nil { - env = append(env, "COMMON_NAMES_PARSE_ERROR=" + info.CommonNamesParseError.Error()) + if info.IdentifiersParseError != nil { + env = append(env, "IDENTIFIERS_PARSE_ERROR=" + info.IdentifiersParseError.Error()) } else { - env = append(env, "COMMON_NAMES=" + strings.Join(info.CommonNames, ",")) - } - - if info.DNSNamesParseError != nil { - env = append(env, "DNS_NAMES_PARSE_ERROR=" + info.DNSNamesParseError.Error()) - } else { - env = append(env, "DNS_NAMES=" + strings.Join(info.DNSNames, ",")) + env = append(env, "DNS_NAMES=" + info.dnsNamesString(",")) + env = append(env, "IP_ADDRESSES=" + info.ipAddrsString(",")) } if info.SerialNumberParseError != nil { @@ -250,8 +249,7 @@ func (info *CertInfo) Environ () []string { func (info *EntryInfo) HasParseErrors () bool { return info.ParseError != nil || - info.CertInfo.CommonNamesParseError != nil || - info.CertInfo.DNSNamesParseError != nil || + info.CertInfo.IdentifiersParseError != nil || info.CertInfo.SubjectParseError != nil || info.CertInfo.IssuerParseError != nil || info.CertInfo.SerialNumberParseError != nil || @@ -335,8 +333,16 @@ func (info *EntryInfo) Write (out io.Writer) { if info.ParseError != nil { writeField(out, "Parse Error", "*** " + info.ParseError.Error() + " ***", nil) } else { - writeField(out, "Common Name", info.CertInfo.commonNamesString(), info.CertInfo.CommonNamesParseError) - writeField(out, "DNS Names", info.CertInfo.dnsNamesString(), info.CertInfo.DNSNamesParseError) + if info.CertInfo.IdentifiersParseError != nil { + writeField(out, "Identifiers", nil, info.CertInfo.IdentifiersParseError) + } else { + for _, dnsName := range info.CertInfo.Identifiers.DNSNames { + writeField(out, "DNS Name", dnsName, nil) + } + for _, ipaddr := range info.CertInfo.Identifiers.IPAddrs { + writeField(out, "IP Address", ipaddr, nil) + } + } writeField(out, "Pubkey", info.CertInfo.PubkeyHash(), nil) writeField(out, "Subject", info.CertInfo.Subject, info.CertInfo.SubjectParseError) writeField(out, "Issuer", info.CertInfo.Issuer, info.CertInfo.IssuerParseError) diff --git a/identifiers.go b/identifiers.go new file mode 100644 index 0000000..af7a9ca --- /dev/null +++ b/identifiers.go @@ -0,0 +1,198 @@ +package ctwatch + +import ( + "bytes" + "strings" + "fmt" + "net" + "unicode/utf8" + "golang.org/x/net/idna" +) + +const invalidDNSLabelPlaceholder = "" + +/* +const ( + IdentifierSourceSubjectCN = iota + IdentifierSourceDNSName + IdentifierSourceIPAddr +) +type IdentifierSource int + +type UnknownIdentifier struct { + Source IdentifierSource + Value []byte +} +*/ + +type Identifiers struct { + DNSNames []string // stored as ASCII, with IDNs in Punycode + IPAddrs []net.IP + //Unknowns []UnknownIdentifier +} + +func NewIdentifiers () *Identifiers { + return &Identifiers{ + DNSNames: []string{}, + IPAddrs: []net.IP{}, + //Unknowns: []UnknownIdentifier{}, + } +} + +func parseIPAddrString (str string) net.IP { // TODO + return nil +} + +func isASCIIString (value []byte) bool { + for _, b := range value { + if b > 127 { + return false + } + } + return true +} +func isUTF8String (value []byte) bool { + return utf8.Valid(value) +} +func latin1ToUTF8 (value []byte) string { + runes := make([]rune, len(value)) + for i, b := range value { + runes[i] = rune(b) + } + return string(runes) +} + +// Validate a DNS label. We are less strict than we could be, +// because the main purpose of this is to prevent nasty characters +// from getting through that could cause headaches later (e.g. NUL, +// control characters). In particular, we allow '_' (since it's +// quite common in hostnames despite being prohibited), '*' (since +// it's used to represent wildcards), and '?' (since it's used +// in CT to represent redacted labels). +func isValidDNSLabelChar (ch rune) bool { + return (ch >= 'A' && ch <= 'Z') || + (ch >= 'a' && ch <= 'z') || + (ch >= '0' && ch <= '9') || + ch == '-' || ch == '_' || + ch == '*' || ch == '?'; +} +func isValidDNSLabel (label string) bool { + for _, ch := range label { + if !isValidDNSLabelChar(ch) { + return false + } + } + return true +} + +// Convert the DNS name to lower case and replace invalid labels with a placeholder +func sanitizeDNSName (value string) string { + value = strings.ToLower(value) + labels := strings.Split(value, ".") + for i, label := range labels { + if !isValidDNSLabel(label) { + labels[i] = invalidDNSLabelPlaceholder + } + } + return strings.Join(labels, ".") +} + +// Like sanitizeDNSName, but labels that are Unicode are converted to Punycode. +func sanitizeUnicodeDNSName (value string) string { + value = strings.ToLower(value) + labels := strings.Split(value, ".") + for i, label := range labels { + if asciiLabel, err := idna.ToASCII(label); err == nil && isValidDNSLabel(asciiLabel) { + labels[i] = asciiLabel + } else { + labels[i] = invalidDNSLabelPlaceholder + } + } + return strings.Join(labels, ".") +} + +func (ids *Identifiers) addDnsSANnonull (value []byte) { + if ipaddr := parseIPAddrString(string(value)); ipaddr != nil { + // Stupid CAs put IP addresses in DNS SANs because stupid Microsoft + // used to not support IP address SANs. Since there's no way for an IP + // address to also be a valid DNS name, just treat it like an IP address + // and not try to process it as a DNS name. + ids.IPAddrs = append(ids.IPAddrs, ipaddr) + } else if isASCIIString(value) { + ids.DNSNames = append(ids.DNSNames, sanitizeDNSName(string(value))) + } else { + // DNS SANs are supposed to be IA5Strings (i.e. ASCII) but CAs can't follow + // simple rules. Unfortunately, we have no idea what the encoding really is + // in this case, so interpret it as both UTF-8 (if it's valid UTF-8) + // and Latin-1. + if isUTF8String(value) { + ids.DNSNames = append(ids.DNSNames, sanitizeUnicodeDNSName(string(value))) + } + ids.DNSNames = append(ids.DNSNames, sanitizeUnicodeDNSName(latin1ToUTF8(value))) + } +} + +func (ids *Identifiers) AddDnsSAN (value []byte) { + if nullIndex := bytes.IndexByte(value, 0); nullIndex != -1 { + // If the value contains a null byte, process the part of + // the value up to the first null byte in addition to the + // complete value, in case this certificate is an attempt to + // fake out validators that only compare up to the first null. + ids.addDnsSANnonull(value[0:nullIndex]) + } + ids.addDnsSANnonull(value) +} + +func (ids *Identifiers) addCNnonull (value string) { + if ipaddr := parseIPAddrString(value); ipaddr != nil { + ids.IPAddrs = append(ids.IPAddrs, ipaddr) + } else if !strings.ContainsRune(value, ' ') { + // If the CN contains a space it's clearly not a DNS name, so ignore it. + ids.DNSNames = append(ids.DNSNames, sanitizeUnicodeDNSName(value)) + } +} + +func (ids *Identifiers) AddCN (value string) { + if nullIndex := strings.IndexRune(value, 0); nullIndex != -1 { + // If the value contains a null byte, process the part of + // the value up to the first null byte in addition to the + // complete value, in case this certificate is an attempt to + // fake out validators that only compare up to the first null. + ids.addCNnonull(value[0:nullIndex]) + } + ids.addCNnonull(value) +} + +func (ids *Identifiers) AddIPAddress (value net.IP) { + ids.IPAddrs = append(ids.IPAddrs, value) +} + +func (tbs *TBSCertificate) ParseIdentifiers () (*Identifiers, error) { + ids := NewIdentifiers() + + cns, err := tbs.ParseSubjectCommonNames() + if err != nil { + return nil, err + } + for _, cn := range cns { + ids.AddCN(cn) + } + + sans, err := tbs.ParseSubjectAltNames() + if err != nil { + return nil, err + } + for _, san := range sans { + switch san.Type { + case sanDNSName: + ids.AddDnsSAN(san.Value) + case sanIPAddress: + if !(len(san.Value) == 4 || len(san.Value) == 16) { + return nil, fmt.Errorf("IP Address SAN has bogus length %d", len(san.Value)) + } + ids.AddIPAddress(net.IP(san.Value)) + } + } + + return ids, nil +} diff --git a/x509.go b/x509.go index 58ad15f..12b5e76 100644 --- a/x509.go +++ b/x509.go @@ -5,7 +5,6 @@ import ( "bytes" "errors" "encoding/asn1" - "unicode/utf8" "math/big" "time" ) @@ -40,6 +39,22 @@ type Extension struct { Value []byte } +const ( + sanOtherName = 0 + sanRfc822Name = 1 + sanDNSName = 2 + sanX400Address = 3 + sanDirectoryName = 4 + sanEdiPartyName = 5 + sanURI = 6 + sanIPAddress = 7 + sanRegisteredID = 8 +) +type SubjectAltName struct { + Type int + Value []byte +} + type RDNSequence []RelativeDistinguishedNameSET type RelativeDistinguishedNameSET []AttributeTypeAndValue type AttributeTypeAndValue struct { @@ -122,7 +137,7 @@ func (rdns RDNSequence) String () string { buf.WriteString("=") valueString, err := decodeASN1String(&atv.Value) if err == nil { - buf.WriteString(valueString) + buf.WriteString(valueString) // TODO: escape non-printable characters, '\', and ',' } else { fmt.Fprintf(&buf, "%v", atv.Value.FullBytes) } @@ -241,7 +256,7 @@ func (tbs *TBSCertificate) ParseIssuer () (RDNSequence, error) { return issuer, nil } -func (tbs *TBSCertificate) ParseCommonNames () ([]string, error) { +func (tbs *TBSCertificate) ParseSubjectCommonNames () ([]string, error) { subject, err := tbs.ParseSubject() if err != nil { return nil, err @@ -254,19 +269,18 @@ func (tbs *TBSCertificate) ParseCommonNames () ([]string, error) { return cns, nil } -func (tbs *TBSCertificate) ParseDNSNames () ([]string, error) { - dnsNames := []string{} +func (tbs *TBSCertificate) ParseSubjectAltNames () ([]SubjectAltName, error) { + sans := []SubjectAltName{} - // Extract DNS names from SubjectAlternativeName extension for _, sanExt := range tbs.GetExtension(oidExtensionSubjectAltName) { - dnsSans, err := parseSANExtension(sanExt.Value) + var err error + sans, err = parseSANExtension(sans, sanExt.Value) if err != nil { return nil, err } - dnsNames = append(dnsNames, dnsSans...) } - return dnsNames, nil + return sans, nil } func (tbs *TBSCertificate) GetExtension (id asn1.ObjectIdentifier) []Extension { @@ -298,8 +312,7 @@ func (cert *Certificate) ParseTBSCertificate () (*TBSCertificate, error) { return ParseTBSCertificate(cert.GetRawTBSCertificate()) } -func parseSANExtension (value []byte) ([]string, error) { - var dnsNames []string +func parseSANExtension (sans []SubjectAltName, value []byte) ([]SubjectAltName, error) { var seq asn1.RawValue if rest, err := asn1.Unmarshal(value, &seq); err != nil { return nil, errors.New("failed to parse subjectAltName extension: " + err.Error()) @@ -322,17 +335,9 @@ func parseSANExtension (value []byte) ([]string, error) { if err != nil { return nil, errors.New("failed to parse subjectAltName extension item: " + err.Error()) } - switch val.Tag { - case 2: - // This should be an IA5String (i.e. ASCII) with IDNs encoded in Punycode, but there are - // too many certs in the wild which have UTF-8 in their DNS SANs. - if !utf8.Valid(val.Bytes) { - return nil, errors.New("failed to parse subjectAltName: DNS name contains invalid UTF-8") - } - dnsNames = append(dnsNames, string(val.Bytes)) - } + sans = append(sans, SubjectAltName{Type: val.Tag, Value: val.Bytes}) } - return dnsNames, nil + return sans, nil }