Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[NFR] The Cookie encrypted, I want to get the value of the original #864

Closed
iwai opened this issue Jul 17, 2013 · 8 comments
Closed

[NFR] The Cookie encrypted, I want to get the value of the original #864

iwai opened this issue Jul 17, 2013 · 8 comments

Comments

@iwai
Copy link

iwai commented Jul 17, 2013

In this case, I want to get the value set.
I want you to return, the value that you trim the "\ 0".

And I have confirmed for this issue. #749

$key  = 'passowrd';
$text = 'a';

$di = new Phalcon\DI\FactoryDefault();
$di->set('crypt', function() use ($key) {
    $crypt = new Phalcon\Crypt();
    $crypt->setKey($key);
    return $crypt;
});

$_COOKIE['test-key'] = $di->get('crypt')->encryptBase64($text, $key);

$cookie = new Phalcon\Http\Response\Cookies();
$cookie->useEncryption(true);
$cookie->setDI($di);

var_dump($cookie->get('test-key')->getValue());

Output:

string(32) "a\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000"

@ghost
Copy link

ghost commented Jul 18, 2013

@phalcon

This patch implements encryption/decryption with PKCS7 padding (this will make sure that the decrypted string will be exactly the same as the original one — the length will be preserved) and this will allow one to use encrypt/decrypt with cookies just like @iwai wants:

diff --git a/ext/crypt.c b/ext/crypt.c
index 7b447aa..bd0db33 100644
--- a/ext/crypt.c
+++ b/ext/crypt.c
@@ -168,7 +168,9 @@ PHP_METHOD(Phalcon_Crypt, getKey){
 PHP_METHOD(Phalcon_Crypt, encrypt){

        zval *text, *key = NULL, *encrypt_key = NULL, *cipher, *mode, *iv_size;
-       zval *key_size, *rand, *iv, *encrypt, *final_encrypt;
+       zval *key_size, *rand, *iv, *encrypt, *block_size, *padded;
+       unsigned char pad;
+       char *padding;

        PHALCON_MM_GROW();

@@ -216,13 +218,30 @@ PHP_METHOD(Phalcon_Crypt, encrypt){
        PHALCON_INIT_VAR(iv);
        phalcon_call_func_p2(iv, "mcrypt_create_iv", iv_size, rand);

+       PHALCON_INIT_VAR(block_size);
+       phalcon_call_func_p2(block_size, "mcrypt_get_block_size", cipher, mode);
+       convert_to_long_ex(&block_size);
+       pad = (unsigned char)(Z_LVAL_P(block_size) - (Z_STRLEN_P(text) % Z_LVAL_P(block_size)));
+
+#ifdef HAVE_ALLOCA
+       padding = alloca(pad);
+#else
+       padding = emalloc(pad);
+#endif
+       memset(padding, pad, pad);
+
+       PHALCON_INIT_VAR(padded);
+       phalcon_concat_vs(&padded, text, padding, pad, 0 TSRMLS_CC);
+
+#ifndef HAVE_ALLOCA
+       efree(padding);
+#endif
+
        PHALCON_INIT_VAR(encrypt);
-       phalcon_call_func_p5(encrypt, "mcrypt_encrypt", cipher, encrypt_key, text, mode, iv);
-       
-       PHALCON_INIT_VAR(final_encrypt);
-       PHALCON_CONCAT_VV(final_encrypt, iv, encrypt);
+       phalcon_call_func_p5(encrypt, "mcrypt_encrypt", cipher, encrypt_key, padded, mode, iv);

-       RETURN_CTOR(final_encrypt);
+       PHALCON_CONCAT_VV(return_value, iv, encrypt);
+       RETURN_MM();
 }

 /**
@@ -239,7 +258,8 @@ PHP_METHOD(Phalcon_Crypt, encrypt){
 PHP_METHOD(Phalcon_Crypt, decrypt){

        zval *text, *key = NULL, *decrypt_key = NULL, *cipher, *mode, *iv_size;
-       zval *key_size, *text_size, *zero, *iv, *text_to_decipher;
+       zval *key_size, *text_size, *iv, *text_to_decipher, *decrypted;
+       unsigned char pad;

        PHALCON_MM_GROW();

@@ -273,6 +293,7 @@ PHP_METHOD(Phalcon_Crypt, decrypt){

        PHALCON_INIT_VAR(iv_size);
        phalcon_call_func_p2(iv_size, "mcrypt_get_iv_size", cipher, mode);
+       convert_to_long_ex(&iv_size);

        PHALCON_INIT_VAR(key_size);
        phalcon_fast_strlen(key_size, decrypt_key);
@@ -288,15 +309,32 @@ PHP_METHOD(Phalcon_Crypt, decrypt){
                return;
        }

-       PHALCON_INIT_VAR(zero);
-       ZVAL_LONG(zero, 0);
-       
        PHALCON_INIT_VAR(iv);
-       phalcon_call_func_p3(iv, "substr", text, zero, iv_size);
+       phalcon_substr(iv, text, 0, Z_LVAL_P(iv_size));

        PHALCON_INIT_VAR(text_to_decipher);
-       phalcon_call_func_p2(text_to_decipher, "substr", text, iv_size);
-       phalcon_call_func_p5(return_value, "mcrypt_decrypt", cipher, decrypt_key, text_to_decipher, mode, iv);
+       phalcon_substr(text_to_decipher, text, Z_LVAL_P(iv_size), 0);
+
+       PHALCON_INIT_VAR(decrypted);
+       phalcon_call_func_p5(decrypted, "mcrypt_decrypt", cipher, decrypt_key, text_to_decipher, mode, iv);
+       convert_to_string_ex(&decrypted);
+
+       if (Z_STRLEN_P(decrypted)) {
+               pad = (unsigned char)(Z_STRVAL_P(decrypted)[Z_STRLEN_P(decrypted) - 1]);
+               if (pad < Z_STRLEN_P(decrypted)) {
+                       phalcon_substr(return_value, decrypted, 0, Z_STRLEN_P(decrypted) - pad);
+               }
+               else if (pad == Z_STRLEN_P(decrypted)) {
+                       RETVAL_EMPTY_STRING();
+               }
+               else {
+                       RETVAL_FALSE;
+               }
+       }
+       else {
+               RETVAL_FALSE;
+       }
+
        RETURN_MM();
 }

However, there is one major drawback: this is not compatible with the original encrypt() implementation: this decrypt() implementation won't be able to decrypt the text encrypted with the original encrypt().

What would you suggest?

@phalcon
Copy link
Collaborator

phalcon commented Jul 18, 2013

Can we add a method to Phalcon\Encrypt to specify that PKCS7 padding must be used?

@ghost
Copy link

ghost commented Jul 19, 2013

Yes, we can but does it mean that we will have to update CryptInterface as well?

@phalcon
Copy link
Collaborator

phalcon commented Jul 19, 2013

@yES,

something like:

$crypt = new Phalcon\Crypt();
$crypt->usePadding(false);

@ghost
Copy link

ghost commented Jul 19, 2013

OK, makes sense.

@ghost
Copy link

ghost commented Jul 19, 2013

OK, I am going to implement

  • PKCS#7 padding
  • ANSI X.923 padding
  • ISO 10126 padding
  • ISO/IEC 7816-4 padding
  • partially reversible zero padding

@iwai
Copy link
Author

iwai commented Jul 22, 2013

Sorry for reply late.
Everyone, Thank you for fast answer.
Thank you very much @sjinks !!

@ghost
Copy link

ghost commented Jul 22, 2013

Fixed in #887

@phalcon phalcon closed this as completed Jul 22, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant