encoding/asn1: support ObjectIdentifier sub-oid values greater than 2^31-1

[sebastien-rosset]

This PR adds support for ASN.1 object identifiers where at least one sub-oid has a value greater than 2^31-1. As per ASN.1 specification, each sub-oid is an ASN.1 INTEGER, which can be of arbitrary size. For example, the value of a sub-oid could be 2 << 80. In practice, scenarios with large sub-oids are rare but they do exist.

One problem is that historically, the ObjectIdentifier type has been defined as []int; this means the max value of sub-oids is math.MaxInt32. The problem leaks in multiple go packages, such as crypto.x509. This is a problem because some real-world X.509 certificates have sub-oids values greater than 2^31.

type Certificate struct {
	...
	// Policy identifiers. This can only contains sub-oids whose value is less than math.MaxInt32
	PolicyIdentifiers []asn1.ObjectIdentifier
}

This is an initial proposal using option 3 below. I should add more unit tests, run some benchmarks and optimize code, but before that it would great to know if the approach is going in the right direction.

Besides fixing the ASN.1 compliance issue for the ASN.1 unmarshaler, another issue is how to parse X.509 certificates when policy identifiers have sub-oids larger than math.MaxInt32. The fix in the x.509 package is dependent on the asn1 fix. I am including the x509 fix, let me know if it makes more sense to have two separate PRs.

Option 1

Redefine asn1.ObjectIdentifier as a []int64 or even []*big.Int. This is probably a non-starter because a lot of software has been built with this assumption. It's not possible to change the type to []int64 or []*big.Int without breaking existing software.

type ObjectIdentifier []int64

or

type ObjectIdentifier []*big.Int

Option 2

Add two new types:

type ObjectIdentifierInt64  []int64
type ObjectIdentifierBigInt []*big.Int

If all sub-oids are less than math.MaxInt32, then the return type could still be ObjectIdentifier to retain backward compatibility. If at least one sub-oid is more than math.MaxInt32, then the return value is ObjectIdentifierInt64. If at least one sub-oid is more than math.MaxInt64, then the return value is ObjectIdentifierBigInt.

One problem with this approach is it's cumbersome for consumers (such as crypto.x509) to deal with three different types.

Option 3

Add a new type that can take int, int64 or *big.Int as sub-oid values:

type ObjectIdentifierExt []interface{}

In particular, this could be used in the x509 package to support certificates that have large sub-oid values. The existing PolicyIdentifiers field cannot be removed without breaking the backward compatibility. It's doable but somewhat cumbersome to have two possible PolicyIdentifiers fields.

type Certificate struct {
	...
	// Policy identifiers. This can only contains sub-oids whose value is less than math.MaxInt32
	PolicyIdentifiers []asn1.ObjectIdentifier
        // Policy identifiers that contain sub-oid values greater than math.MaxInt32
	PolicyIdentifiersExt []asn1.ObjectIdentifierExt
}

Option 4

Use go generics:

type SignedInteger interface {
	type int, int32, int64, *big.Int
} 
type ObjectIdentifier(type T SignedInteger) []T

Developers would be able to instantiate object identifiers with different sub-types:

var oid1 ObjectIdentifier(int) = ObjectIdentifier(int){1, 2, 3}
var oid1 ObjectIdentifier(int64) = ObjectIdentifier(int64){1, 2, 1 << 60}

x509.Certificate could also be a generic type. Developers would have the choice of supporting X.509 certificates with policy identifiers that have large sub-oids:

type Certificate(type T SignedInteger) struct {
	...
	// Policy identifiers. This can only contains sub-oids whose value is less than math.MaxInt32
	PolicyIdentifiers []asn1.ObjectIdentifier(T)
}

However, this generic approach does not work for existing code because that would result in compilation failure. For example, the code below causes "cannot use generic type ObjectIdentifier(type T SignedInteger) without instantiation"

var oid1 ObjectIdentifier = ObjectIdentifier{1, 2, 3}

To get around this problem, I was wondering if a default instantiation type has been considered when in the go generic specification, something like below. This would make it possible to retrofit existing code while providing extended use cases.

type ObjectIdentifier(type T SignedInteger=int)  // similar to C++ default type notation

Or if somehow it were possible to have one generic version and one non-generic version:

type ObjectIdentifier(type T SignedInteger) []T
type ObjectIdentifier []int

One little annoyance is that even with generics, it's not possible to use common arithmetic operators (+, *, /) that work across int, int64 and *big.Int.

But even if there is a way to use generics without breaking code, it would seem odd to be required to expose the sub-oid generic type to all types (e.g. x509.Certificate) that use ObjectIdentifier.

Current use of asn1.ObjectIdentifier type

Below are the publicly exposed asn1.ObjectIdentifier fields in the golang/go repo:

