Add OPENSSL_DONT_ZERO_PAD_KEY constant to prevent key padding

It fixes bug #71917 (openssl_open() returns junk on envelope < 16 bytes) and bug #72362 (OpenSSL Blowfish encryption is incorrect for short keys).
php · Jun 25, 2017 · 0c707fc · 0c707fc
1 parent bda0f4e
commit 0c707fc
Show file tree

Hide file tree

Showing 7 changed files with 77 additions and 16 deletions.
diff --git a/NEWS b/NEWS
@@ -14,6 +14,9 @@ PHP                                                                        NEWS
 - OpenSSL:
  . Fixed bug #74798 (pkcs7_en/decrypt does not work if \x0a is used in content).
    (Anatol)
+ . Added OPENSSL_DONT_ZERO_PAD_KEY constant to prevent key padding and fix bug
+   #71917 (openssl_open() returns junk on envelope < 16 bytes) and bug #72362
+   (OpenSSL Blowfish encryption is incorrect for short keys). (Jakub Zelenka)
 
 - SPL:
   . Fixed bug #73471 (PHP freezes with AppendIterator). (jhdxr)

diff --git a/ext/openssl/openssl.c b/ext/openssl/openssl.c
@@ -1511,6 +1511,7 @@ PHP_MINIT_FUNCTION(openssl)
 
 	REGISTER_LONG_CONSTANT("OPENSSL_RAW_DATA", OPENSSL_RAW_DATA, CONST_CS|CONST_PERSISTENT);
 	REGISTER_LONG_CONSTANT("OPENSSL_ZERO_PADDING", OPENSSL_ZERO_PADDING, CONST_CS|CONST_PERSISTENT);
+	REGISTER_LONG_CONSTANT("OPENSSL_DONT_ZERO_PAD_KEY", OPENSSL_DONT_ZERO_PAD_KEY, CONST_CS|CONST_PERSISTENT);
 
 #ifndef OPENSSL_NO_TLSEXT
 	/* SNI support included */
