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");
+ 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
signature.asc
Description: OpenPGP digital signature
