Make trailing garbage a fatal error when extracting DNS names

Logging something to stderr was not helpful, and it's best to be
on the safe side anyways.

Whitelist a single null byte following the SAN extension.  This
is a harmless and common error.

As of now, all certificates in the CT logs parse successfully.
This commit is contained in:
Andrew Ayer 2016-02-22 19:35:21 -08:00
parent 08fa700d29
commit 2608a74e66
1 changed files with 8 additions and 5 deletions

View File

@ -1,7 +1,6 @@
package ctwatch package ctwatch
import ( import (
"os"
"fmt" "fmt"
"errors" "errors"
"bytes" "bytes"
@ -123,7 +122,11 @@ func parseSANExtension (value []byte) ([]string, error) {
if rest, err := asn1.Unmarshal(value, &seq); err != nil { if rest, err := asn1.Unmarshal(value, &seq); err != nil {
return nil, errors.New("failed to parse subjectAltName extension: " + err.Error()) return nil, errors.New("failed to parse subjectAltName extension: " + err.Error())
} else if len(rest) != 0 { } else if len(rest) != 0 {
fmt.Fprintf(os.Stderr, "Warning: trailing data after subjectAltName extension\n") // Don't complain if the SAN is followed by exactly one zero byte,
// which is a common error.
if !(len(rest) == 1 && rest[0] == 0) {
return nil, fmt.Errorf("trailing data in subjectAltName extension: %v", rest)
}
} }
if !seq.IsCompound || seq.Tag != 16 || seq.Class != 0 { if !seq.IsCompound || seq.Tag != 16 || seq.Class != 0 {
return nil, errors.New("failed to parse subjectAltName extension: bad SAN sequence") return nil, errors.New("failed to parse subjectAltName extension: bad SAN sequence")
@ -153,7 +156,7 @@ func ExtractDNSNamesFromTBS (tbsBytes []byte) ([]string, error) {
if rest, err := asn1.Unmarshal(tbsBytes, &tbs); err != nil { if rest, err := asn1.Unmarshal(tbsBytes, &tbs); err != nil {
return nil, errors.New("failed to parse TBS: " + err.Error()) return nil, errors.New("failed to parse TBS: " + err.Error())
} else if len(rest) > 0 { } else if len(rest) > 0 {
fmt.Fprintf(os.Stderr, "Warning: trailing data after TBS\n") return nil, fmt.Errorf("trailing data after TBS: %v", rest)
} }
// Extract Common Name from Subject // Extract Common Name from Subject
@ -161,7 +164,7 @@ func ExtractDNSNamesFromTBS (tbsBytes []byte) ([]string, error) {
if rest, err := asn1.Unmarshal(tbs.Subject.FullBytes, &subject); err != nil { if rest, err := asn1.Unmarshal(tbs.Subject.FullBytes, &subject); err != nil {
return nil, errors.New("failed to parse certificate subject: " + err.Error()) return nil, errors.New("failed to parse certificate subject: " + err.Error())
} else if len(rest) != 0 { } else if len(rest) != 0 {
fmt.Fprintf(os.Stderr, "Warning: trailing data after certificate subject\n") return nil, fmt.Errorf("trailing data in certificate subject: %v", rest)
} }
cns, err := getCNs(&subject) cns, err := getCNs(&subject)
if err != nil { if err != nil {
@ -188,7 +191,7 @@ func ExtractDNSNames (certBytes []byte) ([]string, error) {
if rest, err := asn1.Unmarshal(certBytes, &cert); err != nil { if rest, err := asn1.Unmarshal(certBytes, &cert); err != nil {
return nil, errors.New("failed to parse certificate: " + err.Error()) return nil, errors.New("failed to parse certificate: " + err.Error())
} else if len(rest) > 0 { } else if len(rest) > 0 {
fmt.Fprintf(os.Stderr, "Warning: trailing data after certificate\n") return nil, fmt.Errorf("trailing data after certificate: %v", rest)
} }
return ExtractDNSNamesFromTBS(cert.TBSCertificate.FullBytes) return ExtractDNSNamesFromTBS(cert.TBSCertificate.FullBytes)