Skip to content

Commit eb441e6

Browse files
christopher-hendersonbradfitz
christopher-henderson
authored and
bradfitz
committed
encoding/asn1: allow '&' in PrintableString fields
There are, unfortunately, intermediate CA ceritificates in circulation that contain the invalid character '&' in some PrintableString fields, notably Organization Name. This patch allows for ampersand to be parsed as though it is valid in an ASN.1 PrintableString. Fixes golang#22970 Change-Id: Ifab1a10bbff1cdac68e843c6b857ff1a031051aa Reviewed-on: https://go-review.googlesource.com/81635 Reviewed-by: Adam Langley <[email protected]> Reviewed-by: Brad Fitzpatrick <[email protected]> Run-TryBot: Adam Langley <[email protected]> TryBot-Result: Gobot Gobot <[email protected]>
1 parent bcf964d commit eb441e6

4 files changed

Lines changed: 21 additions & 6 deletions

File tree

‎src/encoding/asn1/asn1.go‎

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -397,7 +397,7 @@ func isNumeric(b byte) bool {
397397
// array and returns it.
398398
func parsePrintableString(bytes []byte) (ret string, err error) {
399399
for _, b := range bytes {
400-
if !isPrintable(b, allowAsterisk) {
400+
if !isPrintable(b, allowAsterisk, allowAmpersand) {
401401
err = SyntaxError{"PrintableString contains invalid character"}
402402
return
403403
}
@@ -407,16 +407,20 @@ func parsePrintableString(bytes []byte) (ret string, err error) {
407407
}
408408

409409
type asteriskFlag bool
410+
type ampersandFlag bool
410411

411412
const (
412413
allowAsterisk asteriskFlag = true
413414
rejectAsterisk asteriskFlag = false
415+
416+
allowAmpersand ampersandFlag = true
417+
rejectAmpersand ampersandFlag = false
414418
)
415419

416420
// isPrintable reports whether the given b is in the ASN.1 PrintableString set.
417421
// If asterisk is allowAsterisk then '*' is also allowed, reflecting existing
418-
// practice.
419-
func isPrintable(b byte, asterisk asteriskFlag) bool {
422+
// practice. If ampersand is allowAmpersand then '&' is allowed as well.
423+
func isPrintable(b byte, asterisk asteriskFlag, ampersand ampersandFlag) bool {
420424
return 'a' <= b && b <= 'z' ||
421425
'A' <= b && b <= 'Z' ||
422426
'0' <= b && b <= '9' ||
@@ -429,7 +433,12 @@ func isPrintable(b byte, asterisk asteriskFlag) bool {
429433
// This is technically not allowed in a PrintableString.
430434
// However, x509 certificates with wildcard strings don't
431435
// always use the correct string type so we permit it.
432-
(bool(asterisk) && b == '*')
436+
(bool(asterisk) && b == '*') ||
437+
// This is not technically allowed either. However, not
438+
// only is it relatively common, but there are also a
439+
// handful of CA certificates that contain it. At least
440+
// one of which will not expire until 2027.
441+
(bool(ampersand) && b == '&')
433442
}
434443

435444
// IA5String

‎src/encoding/asn1/asn1_test.go‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -487,6 +487,8 @@ var unmarshalTestData = []struct {
487487
{[]byte{0x02, 0x01, 0x10}, newInt(16)},
488488
{[]byte{0x13, 0x04, 't', 'e', 's', 't'}, newString("test")},
489489
{[]byte{0x16, 0x04, 't', 'e', 's', 't'}, newString("test")},
490+
// Ampersand is allowed in PrintableString due to mistakes by major CAs.
491+
{[]byte{0x13, 0x05, 't', 'e', 's', 't', '&'}, newString("test&")},
490492
{[]byte{0x16, 0x04, 't', 'e', 's', 't'}, &RawValue{0, 22, false, []byte("test"), []byte("\x16\x04test")}},
491493
{[]byte{0x04, 0x04, 1, 2, 3, 4}, &RawValue{0, 4, false, []byte{1, 2, 3, 4}, []byte{4, 4, 1, 2, 3, 4}}},
492494
{[]byte{0x30, 0x03, 0x81, 0x01, 0x01}, &TestContextSpecificTags{1}},

‎src/encoding/asn1/marshal.go‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -271,7 +271,10 @@ func makePrintableString(s string) (e encoder, err error) {
271271
// The asterisk is often used in PrintableString, even though
272272
// it is invalid. If a PrintableString was specifically
273273
// requested then the asterisk is permitted by this code.
274-
if !isPrintable(s[i], allowAsterisk) {
274+
// Ampersand is allowed in parsing due a handful of CA
275+
// certificates, however when making new certificates
276+
// it is rejected.
277+
if !isPrintable(s[i], allowAsterisk, rejectAmpersand) {
275278
return nil, StructuralError{"PrintableString contains invalid character"}
276279
}
277280
}
@@ -591,7 +594,7 @@ func makeField(v reflect.Value, params fieldParameters) (e encoder, err error) {
591594
// a PrintableString if the character set in the string is
592595
// sufficiently limited, otherwise we'll use a UTF8String.
593596
for _, r := range v.String() {
594-
if r >= utf8.RuneSelf || !isPrintable(byte(r), rejectAsterisk) {
597+
if r >= utf8.RuneSelf || !isPrintable(byte(r), rejectAsterisk, rejectAmpersand) {
595598
if !utf8.ValidString(v.String()) {
596599
return nil, errors.New("asn1: string not valid UTF-8")
597600
}

‎src/encoding/asn1/marshal_test.go‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -157,6 +157,7 @@ var marshalTests = []marshalTest{
157157
{printableStringTest{"test*"}, "30071305746573742a"},
158158
{genericStringTest{"test"}, "3006130474657374"},
159159
{genericStringTest{"test*"}, "30070c05746573742a"},
160+
{genericStringTest{"test&"}, "30070c057465737426"},
160161
{rawContentsStruct{nil, 64}, "3003020140"},
161162
{rawContentsStruct{[]byte{0x30, 3, 1, 2, 3}, 64}, "3003010203"},
162163
{RawValue{Tag: 1, Class: 2, IsCompound: false, Bytes: []byte{1, 2, 3}}, "8103010203"},

0 commit comments

Comments
 (0)