crypto/x509: insufficient validation in x509.DecryptPEMBlock leads to decryption false positives

[diogomonica]

The validation of x509.DecryptPEMBloc seems to be insufficient and any password that matches the padding validations in (https://golang.org/src/crypto/x509/pem_decrypt.go) provides a non-error return.

The following code shows two different passwords decoding the same encrypted PEM file successfully. The correct password for the encrypted PEM file in privateKeyString is "omgomgponies".

package main

import (
    "crypto/x509"
    "encoding/pem"
    "fmt"
)

var privateKeyString = `
-----BEGIN RSA PRIVATE KEY-----
Proc-Type: 4,ENCRYPTED
DEK-Info: DES-EDE3-CBC,68CA28419ADC598B

0ro1rMiglCY/LVpL4nC3/+JvfcmE84nx317uz1+uXzu5Y+OXxekejTWZrk7hWBeA
chl8anQM8zPEZ+sJ13qMWH+1lUbbd/5ZsCpOlERkgDrut8yHEKkRFRj5MQtrg1Vn
eEpd0B7P2UD1ctJb0T8uBo1hhzFyxDI8sG47fSYU/oTnBqqofTom8S+lXPBvKZEm
LcU9ZuJg6M9Q1HO1D3uIbjEGlYP9VH8AZpEdMpcJDnbCZxJ+FBufuq5pIBiHwQW8
SuDoc47iliArbymlbq8ple7rsXxUrzvjjqYKFUz/0DsLeaJLt1CL1dcke3w1sMNS
Gy+//b+/Q255lZ94MgYj7ag7zLoCnoEr4AXItKD7mgqDQ7BuCa/UCiW/M9+r+V5Z
MKoE2SEN2BDZSEc53RD2eXKDn1OtwLnNd36dpbHaSqtVPQ6QJDIQKo/GXo/sTVst
AEFCJYBHmrwFw3YNKtCywBmUieeQeHPhooCzoGaXQf8kb5bVoJT5YxlK0lQNV9TQ
lDoim27w11aJMwQg9RCdl3/rHpmnkpiFFFKx3IYGhQko5QrBRmZ67IJUGbgj31/w
q448ZR3s5sIoREaHfGW5XpQA8s2wj88TFm5K4vxoGrMwJeZBQG61/1InAGRC8dX4
jwZS7qB/eWLWr6HalY2yi4G/12TJB84yRCnIlFFnbUU9oLJ3xPhhehdqlS9YsARX
awvJTb58ZTo68wN7UKBEw/yz/pKrtCBTMtnIJ9khfhTHnUI18wf7a1KBYv2iZZzp
4jhVaCDOFPv70ydRkoURJTRXyMpTAtAyflLPRNNkvtFMzzLst7Ex367xcAg2af9+
7UYdrIp0q8JOClCdzNkb6+Iy00G2HBAyCfhnf8TupU8SCADNjRReGUzioWOIRu24
TnX8o6fv2G8wGHANJfrS2BGmtBOWW3wdo08WScyCxKQ/lrHMhKW+xeZ+UqCgKfiT
AHKagheEH1+nm0RDZzII0hDDVGLUzZzT6DGrYp5xW5WgOJHDTZCU4dEyZ30N2wkJ
FI2dEGx3lY/YmE7X4knE7UXccjHNJq7NmCbhLzeFfT38FNCwraWIxbRVpdeHClAx
dSYENqn04wER4bSXRsfF9fcDrjAGtb1CGk0PPoEs0D0OoYpyyLVgWw+qnxBMKuvz
5z+mSBDJ8QMybC3x7SOF/kdXsIyXUqknInaUBzJ5qOigRVD+/g4VjZoRtD/eeM4X
mpKtZ4THx9WYz0ZeJijYwUeYBrGt0kglwf/VrMZFSTeYTvxm5BmDkf8+jRB09mV0
YcPPuqGeJvcgrAJqUJ0iny1SyagpSJp4wPFwTYd9xOxjBJzYWgafCfaEdrLpAUSk
YvAVmfVhj591LWzg7r2pATK4Zi75E4nBRORcOlAqyea2zibfKYI0kmtZw8JRj9IL
u3N5piXDAqgQlGqoY2y+GgvMk4vFhF/SO/ZeQaUplYG0XMQlViArE5n/2bIY4Jh4
h4vl6GSXvOWYEhi4NmF0kY34ABmfxbcS8Sngym+tYGPWUb4t+o74DzYvGl+rJ/9Z
lOb1XwXRJvKsaHMI7ujCRu3nhqVgsRlTjCIkWExc5/MnK5leiUVWLHmsfLqC3Kht
-----END RSA PRIVATE KEY-----
`

func checkPassword(pem *pem.Block, password string) {
    _, err := x509.DecryptPEMBlock(pem, []byte(password))

    if err == nil {
        fmt.Println("Valid password found: " + password)
    }
}

func main() {
    decodedPEM, _ := pem.Decode([]byte(privateKeyString))

    checkPassword(decodedPEM, "ruvf")
    checkPassword(decodedPEM, "omgomgponies")
}

The correct password can be verified using openssl (omgomgponies):

openssl rsa -in private.pem -outform PEM -pubout -out public.pem
Enter pass phrase for private.pem:
writing RSA key

And we can also demonstrate openssl returning an error for the invalid password (ruvf):

openssl rsa -in private.pem -outform PEM -pubout -out public.pem
Enter pass phrase for private.pem:
unable to load Private Key
140735220982608:error:0D0680A8:asn1 encoding routines:ASN1_CHECK_TLEN:wrong tag:tasn_dec.c:1180:
140735220982608:error:0D07803A:asn1 encoding routines:ASN1_ITEM_EX_D2I:nested asn1 error:tasn_dec.c:365:Type=RSA
140735220982608:error:04093004:rsa routines:OLD_RSA_PRIV_DECODE:RSA lib:rsa_ameth.c:119:
140735220982608:error:0D0680A8:asn1 encoding routines:ASN1_CHECK_TLEN:wrong tag:tasn_dec.c:1180:
140735220982608:error:0D07803A:asn1 encoding routines:ASN1_ITEM_EX_D2I:nested asn1 error:tasn_dec.c:365:Type=PKCS8_PRIV_KEY_INFO
140735220982608:error:0907B00D:PEM routines:PEM_READ_BIO_PRIVATEKEY:ASN1 lib:pem_pkey.c:141:

Found while trying to create a PEM cracker: https://github.com/diogomonica/go-pemcrack

[minux]

To really verify the password, the function needs to verify
the encoding of decrypted DER block, but that can't be
done without knowing what the block represents.

As the function only returns the decrypted pem block,
without any interpretation, I don't think we can fix this issue.

I think it's possible to construct a PEM that can be decrypted
into two different but valid private keys with two different
passwords, although I don't have a proof.

IMHO, this is working as intended.

PS: your openssl example can detect the password is wrong
because it knows it's a RSA private key. Have you tried to
tell openssl it's a ECDSA private key and see if it can accept
the correct password or not?

[diogomonica]

Looking at the openssl output more carefully it does seem that they are just going through all the RSA private key parsers and trying to see if it matches any of them, before bailing.

It seems that to achieve what I want to do in Go I will have to do the PEM decoding and then try to parse it using the available formats:

func checkPassword(pem *pem.Block, password string) {
    key, err := x509.DecryptPEMBlock(pem, []byte(password))
    if err == nil {
        validKey := false
        _, err = x509.ParsePKCS8PrivateKey(key)
        if err == nil {
            validKey = true
        }

        _, err = x509.ParsePKCS1PrivateKey(key)
        if err == nil {
            validKey = true
        }

        if validKey == true {
            fmt.Println("Valid password found: " + password)
        }
    }
}

I'm ok with closing this as a "works as intended" with the caveat that this might be confusing for other people too. Maybe a source-code comment indicating this potential caveat would be appropriate? Not sure if it is a contrived example someone decoding an encrypted PEM but not actually trying to parse the private key.

[minux]

The problem is that there might be private key encodings that Go
doesn't support (say, you invented a new public key scheme and
want to use PEM to store its private key), so even if DecryptPEMBlock
do what openssl does, it still can't be certain the password is wrong
even if none of the parser routine succeeded.

The docs is also correct, technically:
If an incorrect password is detected an IncorrectPasswordError is
returned.

The condition for returning IncorrectPasswordError is not "if password
is incorrect", instead it's "if incorrect password is detected".
that means there might be cases where an incorrect password can't
be detected.

I will propose a CL to add one sentence after that:
note that DecryptPEMBlock cannot detect all cases of incorrect password.

[minux]

Sent https://golang.org/cl/7603

[diogomonica]

Perfect. Thank you @minux.

[mikioh]

The change 7603 is not submitted yet, re-open.
PS: Gerrit+gopherbot will close this issue automatically when the change is submitted.

[agl]

Note that this problem arises because of deficiencies in PEM encryption. Although PEM encryption may be needed to support reading existing keys, it shouldn't be used in anything new. Putting together scrypt[1] and secretbox[2] will be far stronger and won't have this problem.

[1] https://godoc.org/golang.org/x/crypto/scrypt
[2] https://godoc.org/golang.org/x/crypto/nacl/secretbox