Gracefully handle all manner of poorly encoded identifiers

Also add preliminary support for IP address identifiers.

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 "*.<DOMAIN>") 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(strings.ToLower(domain))
-
-			asciiDomain, err := idna.ToASCII(domain)
 			if err != nil {
 				return fmt.Errorf("Invalid domain `%s': %s", domain, err)
 			}
-			if asciiDomain != domain {
-				addWatchDomain(asciiDomain)
-			}
 
-			unicodeDomain, err := idna.ToUnicode(domain)
+			watchDomains = append(watchDomains, asciiDomain)
-			if err != nil {
+			watchDomainSuffixes = append(watchDomainSuffixes, "." + asciiDomain)
-				return fmt.Errorf("Invalid domain `%s': %s", domain, err)
+
-			}
+			if dot := strings.IndexRune(asciiDomain, '.'); dot != -1 {
-			if unicodeDomain != domain {
+				// also look for wildcard names that could match
-				addWatchDomain(unicodeDomain)
+				// TODO: support exotic wildcards (wildcards besides "*.<DOMAIN>") 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 ?.<DOMAIN> and <invalid>.<DOMAIN> 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
+	// we care about (fail safe behavior).
-	// because many TLS clients do.
 	if info.ParseError != nil ||
-			info.CertInfo.CommonNamesParseError != nil ||
+			info.CertInfo.IdentifiersParseError != nil ||
-			info.CertInfo.DNSNamesParseError != nil ||
+			anyDnsNameMatches(info.CertInfo.Identifiers.DNSNames) {
-			anyDnsNameMatches(info.CertInfo.CommonNames) ||
-			anyDnsNameMatches(info.CertInfo.DNSNames) {
 		cmd.LogEntry(&info)
 	}
 }

helpers.go

@@ -102,10 +102,8 @@ type EntryInfo struct {
 type CertInfo struct {
 	TBS			*TBSCertificate
 
-	CommonNames		[]string
+	Identifiers		*Identifiers
-	CommonNamesParseError	error
+	IdentifiersParseError	error
-	DNSNames		[]string
-	DNSNamesParseError	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.Identifiers, info.IdentifiersParseError = tbs.ParseIdentifiers()
-	info.DNSNames, info.DNSNamesParseError = tbs.ParseDNSNames()
 	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 {
+func (info *CertInfo) dnsNamesString (sep string) string {
-	if info.CommonNamesParseError == nil {
+	if info.IdentifiersParseError == nil {
-		return strings.Join(info.CommonNames, ", ")
+		return strings.Join(info.Identifiers.DNSNames, sep)
 	} else {
 		return ""
 	}
 }
 
-func (info *CertInfo) dnsNamesString () string {
+func (info *CertInfo) ipAddrsString (sep string) string {
-	if info.DNSNamesParseError == nil {
+	if info.IdentifiersParseError == nil {
-		return strings.Join(info.DNSNames, ", ")
+		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 {
+	if info.IdentifiersParseError != nil {
-		env = append(env, "COMMON_NAMES_PARSE_ERROR=" + info.CommonNamesParseError.Error())
+		env = append(env, "IDENTIFIERS_PARSE_ERROR=" + info.IdentifiersParseError.Error())
 	} else {
-		env = append(env, "COMMON_NAMES=" + strings.Join(info.CommonNames, ","))
+		env = append(env, "DNS_NAMES=" + info.dnsNamesString(","))
-	}
+		env = append(env, "IP_ADDRESSES=" + info.ipAddrsString(","))
-
-	if info.DNSNamesParseError != nil {
-		env = append(env, "DNS_NAMES_PARSE_ERROR=" + info.DNSNamesParseError.Error())
-	} else {
-		env = append(env, "DNS_NAMES=" + strings.Join(info.DNSNames, ","))
 	}
 
 	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.IdentifiersParseError != nil ||
-		info.CertInfo.DNSNamesParseError != 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)
+		if info.CertInfo.IdentifiersParseError != nil {
-		writeField(out, "DNS Names", info.CertInfo.dnsNamesString(), info.CertInfo.DNSNamesParseError)
+			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)

identifiers.go

@@ -0,0 +1,198 @@
+package ctwatch
+
+import (
+	"bytes"
+	"strings"
+	"fmt"
+	"net"
+	"unicode/utf8"
+	"golang.org/x/net/idna"
+)
+
+const invalidDNSLabelPlaceholder = "<invalid>"
+
+/*
+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
+}

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) {
+func (tbs *TBSCertificate) ParseSubjectAltNames () ([]SubjectAltName, error) {
-	dnsNames := []string{}
+	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) {
+func parseSANExtension (sans []SubjectAltName, value []byte) ([]SubjectAltName, error) {
-	var dnsNames []string
 	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 {
+		sans = append(sans, SubjectAltName{Type: val.Tag, Value: val.Bytes})
-		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))
-		}
 	}
 
-	return dnsNames, nil
+	return sans, nil
 }