[openssl-dev] [openssl.org #4320] [Patch] OpenSSL 1.1.0-pre3: "unable to load Key" error in PEM_get_EVP_CIPHER_INFO()

[Rich Salz via RT]

closing per OP; we fixed the bug.

-- 
Ticket here: http://rt.openssl.org/Ticket/Display.html?id=4320
Please log in as guest with password guest if prompted

-- 
openssl-dev mailing list
To unsubscribe: https://mta.openssl.org/mailman/listinfo/openssl-dev

Re: [openssl-dev] [openssl.org #4320] [Patch] OpenSSL 1.1.0-pre3: "unable to load Key" error in PEM_get_EVP_CIPHER_INFO()

[Roumen Petrov via RT]

Hi Rich,

Rich Salz via RT wrote:
> fixed in commit 985c3146967633707f7c165df82bb0fd8f279758 thanks for the 
> report!
 From initial patch is missing line with header += 9.
Please could you review parsing with ENCRYPTED

Roumen

-- 
Ticket here: http://rt.openssl.org/Ticket/Display.html?id=4320
Please log in as guest with password guest if prompted

>From b359b5caf689583b247d825892ccd6dd42474de1 Mon Sep 17 00:00:00 2001
From: Roumen Petrov 
Date: Thu, 18 Feb 2016 23:26:43 +0200
Subject: [PATCH 4/4] #4320 OpenSSL 1.1.0-pre3: "unable to load Key" error in
 PEM_get_EVP_CIPHER_INFO()

---
 crypto/pem/pem_lib.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/crypto/pem/pem_lib.c b/crypto/pem/pem_lib.c
index a75d9ac..5e8077e 100644
--- a/crypto/pem/pem_lib.c
+++ b/crypto/pem/pem_lib.c
@@ -509,6 +509,7 @@ int PEM_get_EVP_CIPHER_INFO(char *header, EVP_CIPHER_INFO *cipher)
 PEMerr(PEM_F_PEM_GET_EVP_CIPHER_INFO, PEM_R_NOT_ENCRYPTED);
 return (0);
 }
+header += 9;
 for (; (*header != '\n') && (*header != '\0'); header++) ;
 if (*header == '\0') {
 PEMerr(PEM_F_PEM_GET_EVP_CIPHER_INFO, PEM_R_SHORT_HEADER);
@@ -536,7 +537,7 @@ int PEM_get_EVP_CIPHER_INFO(char *header, EVP_CIPHER_INFO *cipher)
 }
 *header = '\0';
 cipher->cipher = enc = EVP_get_cipherbyname(dekinfostart);
-*header = c;
+*header++ = c;
 
 if (enc == NULL) {
 PEMerr(PEM_F_PEM_GET_EVP_CIPHER_INFO, PEM_R_UNSUPPORTED_ENCRYPTION);
-- 
1.8.4

-- 
openssl-dev mailing list
To unsubscribe: https://mta.openssl.org/mailman/listinfo/openssl-dev

[openssl-dev] [openssl.org #4320] [Patch] OpenSSL 1.1.0-pre3: "unable to load Key" error in PEM_get_EVP_CIPHER_INFO()

[Rich Salz via RT]

fixed in commit 985c3146967633707f7c165df82bb0fd8f279758 thanks for the report!
--
Rich Salz, OpenSSL dev team; rs...@openssl.org

-- 
Ticket here: http://rt.openssl.org/Ticket/Display.html?id=4320
Please log in as guest with password guest if prompted

-- 
openssl-dev mailing list
To unsubscribe: https://mta.openssl.org/mailman/listinfo/openssl-dev

Re: [openssl-dev] [openssl.org #4320] [Patch] OpenSSL 1.1.0-pre3: "unable to load Key" error in PEM_get_EVP_CIPHER_INFO()

[Salz, Rich]

Yes, thanks, I was being dumb :(
-- 
openssl-dev mailing list
To unsubscribe: https://mta.openssl.org/mailman/listinfo/openssl-dev

Re: [openssl-dev] [openssl.org #4320] [Patch] OpenSSL 1.1.0-pre3: "unable to load Key" error in PEM_get_EVP_CIPHER_INFO()

[Rainer Jung via RT]

Am 17.02.2016 um 19:51 schrieb Salz, Rich:
>
>>*header = c;
>> +header++;
>
> Header isn't used after that assignment.  How does this line change anything?

The call to load_iv() that occurs next, has as its first argument 
header_pp which is a pointer to header:

char **header_pp = 

Inside load_iv() this pointer is named fromp and is immediately being 
dereferenced:

from = *fromp;

so "from" is an alias to "header", it contains the same address as 
"header". When being dereferenced, "from" will return the same char, 
that "header" points to.

Now in load_iv() "from" is used to iterate over the IV hex chars:

 for (i = 0; i < num; i++) {
 if ((*from >= '0') && (*from <= '9'))
 v = *from - '0';
 else if ((*from >= 'A') && (*from <= 'F'))
 v = *from - 'A' + 10;
 else if ((*from >= 'a') && (*from <= 'f'))
 v = *from - 'a' + 10;
 else {
 PEMerr(PEM_F_LOAD_IV, PEM_R_BAD_IV_CHARS);
 return (0);
 }
 from++;
 to[i / 2] |= v << (long)((!(i & 1)) * 4);
 }

Since *from == *header == ',' at the beginning of the loop, this bombs. 
"header" needs to be incremented to actually point to the beginning of 
the IV.

I hope this is understandable. It took me a moment as well to 
understand, how "from" in load_iv() relates to "header" in 
PEM_get_EVP_CIPHER_INFO().

I checked the patch with the reproduction case before posting and also 
added some debug logging to the "from" loop to double check.

Regards,

Rainer


-- 
Ticket here: http://rt.openssl.org/Ticket/Display.html?id=4320
Please log in as guest with password guest if prompted

-- 
openssl-dev mailing list
To unsubscribe: https://mta.openssl.org/mailman/listinfo/openssl-dev

Re: [openssl-dev] [openssl.org #4320] [Patch] OpenSSL 1.1.0-pre3: "unable to load Key" error in PEM_get_EVP_CIPHER_INFO()

[Rainer Jung]

Am 17.02.2016 um 19:51 schrieb Salz, Rich:



   *header = c;
+header++;


Header isn't used after that assignment.  How does this line change anything?


The call to load_iv() that occurs next, has as its first argument 
header_pp which is a pointer to header:


char **header_pp = 

Inside load_iv() this pointer is named fromp and is immediately being 
dereferenced:


from = *fromp;

so "from" is an alias to "header", it contains the same address as 
"header". When being dereferenced, "from" will return the same char, 
that "header" points to.


Now in load_iv() "from" is used to iterate over the IV hex chars:

for (i = 0; i < num; i++) {
if ((*from >= '0') && (*from <= '9'))
v = *from - '0';
else if ((*from >= 'A') && (*from <= 'F'))
v = *from - 'A' + 10;
else if ((*from >= 'a') && (*from <= 'f'))
v = *from - 'a' + 10;
else {
PEMerr(PEM_F_LOAD_IV, PEM_R_BAD_IV_CHARS);
return (0);
}
from++;
to[i / 2] |= v << (long)((!(i & 1)) * 4);
}

Since *from == *header == ',' at the beginning of the loop, this bombs. 
"header" needs to be incremented to actually point to the beginning of 
the IV.


I hope this is understandable. It took me a moment as well to 
understand, how "from" in load_iv() relates to "header" in 
PEM_get_EVP_CIPHER_INFO().


I checked the patch with the reproduction case before posting and also 
added some debug logging to the "from" loop to double check.


Regards,

Rainer
--
openssl-dev mailing list
To unsubscribe: https://mta.openssl.org/mailman/listinfo/openssl-dev

Re: [openssl-dev] [openssl.org #4320] [Patch] OpenSSL 1.1.0-pre3: "unable to load Key" error in PEM_get_EVP_CIPHER_INFO()

[Salz, Rich]

>   *header = c;
> +header++;

Header isn't used after that assignment.  How does this line change anything?
-- 
openssl-dev mailing list
To unsubscribe: https://mta.openssl.org/mailman/listinfo/openssl-dev

Re: [openssl-dev] [openssl.org #4320] [Patch] OpenSSL 1.1.0-pre3: "unable to load Key" error in PEM_get_EVP_CIPHER_INFO()

[Salz, Rich via RT]

>   *header = c;
> +header++;

Header isn't used after that assignment.  How does this line change anything?


-- 
Ticket here: http://rt.openssl.org/Ticket/Display.html?id=4320
Please log in as guest with password guest if prompted

-- 
openssl-dev mailing list
To unsubscribe: https://mta.openssl.org/mailman/listinfo/openssl-dev

[openssl-dev] [openssl.org #4320] [Patch] OpenSSL 1.1.0-pre3: "unable to load Key" error in PEM_get_EVP_CIPHER_INFO()

[Rainer Jung via RT]

Change

https://github.com/openssl/openssl/commit/33a6d5a0e565e08758bcb6af456ec657c3a7a76a

introduced a bug in crypto/pem/pem_lib.c function 
PEM_get_EVP_CIPHER_INFO(). One line was removed that is actually needed.

The following patch fixes it:

--- crypto/pem/pem_lib.c 2016-02-15 19:08:07.0 +0100
+++ crypto/pem/pem_lib.c  2016-02-17 18:45:14.092815000 +0100
@@ -537,6 +537,7 @@
  *header = '\0';
  cipher->cipher = enc = EVP_get_cipherbyname(dekinfostart);
  *header = c;
+header++;

  if (enc == NULL) {
  PEMerr(PEM_F_PEM_GET_EVP_CIPHER_INFO, 
PEM_R_UNSUPPORTED_ENCRYPTION);


While you are at it, the following is a small improvement which is used 
in similar ways close to this place:

--- crypto/pem/pem_lib.c.orig 2016-02-17 18:45:14.092815000 +0100
+++ crypto/pem/pem_lib.c  2016-02-17 19:15:19.901402000 +0100
@@ -509,6 +509,7 @@
  PEMerr(PEM_F_PEM_GET_EVP_CIPHER_INFO, PEM_R_NOT_ENCRYPTED);
  return (0);
  }
+header += 9;
  for (; (*header != '\n') && (*header != '\0'); header++) ;
  if (*header == '\0') {
  PEMerr(PEM_F_PEM_GET_EVP_CIPHER_INFO, PEM_R_SHORT_HEADER);



How to reproduce the bug:

OpenSSL> dsaparam -out dsa-test 2048
Generating DSA parameters, 2048 bit long prime
This could take some time
...

OpenSSL> gendsa -out dsa-test.pem -aes128 dsa-test
Generating DSA key, 2048 bits
Enter PEM pass phrase:
Verifying - Enter PEM pass phrase:

OpenSSL> dsa -in dsa-test.pem -text
read DSA key
unable to load Private Key
4280523828:error:09065067:PEM routines:load_iv:bad iv chars:pem_lib.c:568:
unable to load Key
error in dsa

The same happens e.g. when using -des or -des3 instead of -aes128.

Without incrementing the header pointer, the parsing of the line

DEK-Info: AES-128-CBC,CBFAADAF91039DF800391FB382CAC3B9

proceeds at the comma, instead of the hex string and bombs out.

Thanks and Regards,

Rainer


-- 
Ticket here: http://rt.openssl.org/Ticket/Display.html?id=4320
Please log in as guest with password guest if prompted

-- 
openssl-dev mailing list
To unsubscribe: https://mta.openssl.org/mailman/listinfo/openssl-dev