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

Stop using deprecated x509.EncryptPEMBlock #102

Open
k4leung4 opened this issue Apr 8, 2022 · 9 comments
Open

Stop using deprecated x509.EncryptPEMBlock #102

k4leung4 opened this issue Apr 8, 2022 · 9 comments
Labels
enhancement New feature or request

Comments

@k4leung4
Copy link
Contributor

k4leung4 commented Apr 8, 2022

Description

As noted in the documentation, https://pkg.go.dev/crypto/x509#EncryptPEMBlock

Deprecated: Legacy PEM encryption as specified in [RFC 1423](https://rfc-editor.org/rfc/rfc1423.html) is insecure by design. Since it does not authenticate the ciphertext, it is vulnerable to padding oracle attacks that can let an attacker recover the plaintext.

https://github.com/sigstore/scaffolding/blob/main/cmd/fulcio/createcerts/main.go#L172

@k4leung4 k4leung4 added the enhancement New feature or request label Apr 8, 2022
@vaikas
Copy link
Contributor

vaikas commented Apr 12, 2022

I'd be happy to fix this, since this is for test code, I'm curious how important it is atm.
I poked a bit and having a hard time finding a suitable replacement without going to external libraries:
golang/go#8860

@k4leung4
Copy link
Contributor Author

k4leung4 commented Apr 12, 2022

I don't have a sense of how important this is.
With us using scaffolding for bringing up production infrastructure rather than just for testing purposes, we might want to review these things to ensure that it is something that we are comfortable using for production environments.

@vaikas
Copy link
Contributor

vaikas commented Apr 14, 2022

Yes! Makes sense. I guess we need to chase down what a suitable replacement is.

@haydentherapper
Copy link
Contributor

@k4leung4 A couple questions for this -

  • Where is createAll used?
  • Is this for setting up a CA that's backed by an on-disk private key?
  • Does this run regardless of the type of CA backend that's being used (like KMS/CA Service)? If so, we should revisit this, because we shouldn't generate unused keys.

On-disk signing keys are not as secure as a remote signer, so I want to make sure this isn't the default. I've also been digging into this a bit recently - EncryptPEMBlock is deprecated because there's a certain attack that's possible with its weak encryption scheme. One option are PKCS#8 encrypted keys, what Ville linked, which are still vulnerable to the same attack but use a stronger PBKDF function that makes brute force harder. The other option is a better encryption algorithm like AES-GCM, but it's harder to generate an encrypted key using available tooling.

@k4leung4
Copy link
Contributor Author

@haydentherapper

This not used for staging or production, as it is only used when the certificate authority is set to fileca
For sigstore staging and production, we use kmsca, which does not use createcerts job.

@haydentherapper
Copy link
Contributor

Sweet, thanks for confirming. I'm looking at dropping support for RFC1423 keys in Fulcio, so I may need to import a third-party library for PKCS#8 key generation in Scaffolding. Any concerns?

@k4leung4
Copy link
Contributor Author

the main use case for this at the moment is e2e testing i think.
no concerns from me as long as we have a way to run e2e tests.

@bobcallaway
Copy link
Member

@priyawadhwa is this done now with #1310 ?

@priyawadhwa
Copy link
Contributor

I think so -- I replaced it everywhere I could, but couldn't make the change in ctlog createcerts because I don't think the ctlog supports using PKCS#8 keys

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

5 participants