crypto/x509: fix DecryptPEMBlock

The current implement can fail when the
block size is not a multiple of 8 bytes.
This CL makes it work, and also checks that the
data is in fact a multiple of the block size.

R=agl, agl
CC=golang-dev
https://golang.org/cl/6827058

diff --git a/src/pkg/crypto/x509/pem_decrypt.go b/src/pkg/crypto/x509/pem_decrypt.go
index 21f62e5d76bdfb85f57c072b07553c21201be4a0..0dddd7ff9a8d2334024f3001c699fadb77615a96 100644 (file)
--- a/src/pkg/crypto/x509/pem_decrypt.go
+++ b/src/pkg/crypto/x509/pem_decrypt.go
@@ -25,6 +25,16 @@ type rfc1423Algo struct {
        keySize    int
 }
 
+// rfc1423Algos is a mapping of encryption algorithm to an rfc1423Algo that can
+// create block ciphers for that mode.
+var rfc1423Algos = map[string]rfc1423Algo{
+       "DES-CBC":      {des.NewCipher, 8},
+       "DES-EDE3-CBC": {des.NewTripleDESCipher, 24},
+       "AES-128-CBC":  {aes.NewCipher, 16},
+       "AES-192-CBC":  {aes.NewCipher, 24},
+       "AES-256-CBC":  {aes.NewCipher, 32},
+}
+
 // 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.
@@ -45,16 +55,6 @@ func (c rfc1423Algo) deriveKey(password, salt []byte) []byte {
        return out
 }
 
-// rfc1423Algos is a mapping of encryption algorithm to an rfc1423Algo that can
-// create block ciphers for that mode.
-var rfc1423Algos = map[string]rfc1423Algo{
-       "DES-CBC":      {des.NewCipher, 8},
-       "DES-EDE3-CBC": {des.NewTripleDESCipher, 24},
-       "AES-128-CBC":  {aes.NewCipher, 16},
-       "AES-192-CBC":  {aes.NewCipher, 24},
-       "AES-256-CBC":  {aes.NewCipher, 32},
-}
-
 // IsEncryptedPEMBlock returns if the PEM block is password encrypted.
 func IsEncryptedPEMBlock(b *pem.Block) bool {
        _, ok := b.Headers["DEK-Info"]
@@ -81,6 +81,10 @@ func DecryptPEMBlock(b *pem.Block, password []byte) ([]byte, error) {
        }
 
        mode, hexIV := dek[:idx], dek[idx+1:]
+       ciph, ok := rfc1423Algos[mode]
+       if !ok {
+               return nil, errors.New("x509: unknown encryption mode")
+       }
        iv, err := hex.DecodeString(hexIV)
        if err != nil {
                return nil, err
@@ -89,11 +93,6 @@ func DecryptPEMBlock(b *pem.Block, password []byte) ([]byte, error) {
                return nil, errors.New("x509: not enough bytes in IV")
        }
 
-       ciph, ok := rfc1423Algos[mode]
-       if !ok {
-               return nil, errors.New("x509: unknown encryption mode")
-       }
-
        // Based on the OpenSSL implementation. The salt is the first 8 bytes
        // of the initialization vector.
        key := ciph.deriveKey(password, iv[:8])
@@ -107,27 +106,27 @@ func DecryptPEMBlock(b *pem.Block, password []byte) ([]byte, error) {
        dec.CryptBlocks(data, b.Bytes)
 
        // Blocks are padded using a scheme where the last n bytes of padding are all
-       // equal to n. It can pad from 1 to 8 bytes inclusive. See RFC 1423.
+       // equal to n. It can pad from 1 to blocksize bytes inclusive. See RFC 1423.
        // For example:
        //      [x y z 2 2]
        //      [x y 7 7 7 7 7 7 7]
        // If we detect a bad padding, we assume it is an invalid password.
        dlen := len(data)
-       if dlen == 0 {
+       blockSize := block.BlockSize()
+       if dlen == 0 || dlen%blockSize != 0 {
                return nil, errors.New("x509: invalid padding")
        }
-       last := data[dlen-1]
-       if dlen < int(last) {
+       last := int(data[dlen-1])
+       if dlen < last {
                return nil, IncorrectPasswordError
        }
-       if last == 0 || last > 8 {
+       if last == 0 || last > blockSize {
                return nil, IncorrectPasswordError
        }
-       for _, val := range data[dlen-int(last):] {
-               if val != last {
+       for _, val := range data[dlen-last:] {
+               if int(val) != last {
                        return nil, IncorrectPasswordError
                }
        }
-
-       return data[:dlen-int(last)], nil
+       return data[:dlen-last], nil
 }

diff --git a/src/pkg/crypto/x509/pem_decrypt_test.go b/src/pkg/crypto/x509/pem_decrypt_test.go
index 2cb99836eae040d9057105211f4b6db5ea3d502f..0eb6d08f7930ac7caae84f84b7af5ae805f08862 100644 (file)
--- a/src/pkg/crypto/x509/pem_decrypt_test.go
+++ b/src/pkg/crypto/x509/pem_decrypt_test.go
@@ -114,6 +114,21 @@ Pz3RZScwIuubzTGJ1x8EzdffYOsdCa9Mtgpp3L136+23dOd6L/qK2EG2fzrJSHs/
 2XugkleBFSMKzEp9mxXKRfa++uidQvMZTFLDK9w5YjrRvMBo/l2BoZIsq0jAIE1N
 sv5Z/KwlX+3MDEpPQpUwGPlGGdLnjI3UZ+cjgqBcoMiNc6HfgbBgYJSU6aDSHuCk
 clCwByxWkBNgJ2GrkwNrF26v+bGJJJNR4SKouY1jQf0=
+-----END RSA PRIVATE KEY-----`),
+       },
+       {
+               // generated with:
+               // openssl genrsa -aes128 -passout pass:asdf -out server.orig.key 128
+               kind:     "AES-128-CBC",
+               password: []byte("asdf"),
+               pemData: []byte(`
+-----BEGIN RSA PRIVATE KEY-----
+Proc-Type: 4,ENCRYPTED
+DEK-Info: AES-128-CBC,74611ABC2571AF11B1BF9B69E62C89E7
+
+6ei/MlytjE0FFgZOGQ+jrwomKfpl8kdefeE0NSt/DMRrw8OacHAzBNi3pPEa0eX3
+eND9l7C9meCirWovjj9QWVHrXyugFuDIqgdhQ8iHTgCfF3lrmcttVrbIfMDw+smD
+hTP8O1mS/MHl92NE0nhv0w==
 -----END RSA PRIVATE KEY-----`),
        },
 }