@@ -6280,20 +6281,7 @@ static int php_openssl_cipher_init(const EVP_CIPHER *cipher_type,
 	int key_len, password_len;
 	size_t max_iv_len;
 
-	/* check and set key */
-	password_len = (int) *ppassword_len;
-	key_len = EVP_CIPHER_key_length(cipher_type);
-	if (key_len > password_len) {
-		key = emalloc(key_len);
-		memset(key, 0, key_len);
-		memcpy(key, *ppassword, password_len);
-		*ppassword = (char *) key;
-		*ppassword_len = key_len;
-		*free_password = 1;
-	} else {
-		key = (unsigned char*)*ppassword;
-		*free_password = 0;
-	}
+	*free_password = 0;
 
 	max_iv_len = EVP_CIPHER_iv_length(cipher_type);
 	if (enc && *piv_len == 0 && max_iv_len > 0 && !mode->is_aead) {
@@ -6318,9 +6306,28 @@ static int php_openssl_cipher_init(const EVP_CIPHER *cipher_type,
 			return FAILURE;
 		}
 	}
-	if (password_len > key_len && !EVP_CIPHER_CTX_set_key_length(cipher_ctx, password_len)) {
-		php_openssl_store_errors();
+	/* check and set key */
+	password_len = (int) *ppassword_len;
+	key_len = EVP_CIPHER_key_length(cipher_type);
+	if (key_len > password_len) {
+		if ((OPENSSL_DONT_ZERO_PAD_KEY & options) && !EVP_CIPHER_CTX_set_key_length(cipher_ctx, password_len)) {
+			php_openssl_store_errors();
+			php_error_docref(NULL, E_WARNING, "Key length cannot be set for the cipher method");
+			return FAILURE;
+		}
+		key = emalloc(key_len);
+		memset(key, 0, key_len);
+		memcpy(key, *ppassword, password_len);
+		*ppassword = (char *) key;
+		*ppassword_len = key_len;
+		*free_password = 1;
+	} else {
+		if (password_len > key_len && !EVP_CIPHER_CTX_set_key_length(cipher_ctx, password_len)) {
+			php_openssl_store_errors();
+		}
+		key = (unsigned char*)*ppassword;
 	}
+
 	if (!EVP_CipherInit_ex(cipher_ctx, NULL, NULL, key, (unsigned char *)*piv, enc)) {
 		php_openssl_store_errors();
 		return FAILURE;

diff --git a/ext/openssl/php_openssl.h b/ext/openssl/php_openssl.h
@@ -31,6 +31,7 @@ extern zend_module_entry openssl_module_entry;
 
 #define OPENSSL_RAW_DATA 1
 #define OPENSSL_ZERO_PADDING 2
+#define OPENSSL_DONT_ZERO_PAD_KEY 4
 
 #define OPENSSL_ERROR_X509_PRIVATE_KEY_VALUES_MISMATCH 0x0B080074
 

diff --git a/ext/openssl/tests/bug71917.phpt b/ext/openssl/tests/bug71917.phpt
@@ -0,0 +1,25 @@
+--TEST--
+Bug #71917: openssl_open() returns junk on envelope < 16 bytes
+--SKIPIF--
+<?php
+if (!extension_loaded("openssl")) die("skip openssl not loaded");
+?>
+--FILE--
+<?php
+function test($envkey) {
+  $publicKey = "file://" . dirname(__FILE__) . "/public.key";
+  $privateKey = "file://" . dirname(__FILE__) . "/private_rsa_1024.key";
+  openssl_public_encrypt($envkey, $envelope, $publicKey);
+  $sealed = openssl_encrypt('plaintext', 'rc4', $envkey, OPENSSL_RAW_DATA | OPENSSL_DONT_ZERO_PAD_KEY);
+  openssl_open($sealed, $output, $envelope, $privateKey, 'rc4');
+  var_dump($output === 'plaintext');
+}
+
+// works - key of 16 bytes
+test('1234567890123456i');
+// fails - key of 15 bytes
+test('123456789012345');
+?>
+--EXPECT--
+bool(true)
+bool(true)
diff --git a/ext/openssl/tests/bug72362.phpt b/ext/openssl/tests/bug72362.phpt
@@ -0,0 +1,14 @@
+--TEST--
+Bug #72362: OpenSSL Blowfish encryption is incorrect for short keys
+--SKIPIF--
+<?php
+if (!extension_loaded("openssl")) die("skip openssl not loaded");
+?>
+--FILE--
+<?php
+var_dump(bin2hex(openssl_encrypt("this is a test string","bf-ecb","12345678", OPENSSL_RAW_DATA | OPENSSL_DONT_ZERO_PAD_KEY)));
+var_dump(bin2hex(openssl_encrypt("this is a test string","bf-ecb","1234567812345678" , OPENSSL_RAW_DATA)));
+?>
+--EXPECT--
+string(48) "e3214d1b16e574828c8a3e222202dde81afd1ad2cb165ab3"
+string(48) "e3214d1b16e574828c8a3e222202dde81afd1ad2cb165ab3"
diff --git a/ext/openssl/tests/openssl_decrypt_basic.phpt b/ext/openssl/tests/openssl_decrypt_basic.phpt
@@ -24,8 +24,13 @@ $padded_data = $data . str_repeat(' ', 16 - (strlen($data) % 16));
 $encrypted = openssl_encrypt($padded_data, $method, $password, OPENSSL_RAW_DATA|OPENSSL_ZERO_PADDING, $iv);
 $output = openssl_decrypt($encrypted, $method, $password, OPENSSL_RAW_DATA|OPENSSL_ZERO_PADDING, $iv);
 var_dump(rtrim($output));
+// if we want to prefer variable length cipher setting
+$encrypted = openssl_encrypt($data, "bf-ecb", $password, OPENSSL_DONT_ZERO_PAD_KEY);
+$output = openssl_decrypt($encrypted, "bf-ecb", $password, OPENSSL_DONT_ZERO_PAD_KEY);
+var_dump($output);
 ?>
 --EXPECT--
 string(45) "openssl_encrypt() and openssl_decrypt() tests"
 string(45) "openssl_encrypt() and openssl_decrypt() tests"
 string(45) "openssl_encrypt() and openssl_decrypt() tests"
+string(45) "openssl_encrypt() and openssl_decrypt() tests"
diff --git a/ext/openssl/tests/openssl_encrypt_error.phpt b/ext/openssl/tests/openssl_encrypt_error.phpt
@@ -23,6 +23,9 @@ var_dump(openssl_encrypt($data, $method, $arr));
 
 // invalid using of an authentication tag
 var_dump(openssl_encrypt($data, $method, $password, 0, $iv, $wrong));
+
+// padding of the key is disabled
+var_dump(openssl_encrypt($data, $method, $password, OPENSSL_DONT_ZERO_PAD_KEY, $iv));
 ?>
 --EXPECTF--
 Warning: openssl_encrypt(): Unknown cipher algorithm in %s on line %d
@@ -48,3 +51,6 @@ NULL
 
 Warning: openssl_encrypt(): The authenticated tag cannot be provided for cipher that doesn not support AEAD in %s on line %d
 string(44) "iPR4HulskuaP5Z6me5uImk6BqVyJG73+63tkPauVZYk="
+
+Warning: openssl_encrypt(): Key length cannot be set for the cipher method in %s on line %d
+bool(false)