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: parsing of certificates with exponent length > 32bit fails on GOARCH=386 #14129

Closed
nurio opened this issue Jan 28, 2016 · 6 comments
Labels
FrozenDueToAge v2 A language change or incompatible library change
Milestone

Comments

@nurio
Copy link

nurio commented Jan 28, 2016

1.) What version of Go are you using (go version)?
go1.6beta2
2.) What operating system and processor architecture are you using?
Architecture: 386 OS: windows
3.) What did you do?
Tried to parse a valid X.509 certificate using the public key exponent "4044784889" (0xf11684f9) with crypto/x509.ParseCertificate()
4.) What did you expect to see?
Certificate parses without error
5.) What did you see instead?
Parsing the certificate fails under Windows GOARCH=386 with "asn1: structure error: integer too large". It works correctly if compiled with GOARCH=amd64 and running on a 64bit OS.

Most certificates use 65537 as exponent which works fine, however according to NIST
"applications should be able to process RSA public keys that have any public exponent
that is an odd positive integer greater than or equal to 65537 and less than 2^256."

NIST Cryptographic Algorithms and Key Sizes for Personal Identity Verification

Simple go program containing a test certificate to reproduce the error:
test_cert_fail.go

@bradfitz
Copy link
Contributor

It reproduces in the playground: http://play.golang.org/p/sk1hcmlmBc

/cc @agl

@bradfitz bradfitz changed the title Parsing of X.509 certificates with exponent length > 32bit fails on GOARCH=386 crypto/x509: parsing of certificates with exponent length > 32bit fails on GOARCH=386 Jan 28, 2016
@bradfitz bradfitz added this to the Go1.7 milestone Jan 28, 2016
@agl
Copy link
Contributor

agl commented Jan 28, 2016

I think the bug here is that the 64-bit version works. We should be consistent between 32- and 64-bit systems. (The rsa package already checks for exponents >= 2^31 and rejects them.)

@agl agl self-assigned this Jan 28, 2016
@nurio
Copy link
Author

nurio commented Jan 29, 2016

@agl: I read your post regarding rsa exponent size and agree that there are likely no security benefits in using exponents larger than 2^31-1. However, I think it would be great if go could handle certificates with reasonably large exponents for compatibility reasons. There is crypto hardware (e.g. some athena smartcards) that generate RSA keys with larger randomly chosen exponents. That means that if they are used for issuer certificates in a PKI chain, go programs will not be able to make TLS connections to any certificates signed by these issuers.

Such certificates work with openssl (and therefore python, ruby, ...) and in current browsers (IE, Firefox, Chrome) which means that the PKI likely won't run into any problems with these certificates. And since they work "everywhere else", it might be easy for them to ignore the issue with go compatiblity.

OpenSSL limits the public exponent at 64 Bit, maybe this would be a reasonable compromise between compatibility and performance for go too?

@agl
Copy link
Contributor

agl commented Mar 10, 2016

Given that the public exponent is exposed API, and that it's an int, I think we're stuck with it on 32-bit systems.

@bradfitz
Copy link
Contributor

We could add E64 int64 and document precedence between E and E64 if we wanted.

@rsc rsc modified the milestones: Go1.8, Go1.7 May 18, 2016
@quentinmit quentinmit added the NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. label Oct 5, 2016
@rsc
Copy link
Contributor

rsc commented Nov 3, 2016

I don't think there have been enough reports about needing a fix for this to pollute the API with something like E64. Maybe for Go 2 we can think about making it an int64 for better portability.

@rsc rsc closed this as completed Nov 3, 2016
@rsc rsc added v2 A language change or incompatible library change and removed NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. labels Nov 3, 2016
@golang golang locked and limited conversation to collaborators Nov 3, 2017
@rsc rsc unassigned agl Jun 23, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge v2 A language change or incompatible library change
Projects
None yet
Development

No branches or pull requests

6 participants