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>

6 files changed, 329 insertions, 66 deletions

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"
 )
 
-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 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
 }