Package: release.debian.org
Severity: normal
Tags: bookworm
X-Debbugs-Cc: ya...@packages.debian.org, car...@debian.org
Control: affects -1 + src:yapet
User: release.debian....@packages.debian.org
Usertags: pu

Hi,

[ Reason ]
After the update of openssl/3.0.13-1~deb12u1 in bookworm-pu Sean found
that old 1.0 format databases. While most of people should have moved
some time ago to 2.0 format databases, they are still claimed to be
supported. The update of openssl uncovered though a bug in yapet (as
well present in unstable, and fixed as well).

Sebastian explained the situation in https://bugs.debian.org/1068045#94

[ Impact ]
Users using the old 1.0 format could not open anymore their store.

[ Tests ]
Done explicitly with an old 1.0 format database provided by sean,
running the testsuite, and manual checks with 2.0 format databases.

[ Risks ]
Patches provided by the openssl maintainer. While they are not yet
applied upstream, they tackle the bug in yapet as isolated by the
openssl maintainers.

[ Checklist ]
  [x] *all* changes are documented in the d/changelog
  [x] I reviewed all changes and I approve them
  [x] attach debdiff against the package in (old)stable
  [x] the issue is verified as fixed in unstable

[ Changes ]
The two patches drop EVP_CIPHER_CTX_set_key_length() invocation to
keep compatiblity with 1.0 databases and with openssl versions.
Quoting the commit:

|yapet did for blowfish:
|
||     EVP_CipherInit_ex(ctx, cipher, NULL, KEY, iv, mode);
||     EVP_CIPHER_CTX_set_key_length(ctx, KEY_LENGTH);
||     EVP_CipherUpdate(ctx, …);
|
|this worked in earlier OpenSSL versions and stopped working in
|openssl-3.0.13. The problem here is that the
|EVP_CIPHER_CTX_set_key_length() is ignored and the later OpenSSL version
|returns rightfully an error "Provider routines::no key set" here.
|
|Blowfish does support variable key lenghts but the key length has to be
|set first followed by the actual key. Otherwise the blocksize (16) will
|be used.
|The correct way to deal with this would be:
||     EVP_CipherInit_ex(ctx, cipher, NULL, NULL, NULL, mode);
||     EVP_CIPHER_CTX_set_key_length(ctx, KEY_LENGTH);
||     EVP_CipherInit_ex(ctx, NULL, NULL, KEY, IV, mode);
||     EVP_CipherUpdate(ctx, …);
|
|Using now the proper way will break earlier databases because in the
|blowfish case, always the default blocksize / 16 has been used.
|
|In order to keep compatibility with earlier versions of the database and
|openssl remove the EVP_CIPHER_CTX_set_key_length() invocation.

While at it Sebastian fixed as well the invocation present for the
crypt/aes code.

[ Other info ]
None.

Regards,
Salvatore
diff -Nru yapet-2.6/debian/changelog yapet-2.6/debian/changelog
--- yapet-2.6/debian/changelog  2022-03-14 14:19:11.000000000 +0100
+++ yapet-2.6/debian/changelog  2024-04-11 20:40:18.000000000 +0200
@@ -1,3 +1,16 @@
+yapet (2.6-2~deb12u1) bookworm; urgency=medium
+
+  * Rebuild for bookworm
+
+ -- Salvatore Bonaccorso <car...@debian.org>  Thu, 11 Apr 2024 20:40:18 +0200
+
+yapet (2.6-2) unstable; urgency=medium
+
+  * crypt/blowfish: Remove EVP_CIPHER_CTX_set_key_length() (Closes: #1064724)
+  * crypt/aes: Remove EVP_CIPHER_CTX_set_key_length()
+
+ -- Salvatore Bonaccorso <car...@debian.org>  Mon, 08 Apr 2024 21:32:50 +0200
+
 yapet (2.6-1) unstable; urgency=medium
 
   * New upstream version 2.6
diff -Nru 
yapet-2.6/debian/patches/crypt-aes-Remove-EVP_CIPHER_CTX_set_key_length.patch 
yapet-2.6/debian/patches/crypt-aes-Remove-EVP_CIPHER_CTX_set_key_length.patch
--- 
yapet-2.6/debian/patches/crypt-aes-Remove-EVP_CIPHER_CTX_set_key_length.patch   
    1970-01-01 01:00:00.000000000 +0100
+++ 
yapet-2.6/debian/patches/crypt-aes-Remove-EVP_CIPHER_CTX_set_key_length.patch   
    2024-04-11 20:40:18.000000000 +0200
@@ -0,0 +1,41 @@
+From aaa573b14bafcc9a6b46495bd4ffc15b90d35902 Mon Sep 17 00:00:00 2001
+From: Sebastian Andrzej Siewior <sebast...@breakpoint.cc>
+Date: Mon, 8 Apr 2024 18:19:12 +0200
+Subject: [PATCH] crypt/aes: Remove EVP_CIPHER_CTX_set_key_length().
+
+The EVP_CIPHER_CTX_set_key_length() in the AES-256-CBC case is pointless
+because the key here is fixed EVP_CIPHER_CTX_set_key_length() and the
+function does not change the size.
+
+Remove the EVP_CIPHER_CTX_set_key_length() invocation.
+
+Signed-off-by: Sebastian Andrzej Siewior <sebast...@breakpoint.cc>
+---
+ src/libs/crypt/aes256.cc | 11 -----------
+ 1 file changed, 11 deletions(-)
+
+diff --git a/src/libs/crypt/aes256.cc b/src/libs/crypt/aes256.cc
+index 1041b9c57347..e105b1a5bedd 100644
+--- a/src/libs/crypt/aes256.cc
++++ b/src/libs/crypt/aes256.cc
+@@ -113,17 +113,6 @@ EVP_CIPHER_CTX* Aes256::initializeOrThrow(const 
SecureArray& ivec, MODE mode) {
+         throw CipherError{_("Error initializing cipher")};
+     }
+ 
+-    success = EVP_CIPHER_CTX_set_key_length(context, getKey()->keySize());
+-    if (success != SSL_SUCCESS) {
+-        LOG_MESSAGE(std::string{__func__} + ": Error setting key length");
+-        destroyContext(context);
+-        char msg[YAPET::Consts::EXCEPTION_MESSAGE_BUFFER_SIZE];
+-        std::snprintf(msg, YAPET::Consts::EXCEPTION_MESSAGE_BUFFER_SIZE,
+-                      _("Cannot set key length on context to %d"),
+-                      getKey()->keySize());
+-        throw CipherError{msg};
+-    }
+-
+     return context;
+ }
+ 
+-- 
+2.43.0
+
diff -Nru 
yapet-2.6/debian/patches/crypt-blowfish-Remove-EVP_CIPHER_CTX_set_key_length.patch
 
