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 <r@golang.org>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Roland Shoemaker <roland@golang.org>
Auto-Submit: Roland Shoemaker <roland@golang.org>

diff --git a/api/next/66249.txt b/api/next/66249.txt
new file mode 100644 (file)
index 0000000..f9d7a1a
--- /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 (file)
index 0000000..5b1d982
--- /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 5359af624b2713966c46e34b63e3bc0c852a20ed..b00c35e6967ae24221556165070bd2b0a79ce7c7 100644 (file)
--- 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 eb47244a7325205e34c1776e7b8e31478e80bdf3..cbb3406424656b818736ec4edad12fd850d0ad82 100644 (file)
--- 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"
 )
 
-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},
-       }
+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},
 
-       for _, v := range tests {
+       {[]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) {
+       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 c83071c717278570548e584f4be8565f50065587..5f4d2539faafe3c783ac1102e3546cc45064ee71 100644 (file)
--- 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 3b1dde492c28d89e88626738a107350724ca6da3..c3ac1dbd61fd51b245a433eae445251fd3a539bb 100644 (file)
--- 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
 }