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: IsEncryptedPEMBlock returns false on valid encrypted keys. ParseRawPrivateKeyWithPassphrase fails on PKCS8 format encrypted key. #41949

Closed
HarikrishnanBalagopal opened this issue Oct 13, 2020 · 8 comments
Labels
Documentation FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Milestone

Comments

@HarikrishnanBalagopal
Copy link

HarikrishnanBalagopal commented Oct 13, 2020

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

$ go version
go version go1.15.2 darwin/amd64

Does this issue reproduce with the latest release?

YES

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

go env Output
$ go env
GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOCACHE="/Users/harikrishnanbalagopal/Library/Caches/go-build"
GOENV="/Users/harikrishnanbalagopal/Library/Application Support/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOINSECURE=""
GOMODCACHE="/Users/harikrishnanbalagopal/go/pkg/mod"
GONOPROXY=""
GONOSUMDB=""
GOOS="darwin"
GOPATH="/Users/harikrishnanbalagopal/go"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/usr/local/Cellar/go/1.15.2/libexec"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/usr/local/Cellar/go/1.15.2/libexec/pkg/tool/darwin_amd64"
GCCGO="gccgo"
AR="ar"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
GOMOD="/Users/harikrishnanbalagopal/go/src/github.com/konveyor/testkey/go.mod"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/09/5yjxv27n6njfskvvmkv9v8m40000gn/T/go-build525969776=/tmp/go-build -gno-record-gcc-switches -fno-common"
GOROOT/bin/go version: go version go1.15.2 darwin/amd64
GOROOT/bin/go tool compile -V: compile version go1.15.2
uname -v: Darwin Kernel Version 19.6.0: Thu Jun 18 20:49:00 PDT 2020; root:xnu-6153.141.1~1/RELEASE_X86_64
ProductName:	Mac OS X
ProductVersion:	10.15.6
BuildVersion:	19G2021
lldb --version: lldb-1103.0.22.10
Apple Swift version 5.2.4 (swiftlang-1103.0.32.9 clang-1103.0.32.53)

What did you do?

Example with the command to use to generate the keys and the keys themselves:
https://play.golang.org/p/F2nUIO_S6hT

Called x509.IsEncryptedPEMBlock on pem.Blocks created using pem.Decode.
pem.Decode is called on valid encrypted RSA private keys generated using the following commands:
ssh-keygen -m PEM -t rsa -b 4096 -C 'foobar@example.com'
ssh-keygen -m PKCS8 -t rsa -b 4096 -C 'foobar@example.com'
ssh-keygen -m RFC4716 -t rsa -b 4096 -C 'foobar@example.com'

Also called ssh.ParseRawPrivateKeyWithPassphrase on each of those keys.

What did you expect to see?

The x509.IsEncryptedPEMBlock function should report true in all the cases given in the example.
The ssh.ParseRawPrivateKeyWithPassphrase should succeed on the PKCS8 key instead of failing as it does in the example.

Note that ssh-keygen -yf mykey is able to detect that the file is a valid encrypted key and decrypt it given the correct password in all the 3 cases. So IsEncryptedPEMBlock and ParseRawPrivateKeyWithPassphrase should be able to handle them as well.

What did you see instead?

x509.IsEncryptedPEMBlock incorrectly returns false when given the pem.Blocks of the PKCS8 and RFC4716 keys.

ssh-keygen lets you specify the format for the key file using the -m flag:
https://www.man7.org/linux/man-pages/man1/ssh-keygen.1.html
There are 3 supported formats: PEM, PKCS8 and RFC4716. x509.IsEncryptedPEMBlock only reports correctly on keys generated using PEM. This is because keys generated using PKCS8 and RFC4716 no longer have headers that indicate that the data is encrypted and the decryption algorithm to use. x509.IsEncryptedPEMBlock checks for those headers in order to determine whether the data is encrypted:

func IsEncryptedPEMBlock(b *pem.Block) bool {
_, ok := b.Headers["DEK-Info"]
return ok
}

Interestingly the ssh.ParseRawPrivateKeyWithPassphrase function fails on PKCS8 but is able to handle RFC4716 because of this special case: https://github.com/golang/crypto/blob/master/ssh/keys.go#L1156-L1158

I have tried the example with go version go1.15.2 darwin/amd64 and the latest golang.org/x/crypto v0.0.0-20201012173705-84dcc777aaee on my Macbook Pro macOS Catalina 10.15.6

@networkimprov
Copy link

cc @FiloSottile

@HarikrishnanBalagopal
Copy link
Author

HarikrishnanBalagopal commented Oct 15, 2020

Related issue #8860

I wrote a small example showing how we can get the decryption algorithm info from the encrypted PEM:
https://play.golang.org/p/FOgT9XUbwBM
I think this could be added to pem.Decode so it sets the correct headers.

We can set the DEK-Info header using this info and then we can use https://golang.org/pkg/crypto/x509/#DecryptPEMBlock to decrypt it as usual.

Update: nvm, the x509.DecryptPEMBlock function uses a custom key derivation function:

// deriveKey uses a key derivation function to stretch the password into a key
// with the number of bits our cipher requires. This algorithm was derived from
// the OpenSSL source.
func (c rfc1423Algo) deriveKey(password, salt []byte) []byte {
hash := md5.New()
out := make([]byte, c.keySize)
var digest []byte
for i := 0; i < len(out); i += len(digest) {
hash.Reset()
hash.Write(digest)
hash.Write(password)
hash.Write(salt)
digest = hash.Sum(digest[:0])
copy(out[i:], digest)
}
return out
}

It also can't accept a separate salt. It just takes the first 8 bytes of the IV as the salt.
https://github.com/golang/go/blob/master/src/crypto/x509/pem_decrypt.go#L141

@toothrot toothrot changed the title Bug: IsEncryptedPEMBlock returns false on valid encrypted keys. ParseRawPrivateKeyWithPassphrase fails on PKCS8 format encrypted key. crypto/x509: IsEncryptedPEMBlock returns false on valid encrypted keys. ParseRawPrivateKeyWithPassphrase fails on PKCS8 format encrypted key. Oct 15, 2020
@toothrot toothrot added this to the Backlog milestone Oct 15, 2020
@toothrot toothrot added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Oct 15, 2020
@HarikrishnanBalagopal
Copy link
Author

HarikrishnanBalagopal commented Oct 15, 2020

@FiloSottile @toothrot I just created a quick and dirty version of how it could detect and decrypt the private key:
HarikrishnanBalagopal@3db5714

One major issue is that PKCS8 format encrypted private keys use PBKDF2 as the key derivation function:
https://tools.ietf.org/html/rfc8018#appendix-A.4

Since PBKDF2 is an extension it can't be used to implement the standard library functions.
https://pkg.go.dev/golang.org/x/crypto/pbkdf2

Another issue is how to pass the information about which KDF to use to the DecryptPEMBlock function.
For that I used Key-Info as mentioned here: https://www.freesoft.org/CIE/RFC/1421/19.htm
I wasn't able to find much about which headers are acceptable from the RFC: https://tools.ietf.org/html/rfc7468#section-2

   Unlike legacy PEM encoding [RFC1421], OpenPGP ASCII armor, and the
   OpenSSH key file format, textual encoding does *not* define or permit
   headers to be encoded alongside the data.  Empty space can appear
   between the pre-encapsulation boundary and the base64, but generators
   SHOULD NOT emit such any such spacing.  (The provision for this empty
   area is a throwback to PEM, which defined an "encapsulated header
   portion".)

Note: Even if DecryptPEMBlock can't be made to support this key format because PBKDF2 is an extension,

  • at least IsEncryptedPEMBlock should be able to detect that it is in fact an encrypted private key the way I have implemented it.
  • and ssh.ParseRawPrivateKeyWithPassphrase should still be able to support this key format since ssh is already an extension.

@FiloSottile
Copy link
Contributor

IsEncryptedPEMBlock, DecryptPEMBlock and EncryptPEMBlock don't refer generally to any encrypted format (like PKCS#8) encoded as PEM, but specifically to RFC 1423 PEM encryption. That encryption format is legacy and broken by design, so we should deprecate it, not mix it with newer formats that encourage its use.

We can address this confusion with better docs in the deprecation message.

@gopherbot
Copy link

Change https://golang.org/cl/263181 mentions this issue: crypto/x509: change documentation to reflect that PEM encryption refers to a legacy standard that is to be deprecated soon

@HarikrishnanBalagopal
Copy link
Author

HarikrishnanBalagopal commented Oct 17, 2020

@FiloSottile I submitted a PR to add a warning in the docs for each of those 3 functions.
I think ssh.ParseRawPrivateKeyWithPassphrase still needs to be fixed to handle PKCS8 but that is on a completely different repo so I linked the PR to close the issue.

Update: Actually reading this https://golang.org/doc/contribute.html#ref_issues it seems the issues on extensions are also tracked on this repo. I have changed the PR comment to Updates #41949

HarikrishnanBalagopal added a commit to HarikrishnanBalagopal/go that referenced this issue Oct 17, 2020
The existing documentation does not mention the exact meaning of
"PEM encryption". So add a note to clarify that it is referring to
RFC 1423 and that the functions are not meant to support any newer
standard like PKCS golang#8.

Updates golang#41949
@FiloSottile FiloSottile self-assigned this Oct 20, 2020
@FiloSottile FiloSottile added Documentation NeedsFix The path to resolution is known, but the work has not been done. and removed NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. labels Oct 20, 2020
@FiloSottile FiloSottile modified the milestones: Backlog, Go1.16 Oct 20, 2020
@gopherbot
Copy link

Change https://golang.org/cl/264159 mentions this issue: crypto/x509: deprecate legacy PEM encryption

@HarikrishnanBalagopal
Copy link
Author

@FiloSottile but what about ssh.ParseRawPrivateKeyWithPassphrase ?
ssh is an extension and should be able to support PKCS8 encrypted private keys using https://pkg.go.dev/golang.org/x/crypto/pbkdf2

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Documentation FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Development

No branches or pull requests

5 participants