yapet-2.6/debian/patches/crypt-blowfish-Remove-EVP_CIPHER_CTX_set_key_length.patch
--- 
yapet-2.6/debian/patches/crypt-blowfish-Remove-EVP_CIPHER_CTX_set_key_length.patch
  1970-01-01 01:00:00.000000000 +0100
+++ 
yapet-2.6/debian/patches/crypt-blowfish-Remove-EVP_CIPHER_CTX_set_key_length.patch
  2024-04-11 20:40:18.000000000 +0200
@@ -0,0 +1,66 @@
+From a54b5e81a61aa7e77e45a970ce88b9b4269fde7d Mon Sep 17 00:00:00 2001
+From: Sebastian Andrzej Siewior <sebast...@breakpoint.cc>
+Date: Mon, 8 Apr 2024 18:03:30 +0200
+Subject: [PATCH] crypt/blowfish: Remove EVP_CIPHER_CTX_set_key_length().
+MIME-Version: 1.0
+Content-Type: text/plain; charset=UTF-8
+Content-Transfer-Encoding: 8bit
+
+yapet did for blowfish:
+
+|     EVP_CipherInit_ex(ctx, cipher, NULL, KEY, iv, mode);
+|     EVP_CIPHER_CTX_set_key_length(ctx, KEY_LENGTH);
+|     EVP_CipherUpdate(ctx, …);
+
+this worked in earlier OpenSSL versions and stopped working in
+openssl-3.0.13. The problem here is that the
+EVP_CIPHER_CTX_set_key_length() is ignored and the later OpenSSL version
+returns rightfully an error "Provider routines::no key set" here.
+
+Blowfish does support variable key lenghts but the key length has to be
+set first followed by the actual key. Otherwise the blocksize (16) will
+be used.
+The correct way to deal with this would be:
+|     EVP_CipherInit_ex(ctx, cipher, NULL, NULL, NULL, mode);
+|     EVP_CIPHER_CTX_set_key_length(ctx, KEY_LENGTH);
+|     EVP_CipherInit_ex(ctx, NULL, NULL, KEY, IV, mode);
+|     EVP_CipherUpdate(ctx, …);
+
+Using now the proper way will break earlier databases because in the
+blowfish case, always the default blocksize / 16 has been used.
+
+In order to keep compatibility with earlier versions of the database and
+openssl remove the EVP_CIPHER_CTX_set_key_length() invocation.
+
+Fixes #26
+Fixes #24
+
+Signed-off-by: Sebastian Andrzej Siewior <sebast...@breakpoint.cc>
+---
+ src/libs/crypt/crypto.cc | 10 ----------
+ 1 file changed, 10 deletions(-)
+
+diff --git a/src/libs/crypt/crypto.cc b/src/libs/crypt/crypto.cc
+index ade991edf961..139e0823e753 100644
+--- a/src/libs/crypt/crypto.cc
++++ b/src/libs/crypt/crypto.cc
+@@ -98,16 +98,6 @@ EVP_CIPHER_CTX* Crypto::initializeOrThrow(MODE mode) {
+         throw CipherError{_("Error initializing cipher")};
+     }
+ 
+-    success = EVP_CIPHER_CTX_set_key_length(context, _key->keySize());
+-    if (success != SSL_SUCCESS) {
+-        destroyContext(context);
+-        char msg[YAPET::Consts::EXCEPTION_MESSAGE_BUFFER_SIZE];
+-        std::snprintf(msg, YAPET::Consts::EXCEPTION_MESSAGE_BUFFER_SIZE,
+-                      _("Cannot set key length on context to %d"),
+-                      _key->keySize());
+-        throw CipherError{msg};
+-    }
+-
+     return context;
+ }
+ 
+-- 
+2.43.0
+
diff -Nru yapet-2.6/debian/patches/series yapet-2.6/debian/patches/series
--- yapet-2.6/debian/patches/series     2022-03-14 14:19:11.000000000 +0100
+++ yapet-2.6/debian/patches/series     2024-04-11 20:40:18.000000000 +0200
@@ -1,2 +1,4 @@
 do-not-install-licenses-files.patch
 avoid-remote-font.patch
+crypt-blowfish-Remove-EVP_CIPHER_CTX_set_key_length.patch
+crypt-aes-Remove-EVP_CIPHER_CTX_set_key_length.patch

Reply via email to