1. encoding/asn1
2. crypto/x509/pkix package has 4 exported fields using asn1.ObjectIdentifier
3. crypto/x509 package:
   1. ECDSA keys
   2. Used in the x509.Certificate struct:

// A Certificate represents an X.509 certificate.
type Certificate struct {
  ...
  UnhandledCriticalExtensions []asn1.ObjectIdentifier
  UnknownExtKeyUsage []asn1.ObjectIdentifier
  PolicyIdentifiers []asn1.ObjectIdentifier
}

For #38737

[googlebot]

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here with @googlebot I signed it! and we'll verify it.

---

What to do if you already signed the CLA

Individual signers

• It's possible we don't have your GitHub username or you're using a different email address on your commit. Check your existing CLA data and verify that your email is set on your git commits.

Corporate signers

• Your company has a Point of Contact who decides which employees are authorized to participate. Ask your POC to be added to the group of authorized contributors. If you don't know who your Point of Contact is, direct the Google project maintainer to go/cla#troubleshoot (Public version).
• The email used to register you as an authorized contributor must be the email used for the Git commit. Check your existing CLA data and verify that your email is set on your git commits.
• The email used to register you as an authorized contributor must also be attached to your GitHub account.

ℹ️ Googlers: Go here for more info.

[sebastien-rosset]

@googlebot I signed it!

[googlebot]

CLAs look good, thanks!

ℹ️ Googlers: Go here for more info.

[gopherbot]

This PR (HEAD: 35628c4) has been imported to Gerrit for code review.

Please visit https://go-review.googlesource.com/c/go/+/240006 to see it.

Tip: You can toggle comments from me using the comments slash command (e.g. /comments off)
See the Wiki page for more info

[gopherbot]

Message from Gobot Gobot:

Patch Set 1:

Congratulations on opening your first change. Thank you for your contribution!

Next steps:
A maintainer will review your change and provide feedback. See
https://golang.org/doc/contribute.html#review for more info and tips to get your
patch through code review.

Most changes in the Go project go through a few rounds of revision. This can be
surprising to people new to the project. The careful, iterative review process
is our way of helping mentor contributors and ensuring that their contributions
have a lasting impact.

During May-July and Nov-Jan the Go project is in a code freeze, during which
little code gets reviewed or merged. If a reviewer responds with a comment like
R=go1.11 or adds a tag like "wait-release", it means that this CL will be
reviewed as part of the next development cycle. See https://golang.org/s/release
for more details.

---

Please don’t reply on this GitHub thread. Visit golang.org/cl/240006.
After addressing review feedback, remember to publish your drafts!

[gopherbot]

This PR (HEAD: 2663d81) has been imported to Gerrit for code review.

Please visit https://go-review.googlesource.com/c/go/+/240006 to see it.

Tip: You can toggle comments from me using the comments slash command (e.g. /comments off)
See the Wiki page for more info

[gopherbot]

This PR (HEAD: 5f541c3) has been imported to Gerrit for code review.

Please visit https://go-review.googlesource.com/c/go/+/240006 to see it.

Tip: You can toggle comments from me using the comments slash command (e.g. /comments off)
See the Wiki page for more info

[gopherbot]

Message from Go Bot:

Patch Set 1:

Congratulations on opening your first change. Thank you for your contribution!

Next steps:
A maintainer will review your change and provide feedback. See
https://golang.org/doc/contribute.html#review for more info and tips to get your
patch through code review.

Most changes in the Go project go through a few rounds of revision. This can be
surprising to people new to the project. The careful, iterative review process
is our way of helping mentor contributors and ensuring that their contributions
have a lasting impact.

During May-July and Nov-Jan the Go project is in a code freeze, during which
little code gets reviewed or merged. If a reviewer responds with a comment like
R=go1.11 or adds a tag like "wait-release", it means that this CL will be
reviewed as part of the next development cycle. See https://golang.org/s/release
for more details.

---

Please don’t reply on this GitHub thread. Visit golang.org/cl/240006.
After addressing review feedback, remember to publish your drafts!

[zzo38]

I think it would be better (although I don't know if it can easily be changed; it is not a programming language that I use) to store a OID as the string of bytes that it is encoded as, and use that for comparison. Decoding it into the sequence of numbers is only necessary for display or conversion, not for comparison (if you need to compare a dotted decimal OID with a binary OID, it is better to encode the dotted decimal OID to binary instead of the other way around).

Another alternative (which might avoid needing to change the types) might be to use negative numbers to represent OIDs that cannot be represented with the existing implementation; one possible way to do this would be, if a part of the OID cannot be decoded, use the bitwise complement of each individual byte for the rest of the OID. This has the disavantage that OIDs that can be decoded in the usual way have an alternate representation which encodes into the same thing, and this might be undesirable.