Filter short DH key sizes?

2014-03-13 Thread Hanno Böck
Hello,

I recently had a look at how browsers react to DH key exchanges with
bogus modulus values. here's what I found:
http://blog.hboeck.de/archives/841-Diffie-Hellman-and-TLS-with-nonsense-parameters.html

And here is a test (warning: crashes some chrome/chromium versions)
https://dh.tlsfun.de/

I wanted to bring this up here, because some openssl-based browser
accept just about anything for the DH prime setting (including
completely bogus values like 15).

NSS seems to filter very small values (below 512). I wonder if I should
report this to the browsers or if this is something openssl should fix.

My suggestion would be that openssl as a client just rejects all DH
parameters below 1024 bit. (I'd like to say reject below 2048, but I
know that's not feasible - at least not today)

To give some context: It is not immediately a security issue to allow
insecure DH parameters, because usually TLS is used to protect
connections between two parties that should trust each other.
However, the recent triple handshake issue brought up a problem that
exploited weak DH parameters. But it is important to say that there is
more than one way to weaken DH parameters and not all of them can be
tested in a reasonable way by the client. (e.g. testing if a prime
really is a prime is not efficiently possible for large key exchanges -
and there are also weak primes)

cu,
-- 
Hanno Böck
http://hboeck.de/

mail/jabber: ha...@hboeck.de
GPG: BBB51E42


signature.asc
Description: PGP signature


[PATCH] [openssl.org #3120] Reject DHE groups with 1024-bits

2014-03-13 Thread Daniel Kahn Gillmor via RT
This is a hard-coded patch to make OpenSSL clients reject connections
which use DHE handshakes with  1024 bits.

This patch has no compile-time or runtime configurability.  If the
project wants something more nuanced, we need discussion about what
the right form(s) of configurability should be.

Note that ssltest has also been changed to default to a 1024-bit
(instead of 512-bit) safe-prime DHE so that tests all pass
---
 ssl/s3_clnt.c |  5 +
 ssl/ssl.h |  1 +
 ssl/ssl_err.c |  3 ++-
 ssl/ssltest.c | 13 +++--
 4 files changed, 19 insertions(+), 3 deletions(-)

diff --git a/ssl/s3_clnt.c b/ssl/s3_clnt.c
index 9755a0f..7f0d14a 100644
--- a/ssl/s3_clnt.c
+++ b/ssl/s3_clnt.c
@@ -2635,6 +2635,11 @@ int ssl3_send_client_key_exchange(SSL *s)
else
{
/* generate a new random key */
+   if (DH_size(dh_srvr)  1024/8)
+   {
+   
SSLerr(SSL_F_SSL3_SEND_CLIENT_KEY_EXCHANGE,SSL_R_BAD_DH_WEAK_GROUP);
+   goto err;
+   }
if ((dh_clnt=DHparams_dup(dh_srvr)) == NULL)
{

SSLerr(SSL_F_SSL3_SEND_CLIENT_KEY_EXCHANGE,ERR_R_DH_LIB);
diff --git a/ssl/ssl.h b/ssl/ssl.h
index c6cd6a9..8bcd7ca 100644
--- a/ssl/ssl.h
+++ b/ssl/ssl.h
@@ -2826,6 +2826,7 @@ void ERR_load_SSL_strings(void);
 #define SSL_R_BAD_DH_G_LENGTH   108
 #define SSL_R_BAD_DH_PUB_KEY_LENGTH 109
 #define SSL_R_BAD_DH_P_LENGTH   110
+#define SSL_R_BAD_DH_WEAK_GROUP 394
 #define SSL_R_BAD_DIGEST_LENGTH 111
 #define SSL_R_BAD_DSA_SIGNATURE 112
 #define SSL_R_BAD_ECC_CERT  304
diff --git a/ssl/ssl_err.c b/ssl/ssl_err.c
index e663483..24bc75c 100644
--- a/ssl/ssl_err.c
+++ b/ssl/ssl_err.c
@@ -1,6 +1,6 @@
 /* ssl/ssl_err.c */
 /* 
- * Copyright (c) 1999-2013 The OpenSSL Project.  All rights reserved.
+ * Copyright (c) 1999-2014 The OpenSSL Project.  All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
@@ -327,6 +327,7 @@ static ERR_STRING_DATA SSL_str_reasons[]=
 {ERR_REASON(SSL_R_BAD_DH_G_LENGTH)   ,bad dh g length},
 {ERR_REASON(SSL_R_BAD_DH_PUB_KEY_LENGTH) ,bad dh pub key length},
 {ERR_REASON(SSL_R_BAD_DH_P_LENGTH)   ,bad dh p length},
+{ERR_REASON(SSL_R_BAD_DH_WEAK_GROUP) ,bad dh weak group},
 {ERR_REASON(SSL_R_BAD_DIGEST_LENGTH) ,bad digest length},
 {ERR_REASON(SSL_R_BAD_DSA_SIGNATURE) ,bad dsa signature},
 {ERR_REASON(SSL_R_BAD_ECC_CERT)  ,bad ecc cert},
diff --git a/ssl/ssltest.c b/ssl/ssltest.c
index 64c6743..809abf3 100644
--- a/ssl/ssltest.c
+++ b/ssl/ssltest.c
@@ -870,7 +870,8 @@ static void sv_usage(void)
fprintf(stderr, -num val- number of connections to perform\n);
fprintf(stderr, -bytes val  - number of bytes to swap between 
client/server\n);
 #ifndef OPENSSL_NO_DH
-   fprintf(stderr, -dhe1024  - use 1024 bit key (safe prime) for 
DHE\n);
+   fprintf(stderr, -dhe512   - use 512 bit key (safe prime) for 
DHE\n);
+   fprintf(stderr, -dhe1024  - use 1024 bit key (safe prime) for DHE 
(default)\n);
fprintf(stderr, -dhe1024dsa   - use 1024 bit key (with 160-bit 
subprime) for DHE\n);
fprintf(stderr, -no_dhe   - disable DHE\n);
 #endif
@@ -1079,7 +1080,7 @@ int main(int argc, char *argv[])
long bytes=256L;
 #ifndef OPENSSL_NO_DH
DH *dh;
-   int dhe1024 = 0, dhe1024dsa = 0;
+   int dhe1024 = 1, dhe1024dsa = 0;
 #endif
 #ifndef OPENSSL_NO_ECDH
EC_KEY *ecdh = NULL;
@@ -1164,6 +1165,14 @@ int main(int argc, char *argv[])
debug=1;
else if (strcmp(*argv,-reuse) == 0)
reuse=1;
+   else if (strcmp(*argv,-dhe512) == 0)
+   {
+#ifndef OPENSSL_NO_DH
+   dhe1024=0;
+#else
+   fprintf(stderr,ignoring -dhe512, since I'm compiled 
without DH\n);
+#endif
+   }
else if (strcmp(*argv,-dhe1024) == 0)
{
 #ifndef OPENSSL_NO_DH
-- 
1.9.0


__
OpenSSL Project http://www.openssl.org
Development Mailing List   openssl-dev@openssl.org
Automated List Manager   majord...@openssl.org


Re: Filter short DH key sizes?

2014-03-13 Thread Daniel Kahn Gillmor
On 03/13/2014 06:33 AM, Hanno Böck wrote:

 I recently had a look at how browsers react to DH key exchanges with
 bogus modulus values. here's what I found:
 http://blog.hboeck.de/archives/841-Diffie-Hellman-and-TLS-with-nonsense-parameters.html
 
 And here is a test (warning: crashes some chrome/chromium versions)
 https://dh.tlsfun.de/
 
 I wanted to bring this up here, because some openssl-based browser
 accept just about anything for the DH prime setting (including
 completely bogus values like 15).

thanks for raising this again, Hanno.  I agree that this is a concern.

 NSS seems to filter very small values (below 512). I wonder if I should
 report this to the browsers or if this is something openssl should fix.
 
 My suggestion would be that openssl as a client just rejects all DH
 parameters below 1024 bit. (I'd like to say reject below 2048, but I
 know that's not feasible - at least not today)

https://rt.openssl.org/Ticket/Display.html?id=3120  (from August 31st of
last year) has the same suggestion.  Speaking with folks at the Real
World Crypto conference in January 2014, the consensus was that a limit
of 512-bits was far too low.

I agree that a minimum of 1024 bits for the client is a good idea
(though still rather weak and also overdue).  The question is whether
there should be any additional API or compile-time option to set or
adjust or remove these limits.

In theory, users of OpenSSL as a TLS client are already able to query
the size of the DH key exchange for any given connection, and can choose
to terminate it if they object to the size of the group (or any other
properties of the group).

But in practice most clients don't, and expecting every client to know
enough and be prudent enough to manage this is unrealistic, so having a
safe default in openssl is a good idea.

I've sent a patch to #3120 without any compile-time or run-time
configurability, just a hard-coded limit.  I'm open to discussion about
ways to add configurability or better ways to dynamically select a
plausible limit.

--dkg
commit 04b8b7fbc84d1a28a43a86288d5216d794110a5e
Author: Daniel Kahn Gillmor d...@fifthhorseman.net
Date:   Thu Mar 13 13:23:50 2014 -0400

Reject DHE groups with  1024-bits

This is a hard-coded patch without any compile-time or runtime
configurability.  If the project wants something more nuanced, we need
discussion about what the right form(s) of configurability should be.

note that ssltest has also been changed to default to a 1024-bit
(instead of 512-bit) safe-prime DHE.

diff --git a/ssl/s3_clnt.c b/ssl/s3_clnt.c
index 9755a0f..7f0d14a 100644
--- a/ssl/s3_clnt.c
+++ b/ssl/s3_clnt.c
@@ -2635,6 +2635,11 @@ int ssl3_send_client_key_exchange(SSL *s)
 			else
 {
 /* generate a new random key */
+if (DH_size(dh_srvr)  1024/8)
+	{
+	SSLerr(SSL_F_SSL3_SEND_CLIENT_KEY_EXCHANGE,SSL_R_BAD_DH_WEAK_GROUP);
+	goto err;
+	}
 if ((dh_clnt=DHparams_dup(dh_srvr)) == NULL)
 	{
 	SSLerr(SSL_F_SSL3_SEND_CLIENT_KEY_EXCHANGE,ERR_R_DH_LIB);
diff --git a/ssl/ssl.h b/ssl/ssl.h
index c6cd6a9..8bcd7ca 100644
--- a/ssl/ssl.h
+++ b/ssl/ssl.h
@@ -2826,6 +2826,7 @@ void ERR_load_SSL_strings(void);
 #define SSL_R_BAD_DH_G_LENGTH 108
 #define SSL_R_BAD_DH_PUB_KEY_LENGTH			 109
 #define SSL_R_BAD_DH_P_LENGTH 110
+#define SSL_R_BAD_DH_WEAK_GROUP 394
 #define SSL_R_BAD_DIGEST_LENGTH 111
 #define SSL_R_BAD_DSA_SIGNATURE 112
 #define SSL_R_BAD_ECC_CERT 304
diff --git a/ssl/ssl_err.c b/ssl/ssl_err.c
index e663483..24bc75c 100644
--- a/ssl/ssl_err.c
+++ b/ssl/ssl_err.c
@@ -1,6 +1,6 @@
 /* ssl/ssl_err.c */
 /* 
- * Copyright (c) 1999-2013 The OpenSSL Project.  All rights reserved.
+ * Copyright (c) 1999-2014 The OpenSSL Project.  All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
@@ -327,6 +327,7 @@ static ERR_STRING_DATA SSL_str_reasons[]=
 {ERR_REASON(SSL_R_BAD_DH_G_LENGTH)   ,bad dh g length},
 {ERR_REASON(SSL_R_BAD_DH_PUB_KEY_LENGTH) ,bad dh pub key length},
 {ERR_REASON(SSL_R_BAD_DH_P_LENGTH)   ,bad dh p length},
+{ERR_REASON(SSL_R_BAD_DH_WEAK_GROUP) ,bad dh weak group},
 {ERR_REASON(SSL_R_BAD_DIGEST_LENGTH) ,bad digest length},
 {ERR_REASON(SSL_R_BAD_DSA_SIGNATURE) ,bad dsa signature},
 {ERR_REASON(SSL_R_BAD_ECC_CERT)  ,bad ecc cert},
diff --git a/ssl/ssltest.c b/ssl/ssltest.c
index 64c6743..809abf3 100644
--- a/ssl/ssltest.c
+++ b/ssl/ssltest.c
@@ -870,7 +870,8 @@ static void sv_usage(void)
 	fprintf(stderr, -num val- number of connections to perform\n);
 	fprintf(stderr, -bytes val  - number of bytes to swap between client/server\n);
 #ifndef OPENSSL_NO_DH
-	fprintf(stderr, -dhe1024  - use 1024 bit key (safe prime) for DHE\n);
+	fprintf(stderr, -dhe512   - use 512 bit key (safe prime) for DHE\n);
+	

Re: Filter short DH key sizes?

2014-03-13 Thread Daniel Kahn Gillmor
On 03/13/2014 04:05 PM, Kurt Roeckx wrote:
 On Thu, Mar 13, 2014 at 03:13:01PM -0400, Daniel Kahn Gillmor wrote:
 In theory, users of OpenSSL as a TLS client are already able to query
 the size of the DH key exchange for any given connection, and can choose
 to terminate it if they object to the size of the group (or any other
 properties of the group).
 
 Last time I looked this information is in an internal structure
 not exposed to the client.

hm, i also never figured out how a client is could possibly do it, which
suggests even more strongly that OpenSSL is failing to keep its users
secure.

But even if it turns out that there is some way that the client can get
to this information (as there is in other libraries, e.g.
gnutls_dh_get_prime_bits()), i don't think we can or should expect each
application to do this for each connection.  OpenSSL needs to default to
a mode where the connection is secure, and only move out of that if the
user or application explicitly loosens the security along some specific
axis (e.g. an SMTP daemon explicitly enabling anonymous ciphersuites as
discussed in the TLS WG right now).

--dkg





signature.asc
Description: OpenPGP digital signature


Re: Filter short DH key sizes?

2014-03-13 Thread Dr. Stephen Henson
On Thu, Mar 13, 2014, Kurt Roeckx wrote:

 On Thu, Mar 13, 2014 at 03:13:01PM -0400, Daniel Kahn Gillmor wrote:
  In theory, users of OpenSSL as a TLS client are already able to query
  the size of the DH key exchange for any given connection, and can choose
  to terminate it if they object to the size of the group (or any other
  properties of the group).
 
 Last time I looked this information is in an internal structure
 not exposed to the client.
 

It can be accessed with an API call but only in OpenSSL 1.0.2.

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


[openssl.org #3120] Minimum size of DH

2014-03-13 Thread Stephen Henson via RT
On Thu Mar 13 20:12:38 2014, d...@fifthhorseman.net wrote:
 This is a hard-coded patch to make OpenSSL clients reject connections
 which use DHE handshakes with  1024 bits.


I should've commented on this before, sorry. I'm currently working on a
framework where several security parameters can be configured at both compile
time and runtime, including DH parameter sizes. It's still under development at
present though. I'll commit it to the master branch when it's more stable.

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: [openssl.org #3120] Minimum size of DH

2014-03-13 Thread Daniel Kahn Gillmor via RT
On 03/13/2014 05:52 PM, Stephen Henson via RT wrote:
 I should've commented on this before, sorry. I'm currently working on a
 framework where several security parameters can be configured at both compile
 time and runtime, including DH parameter sizes. It's still under development 
 at
 present though. I'll commit it to the master branch when it's more stable.

Are those changes published on a git branch anywhere?  if not, is there
a reason to avoid publishing them?  you might get more feedback (and
review, and patches) if the work is more visible.

--dkg





signature.asc
Description: PGP signature