Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

crypto/x509, encoding/asn1: ObjectIdentifier and ParseCertificate do not support int > 31 bits, preventing support of OID 2.25 (/UUID) (follow-up on #19933) #30757

Open
vpelletier opened this issue Mar 12, 2019 · 3 comments
Labels
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone

Comments

@vpelletier
Copy link

Commit 40436 extends support for OIDs from original 28 bits to 31 bits as a result of #19933 . But this is insufficient to support the 2.25 OID subtree, which is AFAIK the only place in the tree one can get registration-less OIDs, and it mandates them to be 128bits-big. So certificates issued with such OIDs get rejected by go application (caddy, in my case, acting as an HTTP proxy with proxy target being served with such certificate).

@agl : The above was intended as a response to your post on #19933 . I would have happily posted there if it was not blocked. I would happily have this report deleted/closed if the original one can be reopened.

@julieqiu julieqiu changed the title encoding/asn1: ObjectIdentifier + crypto/x509.ParseCertificate does not support int > 31 bits, preventing support of OID 2.25 (/UUID) (follow-up on #19933) crypto/x509, encoding/asn1: encoding/asn1.ObjectIdentifier and crypto/x509.ParseCertificate do not support int > 31 bits, preventing support of OID 2.25 (/UUID) (follow-up on #19933) Mar 12, 2019
@julieqiu julieqiu added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Mar 12, 2019
@julieqiu
Copy link
Member

cc: @FiloSottile @agl

@mholt
Copy link

mholt commented Mar 13, 2019

We'd be happy to contribute a patch for this -- but before we begin working on that, it's unclear to me why using a BigInt was rejected already:

(quoting @agl from #19933 (comment))

We would not change the type from int to something larger

ASN.1 isn't my specialty so forgive my naivety here. :) Any more background or explanation for this would be appreciated!

@cam-miller
Copy link

We'd be happy to contribute a patch for this -- but before we begin working on that, it's unclear to me why using a BigInt was rejected already:

(quoting @agl from #19933 (comment))

We would not change the type from int to something larger

ASN.1 isn't my specialty so forgive my naivety here. :) Any more background or explanation for this would be appreciated!

I think the general risk is that atomicity is not guaranteed when using a type larger than an Int32 on 32 bit platforms. This is explained reasonably well here - although I'm not too confident in my understanding of the underlying architecture of Go to be able to say whether or not the theory applies here.

https://preshing.com/20130618/atomic-vs-non-atomic-operations/

At a glance I would not consider the impact of this risk to be significant in this case - although a little more research is required to support that opinion. While there is some risk that changing int32 to int64 will introduce some instability in the case of concurrent writes, not being able to handle certificates using large, but standards-conformant OID nodes could also be considered fundamentally unstable.

@odeke-em odeke-em changed the title crypto/x509, encoding/asn1: encoding/asn1.ObjectIdentifier and crypto/x509.ParseCertificate do not support int > 31 bits, preventing support of OID 2.25 (/UUID) (follow-up on #19933) crypto/x509, encoding/asn1: ObjectIdentifier and ParseCertificate do not support int > 31 bits, preventing support of OID 2.25 (/UUID) (follow-up on #19933) Apr 29, 2020
@seankhliao seankhliao added this to the Unplanned milestone Aug 20, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Development

No branches or pull requests

5 participants