From 722d59436bc5881914619d2b95c9d01a46036428 Mon Sep 17 00:00:00 2001 From: Mateusz Poliwczak Date: Wed, 15 May 2024 17:36:53 +0000 Subject: [PATCH] crypto/x509: add text and binary marshal methods to OID Fixes #66249 Change-Id: I5973a19a087a35ad951e8a220d3e6e4456c7577f GitHub-Last-Rev: 921ca8bd0c08687bb727dbfb0890c3355eebe95b GitHub-Pull-Request: golang/go#66599 Reviewed-on: https://go-review.googlesource.com/c/go/+/575295 Reviewed-by: Rob Pike Reviewed-by: Dmitri Shuralyov LUCI-TryBot-Result: Go LUCI Reviewed-by: Roland Shoemaker Auto-Submit: Roland Shoemaker --- api/next/66249.txt | 5 + .../6-stdlib/99-minor/crypto/x509/66249.md | 3 + src/crypto/x509/oid.go | 112 ++++++++ src/crypto/x509/oid_test.go | 259 ++++++++++++++---- src/encoding/gob/encode.go | 2 +- src/encoding/gob/type.go | 14 +- 6 files changed, 329 insertions(+), 66 deletions(-) create mode 100644 api/next/66249.txt create mode 100644 doc/next/6-stdlib/99-minor/crypto/x509/66249.md diff --git a/api/next/66249.txt b/api/next/66249.txt new file mode 100644 index 0000000000..f9d7a1addc --- /dev/null +++ b/api/next/66249.txt @@ -0,0 +1,5 @@ +pkg crypto/x509, func ParseOID(string) (OID, error) #66249 +pkg crypto/x509, method (*OID) UnmarshalBinary([]uint8) error #66249 +pkg crypto/x509, method (*OID) UnmarshalText([]uint8) error #66249 +pkg crypto/x509, method (OID) MarshalBinary() ([]uint8, error) #66249 +pkg crypto/x509, method (OID) MarshalText() ([]uint8, error) #66249 diff --git a/doc/next/6-stdlib/99-minor/crypto/x509/66249.md b/doc/next/6-stdlib/99-minor/crypto/x509/66249.md new file mode 100644 index 0000000000..5b1d98222a --- /dev/null +++ b/doc/next/6-stdlib/99-minor/crypto/x509/66249.md @@ -0,0 +1,3 @@ +The new [`ParseOID`](/pkg/crypto/x509#ParseOID) function parses a dot-encoded ASN.1 Object Identifier string. +The [`OID`](/pkg/crypto/x509#OID) type now implements the [`BinaryMarshaler`](/pkg/encoding#BinaryMarshaler), [`BinaryUnmarshaler`](/pkg/encoding#BinaryUnmarshaler), +[`TextMarshaler`](/pkg/encoding#TextMarshaler), [`TextUnmarshaler`](/pkg/encoding#TextUnmarshaler) interfaces. diff --git a/src/crypto/x509/oid.go b/src/crypto/x509/oid.go index 5359af624b..b00c35e696 100644 --- a/src/crypto/x509/oid.go +++ b/src/crypto/x509/oid.go @@ -24,6 +24,12 @@ type OID struct { der []byte } +// ParseOID parses a Object Identifier string, represented by ASCII numbers separated by dots. +func ParseOID(oid string) (OID, error) { + var o OID + return o, o.unmarshalOIDText(oid) +} + func newOIDFromDER(der []byte) (OID, bool) { if len(der) == 0 || der[len(der)-1]&0x80 != 0 { return OID{}, false @@ -83,6 +89,112 @@ func appendBase128Int(dst []byte, n uint64) []byte { return dst } +func base128BigIntLength(n *big.Int) int { + if n.Cmp(big.NewInt(0)) == 0 { + return 1 + } + return (n.BitLen() + 6) / 7 +} + +func appendBase128BigInt(dst []byte, n *big.Int) []byte { + if n.Cmp(big.NewInt(0)) == 0 { + return append(dst, 0) + } + + for i := base128BigIntLength(n) - 1; i >= 0; i-- { + o := byte(big.NewInt(0).Rsh(n, uint(i)*7).Bits()[0]) + o &= 0x7f + if i != 0 { + o |= 0x80 + } + dst = append(dst, o) + } + return dst +} + +// MarshalText implements [encoding.TextMarshaler] +func (o OID) MarshalText() ([]byte, error) { + return []byte(o.String()), nil +} + +// UnmarshalText implements [encoding.TextUnmarshaler] +func (o *OID) UnmarshalText(text []byte) error { + return o.unmarshalOIDText(string(text)) +} + +func (o *OID) unmarshalOIDText(oid string) error { + // (*big.Int).SetString allows +/- signs, but we don't want + // to allow them in the string representation of Object Identifier, so + // reject such encodings. + for _, c := range oid { + isDigit := c >= '0' && c <= '9' + if !isDigit && c != '.' { + return errInvalidOID + } + } + + var ( + firstNum string + secondNum string + ) + + var nextComponentExists bool + firstNum, oid, nextComponentExists = strings.Cut(oid, ".") + if !nextComponentExists { + return errInvalidOID + } + secondNum, oid, nextComponentExists = strings.Cut(oid, ".") + + var ( + first = big.NewInt(0) + second = big.NewInt(0) + ) + + if _, ok := first.SetString(firstNum, 10); !ok { + return errInvalidOID + } + if _, ok := second.SetString(secondNum, 10); !ok { + return errInvalidOID + } + + if first.Cmp(big.NewInt(2)) > 0 || (first.Cmp(big.NewInt(2)) < 0 && second.Cmp(big.NewInt(40)) >= 0) { + return errInvalidOID + } + + firstComponent := first.Mul(first, big.NewInt(40)) + firstComponent.Add(firstComponent, second) + + der := appendBase128BigInt(make([]byte, 0, 32), firstComponent) + + for nextComponentExists { + var strNum string + strNum, oid, nextComponentExists = strings.Cut(oid, ".") + b, ok := big.NewInt(0).SetString(strNum, 10) + if !ok { + return errInvalidOID + } + der = appendBase128BigInt(der, b) + } + + o.der = der + return nil +} + +// MarshalBinary implements [encoding.BinaryMarshaler] +func (o OID) MarshalBinary() ([]byte, error) { + return bytes.Clone(o.der), nil +} + +// UnmarshalBinary implements [encoding.BinaryUnmarshaler] +func (o *OID) UnmarshalBinary(b []byte) error { + oid, ok := newOIDFromDER(bytes.Clone(b)) + if !ok { + return errInvalidOID + } + *o = oid + return nil +} + // Equal returns true when oid and other represents the same Object Identifier. func (oid OID) Equal(other OID) bool { // There is only one possible DER encoding of diff --git a/src/crypto/x509/oid_test.go b/src/crypto/x509/oid_test.go index eb47244a73..cbb3406424 100644 --- a/src/crypto/x509/oid_test.go +++ b/src/crypto/x509/oid_test.go @@ -5,54 +5,54 @@ package x509 import ( + "encoding" "encoding/asn1" "math" "testing" ) +var oidTests = []struct { + raw []byte + valid bool + str string + ints []uint64 +}{ + {[]byte{}, false, "", nil}, + {[]byte{0x80, 0x01}, false, "", nil}, + {[]byte{0x01, 0x80, 0x01}, false, "", nil}, + + {[]byte{1, 2, 3}, true, "0.1.2.3", []uint64{0, 1, 2, 3}}, + {[]byte{41, 2, 3}, true, "1.1.2.3", []uint64{1, 1, 2, 3}}, + {[]byte{86, 2, 3}, true, "2.6.2.3", []uint64{2, 6, 2, 3}}, + + {[]byte{41, 255, 255, 255, 127}, true, "1.1.268435455", []uint64{1, 1, 268435455}}, + {[]byte{41, 0x87, 255, 255, 255, 127}, true, "1.1.2147483647", []uint64{1, 1, 2147483647}}, + {[]byte{41, 255, 255, 255, 255, 127}, true, "1.1.34359738367", []uint64{1, 1, 34359738367}}, + {[]byte{42, 255, 255, 255, 255, 255, 255, 255, 255, 127}, true, "1.2.9223372036854775807", []uint64{1, 2, 9223372036854775807}}, + {[]byte{43, 0x81, 255, 255, 255, 255, 255, 255, 255, 255, 127}, true, "1.3.18446744073709551615", []uint64{1, 3, 18446744073709551615}}, + {[]byte{44, 0x83, 255, 255, 255, 255, 255, 255, 255, 255, 127}, true, "1.4.36893488147419103231", nil}, + {[]byte{85, 255, 255, 255, 255, 255, 255, 255, 255, 255, 127}, true, "2.5.1180591620717411303423", nil}, + {[]byte{85, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 127}, true, "2.5.19342813113834066795298815", nil}, + + {[]byte{255, 255, 255, 127}, true, "2.268435375", []uint64{2, 268435375}}, + {[]byte{0x87, 255, 255, 255, 127}, true, "2.2147483567", []uint64{2, 2147483567}}, + {[]byte{255, 127}, true, "2.16303", []uint64{2, 16303}}, + {[]byte{255, 255, 255, 255, 127}, true, "2.34359738287", []uint64{2, 34359738287}}, + {[]byte{255, 255, 255, 255, 255, 255, 255, 255, 127}, true, "2.9223372036854775727", []uint64{2, 9223372036854775727}}, + {[]byte{0x81, 255, 255, 255, 255, 255, 255, 255, 255, 127}, true, "2.18446744073709551535", []uint64{2, 18446744073709551535}}, + {[]byte{0x83, 255, 255, 255, 255, 255, 255, 255, 255, 127}, true, "2.36893488147419103151", nil}, + {[]byte{255, 255, 255, 255, 255, 255, 255, 255, 255, 127}, true, "2.1180591620717411303343", nil}, + {[]byte{255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 127}, true, "2.19342813113834066795298735", nil}, + + {[]byte{41, 0x80 | 66, 0x80 | 44, 0x80 | 11, 33}, true, "1.1.139134369", []uint64{1, 1, 139134369}}, + {[]byte{0x80 | 66, 0x80 | 44, 0x80 | 11, 33}, true, "2.139134289", []uint64{2, 139134289}}, +} + func TestOID(t *testing.T) { - var tests = []struct { - raw []byte - valid bool - str string - ints []uint64 - }{ - {[]byte{}, false, "", nil}, - {[]byte{0x80, 0x01}, false, "", nil}, - {[]byte{0x01, 0x80, 0x01}, false, "", nil}, - - {[]byte{1, 2, 3}, true, "0.1.2.3", []uint64{0, 1, 2, 3}}, - {[]byte{41, 2, 3}, true, "1.1.2.3", []uint64{1, 1, 2, 3}}, - {[]byte{86, 2, 3}, true, "2.6.2.3", []uint64{2, 6, 2, 3}}, - - {[]byte{41, 255, 255, 255, 127}, true, "1.1.268435455", []uint64{1, 1, 268435455}}, - {[]byte{41, 0x87, 255, 255, 255, 127}, true, "1.1.2147483647", []uint64{1, 1, 2147483647}}, - {[]byte{41, 255, 255, 255, 255, 127}, true, "1.1.34359738367", []uint64{1, 1, 34359738367}}, - {[]byte{42, 255, 255, 255, 255, 255, 255, 255, 255, 127}, true, "1.2.9223372036854775807", []uint64{1, 2, 9223372036854775807}}, - {[]byte{43, 0x81, 255, 255, 255, 255, 255, 255, 255, 255, 127}, true, "1.3.18446744073709551615", []uint64{1, 3, 18446744073709551615}}, - {[]byte{44, 0x83, 255, 255, 255, 255, 255, 255, 255, 255, 127}, true, "1.4.36893488147419103231", nil}, - {[]byte{85, 255, 255, 255, 255, 255, 255, 255, 255, 255, 127}, true, "2.5.1180591620717411303423", nil}, - {[]byte{85, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 127}, true, "2.5.19342813113834066795298815", nil}, - - {[]byte{255, 255, 255, 127}, true, "2.268435375", []uint64{2, 268435375}}, - {[]byte{0x87, 255, 255, 255, 127}, true, "2.2147483567", []uint64{2, 2147483567}}, - {[]byte{255, 127}, true, "2.16303", []uint64{2, 16303}}, - {[]byte{255, 255, 255, 255, 127}, true, "2.34359738287", []uint64{2, 34359738287}}, - {[]byte{255, 255, 255, 255, 255, 255, 255, 255, 127}, true, "2.9223372036854775727", []uint64{2, 9223372036854775727}}, - {[]byte{0x81, 255, 255, 255, 255, 255, 255, 255, 255, 127}, true, "2.18446744073709551535", []uint64{2, 18446744073709551535}}, - {[]byte{0x83, 255, 255, 255, 255, 255, 255, 255, 255, 127}, true, "2.36893488147419103151", nil}, - {[]byte{255, 255, 255, 255, 255, 255, 255, 255, 255, 127}, true, "2.1180591620717411303343", nil}, - {[]byte{255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 255, 127}, true, "2.19342813113834066795298735", nil}, - } - - for _, v := range tests { + for _, v := range oidTests { oid, ok := newOIDFromDER(v.raw) if ok != v.valid { - if ok { - t.Errorf("%v: unexpected success while parsing: %v", v.raw, oid) - } else { - t.Errorf("%v: unexpected failure while parsing", v.raw) - } + t.Errorf("newOIDFromDER(%v) = (%v, %v); want = (OID, %v)", v.raw, oid, ok, v.valid) continue } @@ -61,7 +61,7 @@ func TestOID(t *testing.T) { } if str := oid.String(); str != v.str { - t.Errorf("%v: oid.String() = %v, want; %v", v.raw, str, v.str) + t.Errorf("(%#v).String() = %v, want; %v", oid, str, v.str) } var asn1OID asn1.ObjectIdentifier @@ -75,33 +75,186 @@ func TestOID(t *testing.T) { o, ok := oid.toASN1OID() if shouldOk := asn1OID != nil; shouldOk != ok { - if ok { - t.Errorf("%v: oid.toASN1OID() unexpected success", v.raw) - } else { - t.Errorf("%v: oid.toASN1OID() unexpected failure", v.raw) - } + t.Errorf("(%#v).toASN1OID() = (%v, %v); want = (%v, %v)", oid, o, ok, asn1OID, shouldOk) continue } - if asn1OID != nil { - if !o.Equal(asn1OID) { - t.Errorf("%v: oid.toASN1OID(asn1OID).Equal(oid) = false, want: true", v.raw) - } + if asn1OID != nil && !o.Equal(asn1OID) { + t.Errorf("(%#v).toASN1OID() = (%v, true); want = (%v, true)", oid, o, asn1OID) } if v.ints != nil { oid2, err := OIDFromInts(v.ints) if err != nil { - t.Errorf("%v: OIDFromInts() unexpected error: %v", v.raw, err) + t.Errorf("OIDFromInts(%v) = (%v, %v); want = (%v, nil)", v.ints, oid2, err, oid) } if !oid2.Equal(oid) { - t.Errorf("%v: %#v.Equal(%#v) = false, want: true", v.raw, oid2, oid) + t.Errorf("OIDFromInts(%v) = (%v, nil); want = (%v, nil)", v.ints, oid2, oid) } } } } -func mustNewOIDFromInts(t *testing.T, ints []uint64) OID { +func TestInvalidOID(t *testing.T) { + cases := []struct { + str string + ints []uint64 + }{ + {str: "", ints: []uint64{}}, + {str: "1", ints: []uint64{1}}, + {str: "3", ints: []uint64{3}}, + {str: "3.100.200", ints: []uint64{3, 100, 200}}, + {str: "1.81", ints: []uint64{1, 81}}, + {str: "1.81.200", ints: []uint64{1, 81, 200}}, + } + + for _, tt := range cases { + oid, err := OIDFromInts(tt.ints) + if err == nil { + t.Errorf("OIDFromInts(%v) = (%v, %v); want = (OID{}, %v)", tt.ints, oid, err, errInvalidOID) + } + + oid2, err := ParseOID(tt.str) + if err == nil { + t.Errorf("ParseOID(%v) = (%v, %v); want = (OID{}, %v)", tt.str, oid2, err, errInvalidOID) + } + + var oid3 OID + err = oid3.UnmarshalText([]byte(tt.str)) + if err == nil { + t.Errorf("(*OID).UnmarshalText(%v) = (%v, %v); want = (OID{}, %v)", tt.str, oid3, err, errInvalidOID) + } + } +} + +var ( + _ encoding.BinaryMarshaler = OID{} + _ encoding.BinaryUnmarshaler = new(OID) + _ encoding.TextMarshaler = OID{} + _ encoding.TextUnmarshaler = new(OID) +) + +func TestOIDMarshal(t *testing.T) { + cases := []struct { + in string + out OID + err error + }{ + {in: "", err: errInvalidOID}, + {in: "0", err: errInvalidOID}, + {in: "1", err: errInvalidOID}, + {in: ".1", err: errInvalidOID}, + {in: ".1.", err: errInvalidOID}, + {in: "1.", err: errInvalidOID}, + {in: "1..", err: errInvalidOID}, + {in: "1.2.", err: errInvalidOID}, + {in: "1.2.333.", err: errInvalidOID}, + {in: "1.2.333..", err: errInvalidOID}, + {in: "1.2..", err: errInvalidOID}, + {in: "+1.2", err: errInvalidOID}, + {in: "-1.2", err: errInvalidOID}, + {in: "1.-2", err: errInvalidOID}, + {in: "1.2.+333", err: errInvalidOID}, + } + + for _, v := range oidTests { + oid, ok := newOIDFromDER(v.raw) + if !ok { + continue + } + cases = append(cases, struct { + in string + out OID + err error + }{ + in: v.str, + out: oid, + err: nil, + }) + } + + for _, tt := range cases { + o, err := ParseOID(tt.in) + if err != tt.err { + t.Errorf("ParseOID(%q) = %v; want = %v", tt.in, err, tt.err) + continue + } + + var o2 OID + err = o2.UnmarshalText([]byte(tt.in)) + if err != tt.err { + t.Errorf("(*OID).UnmarshalText(%q) = %v; want = %v", tt.in, err, tt.err) + continue + } + + if err != nil { + continue + } + + if !o.Equal(tt.out) { + t.Errorf("(*OID).UnmarshalText(%q) = %v; want = %v", tt.in, o, tt.out) + continue + } + + if !o2.Equal(tt.out) { + t.Errorf("ParseOID(%q) = %v; want = %v", tt.in, o2, tt.out) + continue + } + + marshalled, err := o.MarshalText() + if string(marshalled) != tt.in || err != nil { + t.Errorf("(%#v).MarshalText() = (%v, %v); want = (%v, nil)", o, string(marshalled), err, tt.in) + continue + } + + binary, err := o.MarshalBinary() + if err != nil { + t.Errorf("(%#v).MarshalBinary() = %v; want = nil", o, err) + } + + var o3 OID + if err := o3.UnmarshalBinary(binary); err != nil { + t.Errorf("(*OID).UnmarshalBinary(%v) = %v; want = nil", binary, err) + } + + if !o3.Equal(tt.out) { + t.Errorf("(*OID).UnmarshalBinary(%v) = %v; want = %v", binary, o3, tt.out) + continue + } + } +} + +func TestOIDUnmarshalBinary(t *testing.T) { + for _, tt := range oidTests { + var o OID + err := o.UnmarshalBinary(tt.raw) + + expectErr := errInvalidOID + if tt.valid { + expectErr = nil + } + + if err != expectErr { + t.Errorf("(o *OID).UnmarshalBinary(%v) = %v; want = %v; (o = %v)", tt.raw, err, expectErr, o) + } + } +} + +func BenchmarkOIDMarshalUnmarshalText(b *testing.B) { + oid := mustNewOIDFromInts(b, []uint64{1, 2, 3, 9999, 1024}) + for range b.N { + text, err := oid.MarshalText() + if err != nil { + b.Fatal(err) + } + var o OID + if err := o.UnmarshalText(text); err != nil { + b.Fatal(err) + } + } +} + +func mustNewOIDFromInts(t testing.TB, ints []uint64) OID { oid, err := OIDFromInts(ints) if err != nil { t.Fatalf("OIDFromInts(%v) unexpected error: %v", ints, err) diff --git a/src/encoding/gob/encode.go b/src/encoding/gob/encode.go index c83071c717..5f4d2539fa 100644 --- a/src/encoding/gob/encode.go +++ b/src/encoding/gob/encode.go @@ -601,7 +601,7 @@ func compileEnc(ut *userTypeInfo, building map[*typeInfo]bool) *encEngine { if ut.externalEnc == 0 && srt.Kind() == reflect.Struct { for fieldNum, wireFieldNum := 0, 0; fieldNum < srt.NumField(); fieldNum++ { f := srt.Field(fieldNum) - if !isSent(srt, &f) { + if !isSent(&f) { continue } op, indir := encOpFor(f.Type, seen, building) diff --git a/src/encoding/gob/type.go b/src/encoding/gob/type.go index 3b1dde492c..c3ac1dbd61 100644 --- a/src/encoding/gob/type.go +++ b/src/encoding/gob/type.go @@ -538,7 +538,7 @@ func newTypeObject(name string, ut *userTypeInfo, rt reflect.Type) (gobType, err idToTypeSlice[st.id()] = st for i := 0; i < t.NumField(); i++ { f := t.Field(i) - if !isSent(t, &f) { + if !isSent(&f) { continue } typ := userType(f.Type).base @@ -576,7 +576,7 @@ func isExported(name string) bool { // isSent reports whether this struct field is to be transmitted. // It will be transmitted only if it is exported and not a chan or func field // or pointer to chan or func. -func isSent(struct_ reflect.Type, field *reflect.StructField) bool { +func isSent(field *reflect.StructField) bool { if !isExported(field.Name) { return false } @@ -590,16 +590,6 @@ func isSent(struct_ reflect.Type, field *reflect.StructField) bool { return false } - // Special case for Go 1.22: the x509.Certificate.Policies - // field is unencodable but also unused by default. - // Ignore it, so that x509.Certificate continues to be encodeable. - // Go 1.23 will add the right methods so that gob can - // handle the Policies field, and then we can remove this check. - // See go.dev/issue/65633. - if field.Name == "Policies" && struct_.PkgPath() == "crypto/x509" && struct_.Name() == "Certificate" { - return false - } - return true }