Control: tags -1 + patch

On Fri, 20 Jan 2017 18:53:41 +0100 Kali Kaneko <kaliy...@riseup.net> wrote:
> I'm facing the same issue in pysqlcipher.
> 
> I found a workaround, in:
> https://github.com/sqlcipher/sqlcipher/pull/195/commits/37efb6a9e9995c21a3c9e632bb86b3ba6f613f79#diff-c2495c7f2211a99fba2bfb98a19a1916
> 
> They key to avoid the segfault is this line in the sqlcipher_openssl_cipher 
> function:
> 
> +  EVP_CipherInit_ex(ectx, NULL, NULL, key, iv, mode);
> -  EVP_CipherInit(ectx, NULL, key, iv, mode);
> 
> Note that it just uses the _ex version. I'm not familiar enough with openssl 
> to 
> know if this is a bad usage on the side of sqlcipher, or it's just a bug in 
> openssl.

Thank you Kali for these info!

I don't know OpenSSL much either, but I can confirm that changing the
EVP_CipherInit() calls to EVP_CipherInit_ex() ones (adding the required
extra parameter, obviously) in 33-openssl_1.1.patch fixes the issue for
me.  And as far as I can read in the OpenSSL documentation, that change
is at least harmless.  It doesn't really suggest why it would fix
anything, but it definitely does.

In the current situation (before patching) Valgrind reports an invalid
memory access right before the crash (taken with qTox, but as the call
trace is fully inside sqlite and down it's likely the same anywhere):

> ==26696== Invalid read of size 1
> ==26696==    at 0xDB25CAD: EVP_DecryptUpdate (evp_enc.c:424)
> ==26696==    by 0x5FE5E3A: sqlcipher_openssl_cipher (sqlite3.c:16517)
> ==26696==    by 0x5FF63D9: sqlcipher_page_cipher (sqlite3.c:15696)
> ==26696==    by 0x6008198: sqlite3Codec (sqlite3.c:14376)
> ==26696==    by 0x5FE522D: readDbPage (sqlite3.c:46743)
> ==26696==    by 0x601503B: sqlite3PagerAcquire (sqlite3.c:49225)
> ==26696==    by 0x60152DA: btreeGetPage (sqlite3.c:56080)
> ==26696==    by 0x6015DC6: lockBtree (sqlite3.c:56883)
> ==26696==    by 0x6015DC6: sqlite3BtreeBeginTrans (sqlite3.c:57220)
> ==26696==    by 0x6057B96: sqlite3InitOne (sqlite3.c:103754)
> ==26696==    by 0x6057EFB: sqlite3Init (sqlite3.c:103930)
> ==26696==    by 0x6057FAF: sqlite3ReadSchema (sqlite3.c:103967)
> ==26696==    by 0x605853A: sqlite3LocateTable (sqlite3.c:89352)
> ==26696==  Address 0x12 is not stack'd, malloc'd or (recently) free'd

I guess somehow EVP_CipherInit() doesn't work properly in this case and
leaves some parts improperly initialized.

Attached is a fairly trivial patch changing the calls as mentioned.  It
also happens to make the patch closer to
https://github.com/sqlcipher/sqlcipher/pull/195/commits/37efb6a9e9995c21a3c9e632bb86b3ba6f613f79#diff-c2495c7f2211a99fba2bfb98a19a1916

So dear maintainer, please apply this or an equivalent patch.  The
current situation is grave, rendering the package unusable.

IMO this deserves the "grave" severity, but as it's release time I'll
leave it as is for now as I don't know the policy here for sure.  But
this needs to be fixed in Stretch.

Kind regards,
Colomban
diff --git a/sqlcipher-3.2.0.old/debian/changelog b/sqlcipher-3.2.0/debian/changelog
index 41f8795..54071ce 100644
--- a/sqlcipher-3.2.0.old/debian/changelog
+++ b/sqlcipher-3.2.0/debian/changelog
@@ -1,3 +1,10 @@
+sqlcipher (3.2.0-2.1) UNRELEASED; urgency=medium
+
+  * Non-maintainer upload.
+  * Fix OpenSSL 1.1 support (Closes: #850421)
+
+ -- Colomban Wendling <b...@herbesfolles.org>  Fri, 10 Feb 2017 23:16:14 +0100
+
 sqlcipher (3.2.0-2) unstable; urgency=medium
 
   * support building with openssl 1.1 (Closes: #828555)
diff --git a/sqlcipher-3.2.0.old/debian/patches/33-openssl_1.1.patch b/sqlcipher-3.2.0/debian/patches/33-openssl_1.1.patch
index c28515d..64abe5e 100644
--- a/sqlcipher-3.2.0.old/debian/patches/33-openssl_1.1.patch
+++ b/sqlcipher-3.2.0/debian/patches/33-openssl_1.1.patch
@@ -1,6 +1,6 @@
 --- a/src/crypto_openssl.c
 +++ b/src/crypto_openssl.c
-@@ -155,14 +155,24 @@
+@@ -143,14 +143,24 @@ static int sqlcipher_openssl_random (voi
  }
  
  static int sqlcipher_openssl_hmac(void *ctx, unsigned char *hmac_key, int key_sz, unsigned char *in, int in_sz, unsigned char *in2, int in2_sz, unsigned char *out) {
@@ -26,7 +26,7 @@
    return SQLITE_OK; 
  }
  
-@@ -172,9 +182,23 @@
+@@ -160,9 +170,22 @@ static int sqlcipher_openssl_kdf(void *c
  }
  
  static int sqlcipher_openssl_cipher(void *ctx, int mode, unsigned char *key, int key_sz, unsigned char *iv, unsigned char *in, int in_sz, unsigned char *out) {
@@ -36,22 +36,21 @@
 +#if OPENSSL_VERSION_NUMBER >= 0x10100001L
 +  EVP_CIPHER_CTX *ectx;
 +  ectx = EVP_CIPHER_CTX_new();
-+  EVP_CipherInit(ectx, ((openssl_ctx *)ctx)->evp_cipher, NULL, NULL, mode);
++  EVP_CipherInit_ex(ectx, ((openssl_ctx *)ctx)->evp_cipher, NULL, NULL, NULL, mode);
 +  EVP_CIPHER_CTX_set_padding(ectx, 0); // no padding
-+  EVP_CipherInit(ectx, NULL, key, iv, mode);
++  EVP_CipherInit_ex(ectx, NULL, NULL, key, iv, mode);
 +  EVP_CipherUpdate(ectx, out, &tmp_csz, in, in_sz);
 +  csz = tmp_csz;  
 +  out += tmp_csz;
 +  EVP_CipherFinal(ectx, out, &tmp_csz);
 +  csz += tmp_csz;
 +  EVP_CIPHER_CTX_free(ectx);
-+
 +#else
 +  EVP_CIPHER_CTX ectx;
    EVP_CipherInit(&ectx, ((openssl_ctx *)ctx)->evp_cipher, NULL, NULL, mode);
    EVP_CIPHER_CTX_set_padding(&ectx, 0); // no padding
    EVP_CipherInit(&ectx, NULL, key, iv, mode);
-@@ -184,7 +208,9 @@
+@@ -172,7 +195,9 @@ static int sqlcipher_openssl_cipher(void
    EVP_CipherFinal(&ectx, out, &tmp_csz);
    csz += tmp_csz;
    EVP_CIPHER_CTX_cleanup(&ectx);

Reply via email to