[PATCH] ecrypto/ecdsa: fix a zero change in the test suite
At the end of the testsuite in test_builtin() happens the following:
- a previously created signature gets modified at a random spot
- this signature is compared against the data which was used to create the
signature.
Now, in theory the last step should always fail in reality is passed
sometimes. The modification algorithm did the following:
| offset = sig[10] % 66;
| dirt = sig[11];
| dirt = dirt ? dirt : 1;
| sig[offset] ^= dirt;
If sig[10] is 0xa7 and sig[11] is 0x9e the last line evolves to:
| sig[35] ^= 0x9e;
The signature consists of two BIGNUMs encoded as ASN1 string. sig[34] and
sig[35] is the begin of the second and last number. sig[35] contains the
length of this number and its content is 0x1e. Now, 0x9e ^ 0x1e = 0x80
and this is a special value. It means that the length of the value is
infinite i.e. everything until the end of the stream. So the ASN1 parser
considers the remaining data as the last element. Since there is nothing
after it, it succeeds. This random modification was a zero change.
This change ensures that something like this does not happen again. If we
do a zero change by accident (R and S are unchanged) we make a third
change and hope that something will change now.
Signed-off-by: Sebastian Andrzej Siewior sebast...@breakpoint.cc
---
crypto/ecdsa/ecdsatest.c | 87 +++---
1 files changed, 82 insertions(+), 5 deletions(-)
diff --git a/crypto/ecdsa/ecdsatest.c b/crypto/ecdsa/ecdsatest.c
index aa4e148..67db141 100644
--- a/crypto/ecdsa/ecdsatest.c
+++ b/crypto/ecdsa/ecdsatest.c
@@ -281,6 +281,85 @@ x962_err:
return ret;
}
+static int compare_sig(unsigned char *osig, unsigned int sig_len, ECDSA_SIG
*old_sig)
+{
+ unsigned char *signature = osig;
+ ECDSA_SIG *new_sig = NULL;
+ char *org_r = NULL, *org_s = NULL;
+ char *new_r = NULL, *new_s = NULL;
+ int ret = -1;
+
+ org_r = BN_bn2hex(old_sig-r);
+ org_s = BN_bn2hex(old_sig-s);
+ if (!org_r || !org_s)
+ goto out;
+
+ new_sig = ECDSA_SIG_new();
+ if (!new_sig)
+ goto out;
+ if (!d2i_ECDSA_SIG(new_sig, (const unsigned char **)signature,
sig_len))
+ goto out;
+
+ new_r = BN_bn2hex(new_sig-r);
+ new_s = BN_bn2hex(new_sig-s);
+ if (!new_r || !new_s)
+ goto out;
+ if ((!strcmp(org_r, new_r))
+ !strcmp(org_s, new_s))
+ /* the signature did not change */
+ ret = 1;
+ else
+ ret = 0;
+out:
+ if (new_sig)
+ ECDSA_SIG_free(new_sig);
+ if (new_r)
+ OPENSSL_free(new_r);
+ if (new_s)
+ OPENSSL_free(new_s);
+ if (org_r)
+ OPENSSL_free(org_r);
+ if (org_s)
+ OPENSSL_free(org_s);
+ return ret;
+}
+
+static void modify_signature(unsigned char *osig, unsigned int sig_len, BIO
*out)
+{
+ unsigned char dirt, offset;
+ unsigned char *signature = osig;
+ ECDSA_SIG *org_sig;
+ int ret;
+
+ org_sig = ECDSA_SIG_new();
+ if (!org_sig)
+ return;
+
+ if (!d2i_ECDSA_SIG(org_sig, (const unsigned char **)signature,
sig_len))
+ goto out;
+
+ signature = osig;
+ offset = signature[10] % sig_len;
+ dirt = signature[11];
+ dirt = dirt ? dirt : 1;
+ signature[offset] ^= dirt;
+
+ ret = compare_sig(osig, sig_len, org_sig);
+ if (ret = 0)
+ goto out;
+
+ signature[offset] = ~signature[offset];
+ ret = compare_sig(osig, sig_len, org_sig);
+ if (ret = 0)
+ goto out;
+ BIO_printf(out, Failed to modify signature. Tried: %02x = %02x =
%02x\n,
+ (unsigned char) (~osig[offset] ^ dirt),
+ (unsigned char)~osig[offset], osig[offset]);
+ BIO_printf(out, at offset 0x%02x and it was always equal.\n, offset);
+out:
+ ECDSA_SIG_free(org_sig);
+}
+
int test_builtin(BIO *out)
{
EC_builtin_curve *curves = NULL;
@@ -325,8 +404,6 @@ int test_builtin(BIO *out)
/* now create and verify a signature for every curve */
for (n = 0; n crv_len; n++)
{
- unsigned char dirt, offset;
-
nid = curves[n].nid;
if (nid == NID_ipsec4)
continue;
@@ -416,9 +493,9 @@ int test_builtin(BIO *out)
BIO_printf(out, .);
(void)BIO_flush(out);
/* modify a single byte of the signature */
- offset = signature[10] % sig_len;
- dirt = signature[11];
- signature[offset] ^= dirt ? dirt : 1;
+
+ modify_signature(signature, sig_len, out);
+
if (ECDSA_verify(0, digest, 20, signature, sig_len, eckey) == 1)
{
Non empty error stack failing non-blocking SSL IO
I'm using OpenSSL 0.9.8i, and have noticed the following scenario: - Some OpenSSL crypto function returns with an error, leaving a description of the error on the error queue - The application neglects to call ERR_clear_error() - SSL_read() is then called on a non-blocking socket and returns because there's no input available - Calling SSL_get_error() returns SSL_ERROR_SSL instead of SSL_ERROR_WANT_READ, because the error queue is not empty. Would it be possible to modify the code so that blocking socket takes precedence over the error queue? If not, what is the recommended programming practice with non-blocking sockets? - ensure the everybody call ERR_clear_error() after an error - call ERR_clear_error() before SSL read/write (but if that's recommended why isn't it inside SSL_read/SSL_write) Thanks, Uri
What is the REALLY proper way to use an ENGINE?
If we take a look at any ENGINE_load_XXX function, we find that they all has similar structure: ENGINE *toadd = engine_XXX(); if(!toadd) return; ENGINE_add(toadd); ENGINE_free(toadd); ERR_clear_error(); My question is: why we need call ENGINE_free(toadd) ?? Somewhere inside it calls EVP_PKEY_asn1_free(), which besides everything else desroy all AMETH structures, created during engine initialization via EVP_PKEY_asn1_set_* . So, let's consider following example: CRYPTO_malloc_init(); ERR_load_crypto_strings(); ENGINE_load_builtin_engines(); e = ENGINE_by_id(XXX); ENGINE_set_default(e, ENGINE_METHOD_ALL); OpenSSL_add_all_algorithms(); OpenSSL_add_ssl_algorithms(); EVP_PKEY *signing_key = ENGINE_load_private_key(e, NULL, NULL, NULL); // bla-bla-bla... And, if in engine load priv func we write something like EVP_PKEY *pkey = EVP_PKEY_new(); pkey-ameth = ENGINE_get_pkey_asn1_meth(eng, NID_id_SOME_NID); it's not gonna work, because all ASN1 structures, carefully created by engine via calls to EVP_PKEY_asn1_set_* are invalid and possibly corrupted at this moment. Is it a design issue, or provided example is invalid (but it taken from openssl sources, hm...)?
Re: What is the REALLY proper way to use an ENGINE?
Update: adding ENGINE_init(e) after e = ENGINE_by_id(XXX); doesn't make any difference, as in my case functional reference count is 8(???) at the moment of ENGINE_init(e) call, so engine is not re-initialised. :( On 4 January 2011 04:12, Andrey Kulikov amde...@gmail.com wrote: If we take a look at any ENGINE_load_XXX function, we find that they all has similar structure: ENGINE *toadd = engine_XXX(); if(!toadd) return; ENGINE_add(toadd); ENGINE_free(toadd); ERR_clear_error(); My question is: why we need call ENGINE_free(toadd) ?? Somewhere inside it calls EVP_PKEY_asn1_free(), which besides everything else desroy all AMETH structures, created during engine initialization via EVP_PKEY_asn1_set_* . So, let's consider following example: CRYPTO_malloc_init(); ERR_load_crypto_strings(); ENGINE_load_builtin_engines(); e = ENGINE_by_id(XXX); ENGINE_set_default(e, ENGINE_METHOD_ALL); OpenSSL_add_all_algorithms(); OpenSSL_add_ssl_algorithms(); EVP_PKEY *signing_key = ENGINE_load_private_key(e, NULL, NULL, NULL); // bla-bla-bla... And, if in engine load priv func we write something like EVP_PKEY *pkey = EVP_PKEY_new(); pkey-ameth = ENGINE_get_pkey_asn1_meth(eng, NID_id_SOME_NID); it's not gonna work, because all ASN1 structures, carefully created by engine via calls to EVP_PKEY_asn1_set_* are invalid and possibly corrupted at this moment. Is it a design issue, or provided example is invalid (but it taken from openssl sources, hm...)?
Re: What is the REALLY proper way to use an ENGINE?
On Tue, Jan 04, 2011, Andrey Kulikov wrote: If we take a look at any ENGINE_load_XXX function, we find that they all has similar structure: ENGINE *toadd = engine_XXX(); if(!toadd) return; ENGINE_add(toadd); ENGINE_free(toadd); ERR_clear_error(); My question is: why we need call ENGINE_free(toadd) ?? To avoid a memory leak. When you call engine_XXX() you get a reference to the ENGINE. When it is added to the ENGINE list the reference count is incremented. When you call ENGINE_free() the count is decremented and the ENGINE is freed only if the reference count is zero. Steve. -- Dr Stephen N. Henson. OpenSSL project core developer. Commercial tech support now available see: http://www.openssl.org __ OpenSSL Project http://www.openssl.org Development Mailing List openssl-dev@openssl.org Automated List Manager majord...@openssl.org
Re: Non empty error stack failing non-blocking SSL IO
If your program ignores the error queue, your program is doing the equivalent of not checking errno after every system call. The program is required to deal with the error queue, because it is OpenSSL's only mechanism for informing the application code of the wide variety of potential protocol and authentication issues. The program should absolutely not be doing the same things in the cases of SSL_get_error() returning SSL_ERROR_SSL and SSL_WANT_READ. (It may be that someone missed a break statement at the end of one case and it's falling through to the next.) Either way, this is not anomalous behavior on OpenSSL's part. After you call SSL_read() and get zero bytes, you must determine why you got zero bytes, and that's where you should call SSL_get_error(). If it returns SSL_ERROR_SSL, you must check the error queue to determine exactly why the SSL session is in an error state. (The reason for the queue is because you're supposed to be interested in and handle every error that comes up in the process, not merely the most-recent.) -Kyle H On Mon, Jan 3, 2011 at 4:22 AM, Uri Simchoni u...@ctera.com wrote: I’m using OpenSSL 0.9.8i, and have noticed the following scenario: - Some OpenSSL crypto function returns with an error, leaving a description of the error on the error queue - The application neglects to call ERR_clear_error() - SSL_read() is then called on a non-blocking socket and returns because there’s no input available - Calling SSL_get_error() returns SSL_ERROR_SSL instead of SSL_ERROR_WANT_READ, because the error queue is not empty. Would it be possible to modify the code so that blocking socket takes precedence over the error queue? If not, what is the recommended programming practice with non-blocking sockets? - ensure the everybody call ERR_clear_error() after an error - call ERR_clear_error() before SSL read/write (but if that’s recommended why isn’t it inside SSL_read/SSL_write) Thanks, Uri smime.p7s Description: S/MIME Cryptographic Signature
RE: Non empty error stack failing non-blocking SSL IO
I realize that I must be doing all that. The difference I see from errno (and the reason I wrote this) is that if you fail to read errno, it does not affect the outcome of the NEXT system call (save for few documented cases which specifically instruct you to clear errno before calling the function). That strikes me as a design problem. It's difficult, in a large system, to make sure that everyone play by the book, and with non-blocking IO it's common that the same thread deals with unrelated tasks (with a select() loop and socket handlers). So what can happen here is that my code runs OpenSSL with full error checking, and somebody else's code runs OpenSSL with no error checking, breaking my code. It's preferable for a general-purpose library to be designed to avoid such scenarios, and in this particular case it appears to have a solution - check socket blocking state before checking the error queue. That's what I was getting at. -Original Message- From: owner-openssl-...@openssl.org [mailto:owner-openssl-...@openssl.org] On Behalf Of aerow...@gmail.com Sent: Tuesday, January 04, 2011 5:51 AM To: openssl-dev@openssl.org Subject: Re: Non empty error stack failing non-blocking SSL IO If your program ignores the error queue, your program is doing the equivalent of not checking errno after every system call. The program is required to deal with the error queue, because it is OpenSSL's only mechanism for informing the application code of the wide variety of potential protocol and authentication issues. The program should absolutely not be doing the same things in the cases of SSL_get_error() returning SSL_ERROR_SSL and SSL_WANT_READ. (It may be that someone missed a break statement at the end of one case and it's falling through to the next.) Either way, this is not anomalous behavior on OpenSSL's part. After you call SSL_read() and get zero bytes, you must determine why you got zero bytes, and that's where you should call SSL_get_error(). If it returns SSL_ERROR_SSL, you must check the error queue to determine exactly why the SSL session is in an error state. (The reason for the queue is because you're supposed to be interested in and handle every error that comes up in the process, not merely the most-recent.) -Kyle H On Mon, Jan 3, 2011 at 4:22 AM, Uri Simchoni u...@ctera.com wrote: I’m using OpenSSL 0.9.8i, and have noticed the following scenario: - Some OpenSSL crypto function returns with an error, leaving a description of the error on the error queue - The application neglects to call ERR_clear_error() - SSL_read() is then called on a non-blocking socket and returns because there’s no input available - Calling SSL_get_error() returns SSL_ERROR_SSL instead of SSL_ERROR_WANT_READ, because the error queue is not empty. Would it be possible to modify the code so that blocking socket takes precedence over the error queue? If not, what is the recommended programming practice with non-blocking sockets? - ensure the everybody call ERR_clear_error() after an error - call ERR_clear_error() before SSL read/write (but if that’s recommended why isn’t it inside SSL_read/SSL_write) Thanks, Uri
Re: What is the REALLY proper way to use an ENGINE?
Thanks for a explanations.
Let's consider following main, using ccgost engine:
main(){
OPENSSL_config(NULL);
ENGINE *e = ENGINE_by_id(gost);
ENGINE_init(e);
ENGINE_free(e);
ENGINE_set_default(e, ENGINE_METHOD_ALL);
OpenSSL_add_all_algorithms();
// emulating ENGINE_load_private_key()
EVP_PKEY *pkey = EVP_PKEY_new();
pkey-ameth = ENGINE_get_pkey_asn1_meth(e, NID_id_GostR3410_2001);
bla-bla-bla...
//end emulating
EVP_PKEY_CTX *ctx = EVP_PKEY_CTX_new(pkey, e);
}
Result ctx is always NULL.
It happens because ENGINE_get_pkey_asn1_meth() returns pointer to corrupted
internall ccgost structure ameth_GostR3410_2001.
It IS initialized, then being freed. And contains garbage. Specifically
pkey-ameth-pkey_id contains some random value instead of 811
(NID_id_GostR3410_2001).
Is this code contain some error or invalid engine API usage?
On 4 January 2011 06:23, Dr. Stephen Henson st...@openssl.org wrote:
On Tue, Jan 04, 2011, Andrey Kulikov wrote:
If we take a look at any ENGINE_load_XXX function, we find that they all
has
similar structure:
ENGINE *toadd = engine_XXX();
if(!toadd) return;
ENGINE_add(toadd);
ENGINE_free(toadd);
ERR_clear_error();
My question is: why we need call ENGINE_free(toadd) ??
To avoid a memory leak.
When you call engine_XXX() you get a reference to the ENGINE. When it is
added
to the ENGINE list the reference count is incremented. When you call
ENGINE_free() the count is decremented and the ENGINE is freed only if the
reference count is zero.
Steve.
Re: [CVS] OpenSSL: OpenSSL_1_0_1-stable: openssl/crypto/x509v3/ v3_asid.c
On Mon, Jan 03, 2011 at 01:52:11PM +0100, Dr. Stephen Henson wrote:
OpenSSL CVS Repository
http://cvs.openssl.org/
Server: cvs.openssl.org Name: Dr. Stephen Henson
Root: /v/openssl/cvs Email: st...@openssl.org
Module: openssl Date: 03-Jan-2011 13:52:11
Branch: OpenSSL_1_0_1-stable Handle: 2011010312521100
Modified files: (Branch: OpenSSL_1_0_1-stable)
openssl/crypto/x509v3 v3_asid.c
Log:
oops missed an assert
Summary:
RevisionChanges Path
1.5.6.3 +1 -1 openssl/crypto/x509v3/v3_asid.c
patch -p0 '@@ .'
Index: openssl/crypto/x509v3/v3_asid.c
$ cvs diff -u -r1.5.6.2 -r1.5.6.3 v3_asid.c
--- openssl/crypto/x509v3/v3_asid.c 3 Jan 2011 01:40:45 - 1.5.6.2
+++ openssl/crypto/x509v3/v3_asid.c 3 Jan 2011 12:52:11 - 1.5.6.3
@@ -799,7 +799,7 @@
/*
* Trust anchor can't inherit.
*/
- assert(x != NULL);
+ OPENSSL_assert(x != NULL);
if (x-rfc3779_asid != NULL) {
if (x-rfc3779_asid-asnum != NULL
x-rfc3779_asid-asnum-type == ASIdentifierChoice_inherit)
@@ .
__
OpenSSL Project http://www.openssl.org
CVS Repository Commit List openssl-...@openssl.org
Automated List Manager majord...@openssl.org
This works!! Thank you again!!
--
Member - Liberal International This is doc...@nl2k.ab.ca Ici doc...@nl2k.ab.ca
God, Queen and country! Never Satan President Republic! Beware AntiChrist
rising!
http://twitter.com/rootnl2k http://www.facebook.com/dyadallee
Birthdate 29 Jan 1969 Redhill Surrey England UK
__
OpenSSL Project http://www.openssl.org
Development Mailing List openssl-dev@openssl.org
Automated List Manager majord...@openssl.org
