crypto/x509: IsEncryptedPEMBlock returns false on valid encrypted keys. ParseRawPrivateKeyWithPassphrase fails on PKCS8 format encrypted key.

[HarikrishnanBalagopal]

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:

go/src/crypto/x509/pem_decrypt.go

Lines 99 to 102 in 5b509d9

	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]

cc @FiloSottile

[HarikrishnanBalagopal]

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:

go/src/crypto/x509/pem_decrypt.go

Lines 79 to 96 in 8cd75f3

	// 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

[HarikrishnanBalagopal]

@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]

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]

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]

@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