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/tls: X509KeyPair cannot parse OpenSSL generated ECDSA keys #23591

Closed
chandra-ghub opened this issue Jan 28, 2018 · 5 comments
Closed

crypto/tls: X509KeyPair cannot parse OpenSSL generated ECDSA keys #23591

chandra-ghub opened this issue Jan 28, 2018 · 5 comments
Assignees
Milestone

Comments

@chandra-ghub
Copy link

@chandra-ghub chandra-ghub commented Jan 28, 2018

Please answer these questions before submitting your issue. Thanks!

I am trying to get go's standard tls package accept my openssl generated keys (Prime 256 curve ECDSA). Generated key and cert using OpenSSL -

# openssl ecparam -name prime256v1 -genkey -noout -out priv.pem -param_enc explicit
# openssl req -new -x509 -key priv.pem -out EC_server.pem -days 365

What version of Go are you using (go version)?

Tested with 1.9 and on play.golang.org too.
https://play.golang.org/p/V9GzXsn1zoG (copied the function X509KeyPair() to be able to debug)
https://play.golang.org/p/22G1XNbU8at (direct call to the library)

Does this issue reproduce with the latest release?

Yes

What operating system and processor architecture are you using (go env)?

MACOS High Sierra

What did you do?

here you go -
https://play.golang.org/p/V9GzXsn1zoG with the library copied to debug the error
https://play.golang.org/p/22G1XNbU8at (without go's library code)

What did you expect to see?

A &tls.Certificate{} to have returned.

What did you see instead?

failed to parse private key error from private function parsePrivateKey() in tls.go library.

@odeke-em odeke-em changed the title tls.X509KeyPair() does not parse OpenSSL generated ECDSA keys crypto/tls: x509KeyPair cannot parse OpenSSL generated ECDSA keys Jan 28, 2018
@odeke-em

This comment has been minimized.

Copy link
Member

@odeke-em odeke-em commented Jan 28, 2018

Thank for reporting this issue @chandra-ghub.

Seems like the certificate centric version of this bug is #21502, but am not sure.

I'll tag some crypto folks @agl @titanous @FiloSottile

@odeke-em odeke-em changed the title crypto/tls: x509KeyPair cannot parse OpenSSL generated ECDSA keys crypto/tls: X509KeyPair cannot parse OpenSSL generated ECDSA keys Jan 28, 2018
@bradfitz bradfitz added this to the Go1.11 milestone Jan 28, 2018
@agl

This comment has been minimized.

Copy link
Contributor

@agl agl commented Jan 28, 2018

Drop this: -param_enc explicit

It causes OpenSSL to encode P-256 as if it were a random, arbitrary curve rather than specifying P-256 itself. The default (named_curve) is correct.

(For more details, see the OpenSSL documentation.)

@FiloSottile

This comment has been minimized.

Copy link
Member

@FiloSottile FiloSottile commented Jan 28, 2018

As #21502 (comment) says, -param_enc explicit is not legal for TLS certificates anyway.

I looked into making the error message more useful like in #21502 (which is indeed the same issue but with certificates) but parsePrivateKey does trial decryption of 3 formats, so we would have to guess at which is an error and which is a mismatch. I don't love that.

What we can do is move certificate parsing, which has more useful error messages, above private key parsing in X509KeyPair. I'll CL that.

@gopherbot

This comment has been minimized.

Copy link

@gopherbot gopherbot commented Jan 28, 2018

Change https://golang.org/cl/90435 mentions this issue: crypto/tls: parse certificate first in X509KeyPair to get better errors

@chandra-ghub

This comment has been minimized.

Copy link
Author

@chandra-ghub chandra-ghub commented Jan 29, 2018

thanks @agl and @FiloSottile for your time. Can confirm that dropping -param_enc explicit from openssl's flags does the trick - https://play.golang.org/p/MS_FQ8cqqA8

Appreciate your acute diligence on the error messages, makes life so much easier for tls users going forward.

gopherbot pushed a commit that referenced this issue Mar 27, 2018
parsePrivateKey can't return useful error messages because it does trial
decoding of multiple formats.  Try ParseCertificate first in case it
offers a useful error message.

Fixes #23591

Change-Id: I380490a5850bee593a7d2f584a27b2a14153d768
Reviewed-on: https://go-review.googlesource.com/90435
Run-TryBot: Filippo Valsorda <filippo@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Adam Langley <agl@golang.org>
FiloSottile added a commit to FiloSottile/go that referenced this issue Oct 12, 2018
parsePrivateKey can't return useful error messages because it does trial
decoding of multiple formats.  Try ParseCertificate first in case it
offers a useful error message.

Fixes golang#23591

Change-Id: I380490a5850bee593a7d2f584a27b2a14153d768
Reviewed-on: https://go-review.googlesource.com/90435
Run-TryBot: Filippo Valsorda <filippo@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Adam Langley <agl@golang.org>
FiloSottile added a commit to FiloSottile/go that referenced this issue Oct 12, 2018
parsePrivateKey can't return useful error messages because it does trial
decoding of multiple formats.  Try ParseCertificate first in case it
offers a useful error message.

Fixes golang#23591

Change-Id: I380490a5850bee593a7d2f584a27b2a14153d768
Reviewed-on: https://go-review.googlesource.com/90435
Run-TryBot: Filippo Valsorda <filippo@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Adam Langley <agl@golang.org>
@golang golang locked and limited conversation to collaborators Jan 29, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
6 participants
You can’t perform that action at this time.