FAQ
Reviewers: agl,

Message:
Hello agl@chromium.org (cc: golang-dev@googlegroups.com),

I'd like you to review this change to
https://go.googlecode.com/hg/


Description:
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.

Please review this at http://codereview.appspot.com/6827058/

Affected files:
M src/pkg/crypto/x509/pem_decrypt.go
M src/pkg/crypto/x509/pem_decrypt_test.go


Index: src/pkg/crypto/x509/pem_decrypt.go
===================================================================
--- a/src/pkg/crypto/x509/pem_decrypt.go
+++ b/src/pkg/crypto/x509/pem_decrypt.go
@@ -25,6 +25,16 @@
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 @@
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 @@
}

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 @@
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 @@
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
}
Index: src/pkg/crypto/x509/pem_decrypt_test.go
===================================================================
--- a/src/pkg/crypto/x509/pem_decrypt_test.go
+++ b/src/pkg/crypto/x509/pem_decrypt_test.go
@@ -116,4 +116,19 @@
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-----`),
+ },
}

Search Discussions

Related Discussions

Discussion Navigation
viewthread | post
Discussion Overview
groupgolang-dev @
categoriesgo
postedNov 7, '12 at 3:06p
activeNov 7, '12 at 3:20p
posts3
users2
websitegolang.org

2 users in discussion

Rogpeppe: 2 posts Agl: 1 post

People

Translate

site design / logo © 2022 Grokbase