From: Roger Peppe <rogpeppe@gmail.com>
Date: Wed, 7 Nov 2012 15:16:34 +0000 (+0000)
Subject: crypto/x509: fix DecryptPEMBlock
X-Git-Tag: go1.1rc2~1938
X-Git-Url: http://www.git.cypherpunks.su/?a=commitdiff_plain;h=768ba46cc1bbff1c8f49f395d551ffb75b0b6bdf;p=gostls13.git

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 21f62e5d76..0dddd7ff9a 100644
--- 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 2cb99836ea..0eb6d08f79 100644
--- 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-----`),
 	},
 }