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

PFX container with chain leads to tls: private key does not match public key #68

Closed
rgershkovich opened this issue Oct 15, 2020 · 2 comments · Fixed by #81
Closed

PFX container with chain leads to tls: private key does not match public key #68

rgershkovich opened this issue Oct 15, 2020 · 2 comments · Fixed by #81

Comments

@rgershkovich
Copy link

rgershkovich commented Oct 15, 2020

Hello,

First of all, thank you for the utility, it's very useful 🙏

Unfortunately, we encountered an issue which I am not able to solve properly.
We are using ACME-PS to issue Let's Encrypt certificates and put them into Azure Key Vault. From there certs are synced via azure-key-vault-agent to a number of Linux hosts.
ACME-PS v1.2 produces PFX containers with:

  • server private key
  • server certificate/public key
  • Let's Encrypt Intermediate CA (Authority X3 at the moment) certificate/public key

Trying to get '{{ index .Secrets "pem-test" | privateKey }}' from such PFX container fails in this function:

certAndKey, err := tls.X509KeyPair(pemBytes, pemBytes)

with:

tls: private key does not match public key

After some experiments I found out that the issue seems to occur because tls.X509KeyPair does not work well when there are more than 1 certificate present in pemBytes. It doesn't try to find common modulus between a key and all certs e.g.; it just grabs some previously extracted [first?] cert, and if it doesn't match the private key – fails. I see that the code in akva follows this example, but it doesn't work well for ACME-PS LE PFX certs, since intermediate CA is always the first one in pemData after server private key.

I am not sure why this is happening in golang, as openssl pkcs12 produces proper order (server cert, intermediate cert). I was able to hack it temporarily by changing the order manually:

var pemData []byte
  pemData = append(pemData, pem.EncodeToMemory(blocks[0])...) // private_key
  pemData = append(pemData, pem.EncodeToMemory(blocks[2])...) // server_crt
  pemData = append(pemData, pem.EncodeToMemory(blocks[1])...) // intermediate_ca_crt

... but this is obviously not a good solution.

I would be grateful for a proper fix or recommendation how to get around this issue.

Thank you for your time!

@grep-ir
Copy link
Contributor

grep-ir commented Oct 16, 2020

We really appreciate your interest in our project and the amount of detail provided in this issue!

Digging into things a bit, it looks like your hunch is correct, the tls.X509KeyPair function assumes that the first certificate block it encounters is what should be validated against the private key: https://golang.org/src/crypto/tls/tls.go?#L325.

Despite the TLS 1.3 RFC being a little more lenient on ordering of issuers, it still states that the leaf (end_entity) certificate must be first.

Note: Prior to TLS 1.3, "certificate_list" ordering required each
   certificate to certify the one immediately preceding it; however,
   some implementations allowed some flexibility.  Servers sometimes
   send both a current and deprecated intermediate for transitional
   purposes, and others are simply configured incorrectly, but these
   cases can nonetheless be validated properly.  For maximum
   compatibility, all implementations SHOULD be prepared to handle
   potentially extraneous certificates and arbitrary orderings from any
   TLS version, with the exception of the end-entity certificate which
   MUST be first.

https://tools.ietf.org/html/rfc8446#section-4.4.2

This to me would affirm the tls.X509KeyPair function's assumption that cert.Certificate[0] should match the private key.

While this is not directly related to the implementation of pkcs12, many web servers consume pkcs12 files as input and extend this assumption of ordering to the pkcs12 object as well.

The best course of action in my estimation would be to open an issue with https://github.com/PKISharp/ACME-PS to correct the certificate ordering upon pkcs12 creation. If this proves ineffective we can look at attempting to sort the PEM blocks ourselves.

Thanks

@markercm
Copy link
Contributor

We ran into the same issue using https://github.com/shibayan/keyvault-acmebot so I am hoping we can maybe have AKVA sort the PEM blocks.

drewby08 pushed a commit that referenced this issue May 25, 2021
drewby08 added a commit that referenced this issue May 26, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants