[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Probable bug in openpgp
|
From: |
gmail |
|
Subject: |
Probable bug in openpgp |
|
Date: |
Sat, 26 Mar 2011 15:57:18 +0100 |
|
User-agent: |
Mozilla/5.0 (Windows; U; Windows NT 5.1; fr; rv:1.9.2.15) Gecko/20110303 Thunderbird/3.1.9 |
Hi all,
This is my first contact and mail with gnutls mailing list, to summarize
i'm a coder maintening a personnal home server using several GNU tools
since the late 90's.
I build GNUTLS-2.12.0 in a chroot jail (gcc 4.5.2/libc 2.13/binutils
2.21/make 3.82) with an athlon architecture on ext3 FS and, as root,
and notice tests/openpgp-auth freeze after few error messages :
address@hidden mkdir gnutls-2.12.0_build && cd gnutls-2.12.0_build
address@hidden ../gnutls-2.12.0/configure && make
...
address@hidden make check
...
PASS: mini-x509
PASS: mini-x509-rehandshake
Self test `./rng-fork' finished with 0 errors
successSelf test `./rng-fork' finished with 0 errors
PASS: rng-fork
Self test `./openssl' finished with 0 errors
PASS: openssl
server openpgp keys The request is invalid.
client openpgp keys The request is invalid.
server handshake Could not negotiate a supported cipher suite. (-21)
<[CTRL][C]>
address@hidden
I investigate by put on debugging in tests/openpgp-auth and add strace
support relating to openpgp-auth in tests Makefile :
--- gnutls-2.12.0_build/tests/Makefile.orig 2011-03-26
13:23:14.000000000 +0100
+++ gnutls-2.12.0_build/tests/Makefile 2011-03-26 09:52:44.000000000 +0100
@@ -1670,6 +1670,7 @@
if test -f ./$$tst; then dir=./; \
elif test -f $$tst; then dir=; \
else dir="$(srcdir)/"; fi; \
+ if [ "$$tst" = "openpgp-auth" ] ; then tst="strace -o
pgp.txt -ff $$tst" ; fi; \
if $(TESTS_ENVIRONMENT) $${dir}$$tst; then \
all=`expr $$all + 1`; \
case " $(XFAIL_TESTS) " in \
--- gnutls-2.12.0/tests/openpgp-auth.c.orig 2011-03-16
22:40:25.000000000 +0100
+++ gnutls-2.12.0/tests/openpgp-auth.c 2011-03-26 12:05:10.000000000 +0100
@@ -60,6 +60,7 @@
const char *srcdir;
char *pub_key_path, *priv_key_path;
pid_t child;
+ debug=1;
gnutls_global_init ();
And relaunch tests suite :
address@hidden make check
...
[ 5569| 7] RB: Have 5 bytes into buffer. Adding 112 bytes.
[ 5569| 7] RB: Requested 117 bytes
[ 5569| 4] REC[0x8055e80]: Decrypted Packet[1] Alert(21) with length: 2
[ 5569| 4] REC[0x8055e80]: Alert[1|0] - Close notify - was received
Process 5777 attached
[ 5569| 4] REC[0x8064250]: Allocating epoch #0
[ 5569| 7] new stream `[temp]'
[ 5569| 7] new stream fd=10
[ 5569| 2] ASSERT: ../../../gnutls-2.12.0/lib/opencdk/armor.c:327
[ 5569| 6] armor filter: decode
[ 5569| 7] filter [temp] [read]: type=1 rc=0
[ 5569| 7] replace stream fd=10 with fd=11
[ 5569| 2] ASSERT: ../../../gnutls-2.12.0/lib/opencdk/misc.c:325
[ 5569| 2] ASSERT: ../../../gnutls-2.12.0/lib/opencdk/misc.c:325
[ 5569| 2] ASSERT: ../../../gnutls-2.12.0/lib/opencdk/stream.c:1002
[ 5569| 7] close stream ref=0 `[temp]'
[ 5569| 7] close stream fd=11
[ 5777| 4] REC[0x8064250]: Allocating epoch #0
[ 5569| 6] free armor filter
[ 5569| 2] ASSERT: ../../../gnutls-2.12.0/lib/opencdk/stream.c:515
[ 5569| 7] new stream `[temp]'
[ 5569| 7] new stream fd=10
[ 5569| 2] ASSERT: ../../../gnutls-2.12.0/lib/opencdk/armor.c:327
[ 5777| 7] new stream `[temp]'
[ 5777| 7] new stream fd=10
[ 5777| 2] ASSERT: ../../../gnutls-2.12.0/lib/opencdk/armor.c:327
[ 5777| 6] armor filter: decode
[ 5777| 7] filter [temp] [read]: type=1 rc=0
[ 5777| 7] replace stream fd=10 with fd=11
[ 5777| 2] ASSERT: ../../../gnutls-2.12.0/lib/opencdk/misc.c:325
[ 5777| 2] ASSERT: ../../../gnutls-2.12.0/lib/opencdk/misc.c:325
[ 5777| 2] ASSERT: ../../../gnutls-2.12.0/lib/opencdk/stream.c:1002
[ 5777| 7] close stream ref=0 `[temp]'
[ 5777| 7] close stream fd=11
[ 5777| 6] free armor filter
[ 5777| 2] ASSERT: ../../../gnutls-2.12.0/lib/opencdk/stream.c:515
[ 5777| 7] new stream `[temp]'
[ 5777| 7] new stream fd=10
[ 5777| 2] ASSERT: ../../../gnutls-2.12.0/lib/opencdk/armor.c:327
[ 5777| 6] armor filter: decode
[ 5777| 7] filter [temp] [read]: type=1 rc=0
[ 5777| 7] replace stream fd=10 with fd=11
[ 5777| 2] ASSERT: ../../../gnutls-2.12.0/lib/opencdk/stream.c:1002
[ 5777| 7] close stream ref=0 `[temp]'
[ 5777| 7] close stream fd=11
[ 5777| 6] free armor filter
[ 5777| 2] ASSERT: ../../../gnutls-2.12.0/lib/opencdk/stream.c:515
[ 5777| 2] ASSERT: ../../gnutls-2.12.0/lib/gnutls_str.c:496
[ 5777| 2] Error converting hex string: f30fd423c143e7ba.
[ 5777| 2] ASSERT:
../../../gnutls-2.12.0/lib/openpgp/gnutls_openpgp.c:414
[ 5777| 2] ASSERT:
../../../gnutls-2.12.0/lib/openpgp/gnutls_openpgp.c:425
[ 5777| 2] ASSERT:
../../../gnutls-2.12.0/lib/openpgp/gnutls_openpgp.c:509
client openpgp keys The request is invalid.
[ 5777| 2] ASSERT: ../../gnutls-2.12.0/lib/gnutls_constate.c:695
...
<[CTRL][C]>
address@hidden
I notice then something is probably wrong with the key id : "[ 5777|
2] Error converting hex string: f30fd423c143e7ba." triggered by the
gnutls_str.c assert at line 496 which then trigger the three following
asserts.
I have then checked the gnutls_openpgp.c method at the origin of the
line 414 assert :
@321 "gnutls-2.12.0/lib/openpgp/gnutls_openpgp.c"
static int
get_keyid (gnutls_openpgp_keyid_t keyid, const char *str)
{
size_t keyid_size = sizeof (keyid);
if (strlen (str) != 16)
{
_gnutls_debug_log
("The OpenPGP subkey ID has to be 16 hexadecimal
characters.\n");
return GNUTLS_E_INVALID_REQUEST;
}
if (_gnutls_hex2bin (str, strlen (str), keyid, &keyid_size) < 0)
{
_gnutls_debug_log ("Error converting hex string: %s.\n",
str);
return GNUTLS_E_INVALID_REQUEST;
}
return 0;
}
And the _gnutls_hex2bin method that triggers the converting error message :
@476 "gnutls-2.12.0/lib/gnutls_str.c"
int
_gnutls_hex2bin (const opaque * hex_data, int hex_size,
opaque * bin_data,
size_t * bin_size)
{
int i, j;
opaque hex2_data[3];
unsigned long val;
hex2_data[2] = 0;
for (i = j = 0; i < hex_size;)
{
if (!isxdigit (hex_data[i])) /* skip non-hex such
as the ':' in 00:FF */
{
i++;
continue;
}
if (j > *bin_size)
{
gnutls_assert ();
return GNUTLS_E_SHORT_MEMORY_BUFFER;
}
hex2_data[0] = hex_data[i];
hex2_data[1] = hex_data[i + 1];
i += 2;
val = strtoul ((char *) hex2_data, NULL, 16);
if (val == ULONG_MAX)
{
gnutls_assert ();
return GNUTLS_E_PARSING_ERROR;
}
bin_data[j] = val;
j++;
}
*bin_size = j;
return 0;
}
The line 496 assert (if (j > *bin_size) { gnutls_assert (); return
GNUTLS_E_SHORT_MEMORY_BUFFER; }) is normaly triggered by an unexpected
numbers of digits in keyid requiring too many bytes in the target binary
buffer.
I guess then there is something wrrong with the keyid str string or the
keyid_size. The keyid str in the log above seems correct, with 16 digits
forming a correct hexadecimal string.
I decide to check the keyid_size :
--- gnutls-2.12.0/lib/openpgp/gnutls_openpgp.c.orig 2011-03-16
20:53:46.000000000 +0100
+++ gnutls-2.12.0/lib/openpgp/gnutls_openpgp.c 2011-03-26
13:18:35.000000000 +0100
@@ -332,7 +332,7 @@
if (_gnutls_hex2bin (str, strlen (str), keyid, &keyid_size) < 0)
{
- _gnutls_debug_log ("Error converting hex string: %s.\n", str);
+ _gnutls_debug_log ("Error converting hex string: %s.[%d]\n",
str,keyid_size);
return GNUTLS_E_INVALID_REQUEST;
}
And relaunch tests suite :
address@hidden make check
...
[13182| 7] RB: Requested 181 bytes
[13182| 4] REC[0x8055e80]: Decrypted Packet[2] Alert(21) with length: 2
[13182| 4] REC[0x8055e80]: Alert[1|0] - Close notify - was received
Process 13182 suspended
[13184| 6] armor filter: decode
[13184| 7] filter [temp] [read]: type=1 rc=0
[13184| 7] replace stream fd=10 with fd=11
[13184| 2] ASSERT: ../../../gnutls-2.12.0/lib/opencdk/misc.c:325
[13184| 2] ASSERT: ../../../gnutls-2.12.0/lib/opencdk/misc.c:325
[13184| 2] ASSERT: ../../../gnutls-2.12.0/lib/opencdk/stream.c:1002
[13184| 7] close stream ref=0 `[temp]'
[13184| 7] close stream fd=11
[13184| 6] free armor filter
[13184| 2] ASSERT: ../../../gnutls-2.12.0/lib/opencdk/stream.c:515
[13184| 7] new stream `[temp]'
[13184| 7] new stream fd=10
[13184| 2] ASSERT: ../../../gnutls-2.12.0/lib/opencdk/armor.c:327
[13184| 6] armor filter: decode
[13184| 7] filter [temp] [read]: type=1 rc=0
[13184| 7] replace stream fd=10 with fd=11
[13184| 2] ASSERT: ../../../gnutls-2.12.0/lib/opencdk/stream.c:1002
[13184| 7] close stream ref=0 `[temp]'
[13184| 7] close stream fd=11
[13184| 6] free armor filter
[13184| 2] ASSERT: ../../../gnutls-2.12.0/lib/opencdk/stream.c:515
[13184| 2] ASSERT: ../../gnutls-2.12.0/lib/gnutls_str.c:496
[13184| 2] Error converting hex string: f30fd423c143e7ba.[4]
[13184| 2] ASSERT:
../../../gnutls-2.12.0/lib/openpgp/gnutls_openpgp.c:414
[13184| 2] ASSERT:
../../../gnutls-2.12.0/lib/openpgp/gnutls_openpgp.c:425
[13184| 2] ASSERT:
../../../gnutls-2.12.0/lib/openpgp/gnutls_openpgp.c:509
client openpgp keys The request is invalid.
[13184| 2] ASSERT: ../../gnutls-2.12.0/lib/gnutls_constate.c:695
[13184| 4] REC[0x8064250]: Allocating epoch #1
[13184| 3] HSK[0x8064250]: Keeping ciphersuite:
DHE_DSS_AES_128_CBC_SHA1
[13184| 3] HSK[0x8064250]: Keeping ciphersuite:
DHE_DSS_CAMELLIA_128_CBC_SHA1
[13184| 3] HSK[0x8064250]: Keeping ciphersuite:
DHE_DSS_AES_256_CBC_SHA1
[13184| 3] HSK[0x8064250]: Keeping ciphersuite:
DHE_DSS_CAMELLIA_256_CBC_SHA1
[13184| 3] HSK[0x8064250]: Keeping ciphersuite:
DHE_DSS_3DES_EDE_CBC_SHA1
[13184| 3] HSK[0x8064250]: Keeping ciphersuite: DHE_DSS_ARCFOUR_SHA1
[13184| 3] HSK[0x8064250]: Keeping ciphersuite:
DHE_RSA_AES_128_CBC_SHA1
[13184| 3] HSK[0x8064250]: Keeping ciphersuite:
DHE_RSA_CAMELLIA_128_CBC_SHA1
[13184| 3] HSK[0x8064250]: Keeping ciphersuite:
DHE_RSA_AES_256_CBC_SHA1
[13184| 3] HSK[0x8064250]: Keeping ciphersuite:
DHE_RSA_CAMELLIA_256_CBC_SHA1
<[CTRL][C]>
address@hidden
The ligne "[13184| 2] Error converting hex string:
f30fd423c143e7ba.[4]" is self explaining, hard to store 16 digits
hexadecimal string in 4 bytes.
The gnutls_openpgp_keyid_t type is now my main suspect and i then check
his definition and stuff relating to it :
@56 "gnutls-2.12.0/lib/includes/gnutls/openpgp.h"
typedef unsigned char gnutls_openpgp_keyid_t[8];
@2874 "gnutls-2.12.0/NEWS"
gnutls_openpgp_keyid_t: ADDED, instead of hard-coded 'unsigned
char[8]'.
(in Version 2.3.1, released 2008-02-21 )
Bingo !
sizeof(unsigned char[8]) return 8
but
sizeof(*) return ...
4 on a 32 bits architecture
8 on a 64 bits architecture
The issue appears at the third pass of the openpgp-auth which provides
directly a keyid, which drives openpgp to parse the keyid.
On a 64 bits architecture, the pointer size matchs the keyid buffer size
and then get_keyid works without any troubles.
On a 32 bits architecture, it fails as show the openpgp-auth test.
The problem is that keyid_size in the get_keyid method has to get the
keyid BUFFER size and not the keyid POINTER size.
A way to correct that could be to alter openpgp.h to have sizeof(keyid)
returning BUFFER size without broke ABI compatibility :
typedef struct { unsigned char __gnutls_openpgp_keyid_t[8]; }
gnutls_openpgp_keyid_t;
But everything in gnutls deal with gnutls_openpgp_keyid_t as a pointer,
so it's seems better to directly alter "size_t keyid_size = sizeof
(keyid); " in
gnutls_openpgp.c to "size_t keyid_size = sizeof (unsigned char[8]);" and
doing it properly with a constant.
Here is three suggested patchs for this bug :
Patch 1 :
--- gnutls-2.12.0/lib/includes/gnutls/gnutls.h.in.orig 2011-03-23
19:53:44.000000000 +0100
+++ gnutls-2.12.0/lib/includes/gnutls/gnutls.h.in 2011-03-26
14:11:46.000000000 +0100
@@ -1571,6 +1571,11 @@
#define GNUTLS_KEY_CRL_SIGN 2
#define GNUTLS_KEY_ENCIPHER_ONLY 1
#define GNUTLS_KEY_DECIPHER_ONLY 32768
+
+ /* specifics contants relating to keys and subkeys :
+ */
+#define GNUTLS_KEY_ID_SIZE_IN_BYTES 8
+
void
gnutls_certificate_set_params_function
(gnutls_certificate_credentials_t
Patch 2 :
--- gnutls-2.12.0/lib/includes/gnutls/openpgp.h.orig 2011-02-28
17:12:24.000000000 +0100
+++ gnutls-2.12.0/lib/includes/gnutls/openpgp.h 2011-03-26
14:13:53.000000000 +0100
@@ -53,7 +53,7 @@
GNUTLS_OPENPGP_FMT_BASE64
} gnutls_openpgp_crt_fmt_t;
- typedef unsigned char gnutls_openpgp_keyid_t[8];
+ typedef unsigned char
gnutls_openpgp_keyid_t[GNUTLS_KEY_ID_SIZE_IN_BYTES];
/* gnutls_openpgp_cert_t should be defined in gnutls.h
*/
Patch 3 :
--- gnutls-2.12.0/lib/openpgp/gnutls_openpgp.c.orig 2011-03-16
20:53:46.000000000 +0100
+++ gnutls-2.12.0/lib/openpgp/gnutls_openpgp.c 2011-03-26
14:16:04.000000000 +0100
@@ -321,7 +321,11 @@
static int
get_keyid (gnutls_openpgp_keyid_t keyid, const char *str)
{
- size_t keyid_size = sizeof (keyid);
+ /* keyid_size should get keyid buffer size,
+ * since 2008-02-21 keyid has become a pointer so
+ * sizeof(keyid) become wrong and
+ * keyid_size must be explicity given */
+ size_t keyid_size = sizeof (unsigned char[GNUTLS_KEY_ID_SIZE_IN_BYTES]);
if (strlen (str) != 16)
{
@@ -329,7 +333,6 @@
("The OpenPGP subkey ID has to be 16 hexadecimal characters.\n");
return GNUTLS_E_INVALID_REQUEST;
}
-
if (_gnutls_hex2bin (str, strlen (str), keyid, &keyid_size) < 0)
{
_gnutls_debug_log ("Error converting hex string: %s.\n", str);
I have applied this three patchs on my build and the openpgp-auth is
now succesfull :
address@hidden make check
...
PASS: mini-x509
PASS: mini-x509-rehandshake
Self test `./rng-fork' finished with 0 errors
successSelf test `./rng-fork' finished with 0 errors
PASS: rng-fork
Self test `./openssl' finished with 0 errors
PASS: openssl
Self test `./openpgp-auth' finished with 0 errors
Self test `./openpgp-auth' finished with 0 errors
Self test `./openpgp-auth' finished with 0 errors
Self test `./openpgp-auth' finished with 0 errors
Self test `./openpgp-auth' finished with 0 errors
Self test `./openpgp-auth' finished with 0 errors
Self test `./openpgp-auth' finished with 0 errors
Self test `./openpgp-auth' finished with 0 errors
PASS: openpgp-auth
Self test `./openpgp-keyring' finished with 0 errors
PASS: openpgp-keyring
PASS: pgps2kgnu
Self test `./x509self' finished with 0 errors
Self test `./x509self' finished with 0 errors
...
Hope this will help, i'm available for tests/infos if needed.
Regards, Cedric.
- Probable bug in openpgp,
gmail <=