]> granicus.if.org Git - php/commitdiff
Add OPENSSL_DONT_ZERO_PAD_KEY constant to prevent key padding
authorJakub Zelenka <bukka@php.net>
Sun, 25 Jun 2017 17:08:50 +0000 (18:08 +0100)
committerJakub Zelenka <bukka@php.net>
Sun, 25 Jun 2017 17:08:50 +0000 (18:08 +0100)
It fixes bug #71917 (openssl_open() returns junk on envelope < 16 bytes)
and bug #72362 (OpenSSL Blowfish encryption is incorrect for short
keys).

NEWS
ext/openssl/openssl.c
ext/openssl/php_openssl.h
ext/openssl/tests/bug71917.phpt [new file with mode: 0644]
ext/openssl/tests/bug72362.phpt [new file with mode: 0644]
ext/openssl/tests/openssl_decrypt_basic.phpt
ext/openssl/tests/openssl_encrypt_error.phpt

diff --git a/NEWS b/NEWS
index 7de05f57cd7ec6dd71f0e90a8f19bc231ff93f96..06823969853b2f1c2cee2ba4f32ead377953de3a 100644 (file)
--- 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)
index cbb2d3a78b8a2403488d59159b229a26784ff7be..524e6fca6d90e1c264de815fd3c59e38c4bc9299 100644 (file)
@@ -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;
index 08f240e8e4b2c983d872e16f3b1728cab3a9e233..bab71595120f27b3ab86c32ad63033af507862c5 100644 (file)
@@ -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
new file mode 100644 (file)
index 0000000..d4415b3
--- /dev/null
@@ -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
new file mode 100644 (file)
index 0000000..40acdbe
--- /dev/null
@@ -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"
index 1c29767cc597013185e54daf24f59a63761266af..37d17150fb25ef64fb66e35d52b4b001243418aa 100644 (file)
@@ -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"
index 791c431211377caa5e10517e5be095a8f9612dce..ea69ad9ee2eff136a89692be1eaa4e1928dd4f6c 100644 (file)
@@ -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)