TOOLS-2013: Support PKCS #8 encrypted client private keys #64
Conversation
| decrypted, err := pkcs8.ParsePKCS8PrivateKey(currentBlock.Bytes, []byte(keyPasswd)) | ||
| if err != nil { | ||
| return "", err | ||
| } |
There was a problem hiding this comment.
From my understanding, there's a couple ways to encrypt a PEM key in PKCS 8 format. The only one that the pkcs8 Go package can currently handle is called the PKCS 5 v2.0 scheme. The first time I tried this, I accidentally used the wrong scheme and got this error:
error configuring the connector: error configuring client, can't load client certificate: pkcs8: only PKCS #5 v2.0 supported
Even though the error is probably specific enough, I'm wondering if we should make a note of this in the docs under the --sslPEMKeyFile entry and provide guidance on getting the right scheme (e.g. using openssl with the -v2 des3 option to get PKCS 5 v2.0).
I'm okay either way, but curious to hear others' thoughts.
There was a problem hiding this comment.
Nice catch! I agree we should add this limitation to the doc so that it won't cause confusion to the client.
There was a problem hiding this comment.
I'm not sure that this is a limitation significant enough to warrant a mention in the docs. Generally, the docs describe what we do support, and I think a limitation has to be pretty notable/important for us to want to describe what we don't do.
| decrypted, err := pkcs8.ParsePKCS8PrivateKey(currentBlock.Bytes, []byte(keyPasswd)) | ||
| if err != nil { | ||
| return "", err | ||
| } |
There was a problem hiding this comment.
Nice catch! I agree we should add this limitation to the doc so that it won't cause confusion to the client.
There was a problem hiding this comment.
LGTM.
addClientCertFromBytes() is currently replicated in mtc because we are waiting on the go driver to implement GODRIVER-1588. But the eventual goal is to move back to using the version of addClientCertFromBytes() that exists in the go driver. So we probably want to open a go driver ticket to add PKCS#8 support there too.
| @@ -48,3 +48,7 @@ | |||
| go-tests = true | |||
| unused-packages = true | |||
|
|
|||
|
|
|||
| [[constraint]] | |||
| name = "github.com/youmark/pkcs8" | |||
There was a problem hiding this comment.
Note: we need to update the THIRD-PARTY-NOTICES file in mirror and sqlproxy once we revendor mtc.
There was a problem hiding this comment.
@edobranov could you file a ticket for this?
There was a problem hiding this comment.
Yep, filed TOOLS-2787.
db/db.go
Outdated
| buf, err := x509.DecryptPEMBlock(currentBlock, []byte(keyPasswd)) | ||
| if err != nil { | ||
| return "", err | ||
| isEncrypted := x509.IsEncryptedPEMBlock(currentBlock) || strings.Contains(currentBlock.Type, "ENCRYPTED") |
There was a problem hiding this comment.
Nit: I think it's better to check for equality against the full "ENCRYPTED PRIVATE KEY" string here. It's better to be precise.
There's a Go driver ticket linked to the tools one from a while back (GODRIVER-354). |
|
It might be worth talking to the go driver team to see if they think the solution here would work for them and if they're open to you contributing this. |
|
I'm removing myself as a reviewer on this PR, since there are already two other LGTMs, and I don't think my review needs to be a blocker here! |
Jira: https://jira.mongodb.org/browse/TOOLS-2013
This adds the ability to decrypt PKCS 8 encrypted PEM keys.
The assumption is that these types of keys are the only format possible in PEM blocks labeled
-----BEGIN ENCRYPTED PRIVATE KEY-----. I wasn't able to find any place that explicitly states this, though the PKCS 8 documentation mentions anEncryptedPrivateKeyInfotype, which looks to correspond to that label. So I believe it's unique to PKCS 8.There's one caveat that I'll make a comment about below.