From 07df5018befd637a2a02ebd767969aa953200b3f Mon Sep 17 00:00:00 2001 From: Rob Stradling Date: Mon, 9 Sep 2013 10:44:29 +0100 Subject: [PATCH 1/6] Don't prefer ECDHE-ECDSA ciphers when the client appears to be Safari on OS X. OS X 10.8..10.8.3 has broken support for ECDHE-ECDSA ciphers. --- doc/ssl/SSL_CTX_set_options.pod | 5 +- ssl/s3_lib.c | 16 ++++-- ssl/ssl.h | 2 +- ssl/ssl3.h | 10 +++- ssl/t1_lib.c | 89 +++++++++++++++++++++++++++++++++ 5 files changed, 115 insertions(+), 7 deletions(-) diff --git a/doc/ssl/SSL_CTX_set_options.pod b/doc/ssl/SSL_CTX_set_options.pod index cc588f3a7..fded0601b 100644 --- a/doc/ssl/SSL_CTX_set_options.pod +++ b/doc/ssl/SSL_CTX_set_options.pod @@ -88,9 +88,10 @@ As of OpenSSL 0.9.8q and 1.0.0c, this option has no effect. ... -=item SSL_OP_MSIE_SSLV2_RSA_PADDING +=item SSL_OP_SAFARI_ECDHE_ECDSA_BUG -As of OpenSSL 0.9.7h and 0.9.8a, this option has no effect. +Don't prefer ECDHE-ECDSA ciphers when the client appears to be Safari on OS X. +OS X 10.8..10.8.3 has broken support for ECDHE-ECDSA ciphers. =item SSL_OP_SSLEAY_080_CLIENT_DH_BUG diff --git a/ssl/s3_lib.c b/ssl/s3_lib.c index f3acb8a96..d70286612 100644 --- a/ssl/s3_lib.c +++ b/ssl/s3_lib.c @@ -3066,7 +3066,10 @@ void ssl3_clear(SSL *s) s->s3->tlsext_custom_types = NULL; } s->s3->tlsext_custom_types_count = 0; -#endif +#ifndef OPENSSL_NO_EC + s->s3->is_probably_safari = 0; +#endif /* OPENSSL_NO_EC */ +#endif /* OPENSSL_NO_TLSEXT */ rp = s->s3->rbuf.buf; wp = s->s3->wbuf.buf; @@ -4129,8 +4132,15 @@ SSL_CIPHER *ssl3_choose_cipher(SSL *s, STACK_OF(SSL_CIPHER) *clnt, ii=sk_SSL_CIPHER_find(allow,c); if (ii >= 0) { - ret=sk_SSL_CIPHER_value(allow,ii); - break; + if ((alg_k & SSL_kEECDH) && (alg_a & SSL_aECDSA) && s->s3->is_probably_safari) + { + if (!ret) ret=sk_SSL_CIPHER_value(allow,ii); + } + else + { + ret=sk_SSL_CIPHER_value(allow,ii); + break; + } } } return(ret); diff --git a/ssl/ssl.h b/ssl/ssl.h index bd2b57630..e8f250ed3 100644 --- a/ssl/ssl.h +++ b/ssl/ssl.h @@ -615,7 +615,7 @@ struct ssl_session_st #define SSL_OP_NETSCAPE_REUSE_CIPHER_CHANGE_BUG 0x00000008L #define SSL_OP_SSLREF2_REUSE_CERT_TYPE_BUG 0x00000010L #define SSL_OP_MICROSOFT_BIG_SSLV3_BUFFER 0x00000020L -#define SSL_OP_MSIE_SSLV2_RSA_PADDING 0x00000040L /* no effect since 0.9.7h and 0.9.8b */ +#define SSL_OP_SAFARI_ECDHE_ECDSA_BUG 0x00000040L #define SSL_OP_SSLEAY_080_CLIENT_DH_BUG 0x00000080L #define SSL_OP_TLS_D5_BUG 0x00000100L #define SSL_OP_TLS_BLOCK_PADDING_BUG 0x00000200L diff --git a/ssl/ssl3.h b/ssl/ssl3.h index 171c76a73..56416078a 100644 --- a/ssl/ssl3.h +++ b/ssl/ssl3.h @@ -580,7 +580,15 @@ typedef struct ssl3_state_st * as the types were received in the client hello. */ unsigned short *tlsext_custom_types; size_t tlsext_custom_types_count; /* how many tlsext_custom_types */ -#endif + +#ifndef OPENSSL_NO_EC + /* This is set to true if we believe that this is a version of Safari + * running on OS X 10.6 or newer. We wish to know this because Safari + * on 10.8 .. 10.8.3 has broken ECDHE-ECDSA support. */ + char is_probably_safari; +#endif /* OPENSSL_NO_EC */ + +#endif /* OPENSSL_NO_TLSEXT */ } SSL3_STATE; #endif diff --git a/ssl/t1_lib.c b/ssl/t1_lib.c index 10f094fcd..1992b918b 100644 --- a/ssl/t1_lib.c +++ b/ssl/t1_lib.c @@ -1762,6 +1762,89 @@ unsigned char *ssl_add_serverhello_tlsext(SSL *s, unsigned char *p, unsigned cha return ret; } +#ifndef OPENSSL_NO_EC +/* ssl_check_for_safari attempts to fingerprint Safari using OS X + * SecureTransport using the TLS extension block in |d|, of length |n|. + * Safari, since 10.6, sends exactly these extensions, in this order: + * SNI, + * elliptic_curves + * ec_point_formats + * + * We wish to fingerprint Safari because they broke ECDHE-ECDSA support in 10.8, + * but they advertise support. So enabling ECDHE-ECDSA ciphers breaks them. + * Sadly we cannot differentiate 10.6, 10.7 and 10.8.4 (which work), from + * 10.8..10.8.3 (which don't work). + */ +static void ssl_check_for_safari(SSL *s, const unsigned char *data, const unsigned char *d, int n) { + unsigned short type, size; + static const unsigned char kSafariExtensionsBlock[] = { + 0x00, 0x0a, /* elliptic_curves extension */ + 0x00, 0x08, /* 8 bytes */ + 0x00, 0x06, /* 6 bytes of curve ids */ + 0x00, 0x17, /* P-256 */ + 0x00, 0x18, /* P-384 */ + 0x00, 0x19, /* P-521 */ + + 0x00, 0x0b, /* ec_point_formats */ + 0x00, 0x02, /* 2 bytes */ + 0x01, /* 1 point format */ + 0x00, /* uncompressed */ + }; + + /* The following is only present in TLS 1.2 */ + static const unsigned char kSafariTLS12ExtensionsBlock[] = { + 0x00, 0x0d, /* signature_algorithms */ + 0x00, 0x0c, /* 12 bytes */ + 0x00, 0x0a, /* 10 bytes */ + 0x05, 0x01, /* SHA-384/RSA */ + 0x04, 0x01, /* SHA-256/RSA */ + 0x02, 0x01, /* SHA-1/RSA */ + 0x04, 0x03, /* SHA-256/ECDSA */ + 0x02, 0x03, /* SHA-1/ECDSA */ + }; + + if (data >= (d+n-2)) + return; + data += 2; + + if (data > (d+n-4)) + return; + n2s(data,type); + n2s(data,size); + + if (type != TLSEXT_TYPE_server_name) + return; + + if (data+size > d+n) + return; + data += size; + + if (TLS1_get_version(s) >= TLS1_2_VERSION) + { + const size_t len1 = sizeof(kSafariExtensionsBlock); + const size_t len2 = sizeof(kSafariTLS12ExtensionsBlock); + + if (data + len1 + len2 != d+n) + return; + if (memcmp(data, kSafariExtensionsBlock, len1) != 0) + return; + if (memcmp(data + len1, kSafariTLS12ExtensionsBlock, len2) != 0) + return; + } + else + { + const size_t len = sizeof(kSafariExtensionsBlock); + + if (data + len != d+n) + return; + if (memcmp(data, kSafariExtensionsBlock, len) != 0) + return; + } + + s->s3->is_probably_safari = 1; +} +#endif /* OPENSSL_NO_EC */ + static int ssl_scan_clienthello_tlsext(SSL *s, unsigned char **p, unsigned char *d, int n, int *al) { unsigned short type; @@ -1781,6 +1864,12 @@ static int ssl_scan_clienthello_tlsext(SSL *s, unsigned char **p, unsigned char s->tlsext_heartbeat &= ~(SSL_TLSEXT_HB_ENABLED | SSL_TLSEXT_HB_DONT_SEND_REQUESTS); #endif + +#ifndef OPENSSL_NO_EC + if (s->options & SSL_OP_SAFARI_ECDHE_ECDSA_BUG) + ssl_check_for_safari(s, data, d, n); +#endif /* OPENSSL_NO_EC */ + /* Clear any signature algorithms extension received */ if (s->cert->peer_sigalgs) { From 5fa3b5478834029c69ad110e563eea13f1aa06e0 Mon Sep 17 00:00:00 2001 From: Rob Stradling Date: Tue, 10 Sep 2013 11:45:37 +0100 Subject: [PATCH 2/6] Fix compilation with no-ec and/or no-tlsext. --- ssl/s3_lib.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/ssl/s3_lib.c b/ssl/s3_lib.c index d70286612..26c0e96ae 100644 --- a/ssl/s3_lib.c +++ b/ssl/s3_lib.c @@ -4132,15 +4132,15 @@ SSL_CIPHER *ssl3_choose_cipher(SSL *s, STACK_OF(SSL_CIPHER) *clnt, ii=sk_SSL_CIPHER_find(allow,c); if (ii >= 0) { +#if !defined(OPENSSL_NO_EC) && !defined(OPENSSL_NO_TLSEXT) if ((alg_k & SSL_kEECDH) && (alg_a & SSL_aECDSA) && s->s3->is_probably_safari) { if (!ret) ret=sk_SSL_CIPHER_value(allow,ii); + continue; } - else - { - ret=sk_SSL_CIPHER_value(allow,ii); - break; - } +#endif + ret=sk_SSL_CIPHER_value(allow,ii); + break; } } return(ret); From 6a0b803fc067efc271e2ccd20d12e69618bed0a2 Mon Sep 17 00:00:00 2001 From: Rob Stradling Date: Tue, 10 Sep 2013 11:46:42 +0100 Subject: [PATCH 3/6] Use TLS version supplied by client when fingerprinting Safari. --- ssl/t1_lib.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ssl/t1_lib.c b/ssl/t1_lib.c index 1992b918b..a240587ee 100644 --- a/ssl/t1_lib.c +++ b/ssl/t1_lib.c @@ -1819,7 +1819,7 @@ static void ssl_check_for_safari(SSL *s, const unsigned char *data, const unsign return; data += size; - if (TLS1_get_version(s) >= TLS1_2_VERSION) + if (TLS1_get_client_version(s) >= TLS1_2_VERSION) { const size_t len1 = sizeof(kSafariExtensionsBlock); const size_t len2 = sizeof(kSafariTLS12ExtensionsBlock); From 9409e1817901c4f323ce558e0bee1171adac5cf4 Mon Sep 17 00:00:00 2001 From: Rob Stradling Date: Tue, 10 Sep 2013 11:50:05 +0100 Subject: [PATCH 4/6] Tidy up comments. --- ssl/s3_lib.c | 4 ++-- ssl/ssl3.h | 4 ++-- ssl/t1_lib.c | 4 ++-- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/ssl/s3_lib.c b/ssl/s3_lib.c index 26c0e96ae..2250faa2e 100644 --- a/ssl/s3_lib.c +++ b/ssl/s3_lib.c @@ -3068,8 +3068,8 @@ void ssl3_clear(SSL *s) s->s3->tlsext_custom_types_count = 0; #ifndef OPENSSL_NO_EC s->s3->is_probably_safari = 0; -#endif /* OPENSSL_NO_EC */ -#endif /* OPENSSL_NO_TLSEXT */ +#endif /* !OPENSSL_NO_EC */ +#endif /* !OPENSSL_NO_TLSEXT */ rp = s->s3->rbuf.buf; wp = s->s3->wbuf.buf; diff --git a/ssl/ssl3.h b/ssl/ssl3.h index 56416078a..a142bbf30 100644 --- a/ssl/ssl3.h +++ b/ssl/ssl3.h @@ -586,9 +586,9 @@ typedef struct ssl3_state_st * running on OS X 10.6 or newer. We wish to know this because Safari * on 10.8 .. 10.8.3 has broken ECDHE-ECDSA support. */ char is_probably_safari; -#endif /* OPENSSL_NO_EC */ +#endif /* !OPENSSL_NO_EC */ -#endif /* OPENSSL_NO_TLSEXT */ +#endif /* !OPENSSL_NO_TLSEXT */ } SSL3_STATE; #endif diff --git a/ssl/t1_lib.c b/ssl/t1_lib.c index a240587ee..f13ca4948 100644 --- a/ssl/t1_lib.c +++ b/ssl/t1_lib.c @@ -1843,7 +1843,7 @@ static void ssl_check_for_safari(SSL *s, const unsigned char *data, const unsign s->s3->is_probably_safari = 1; } -#endif /* OPENSSL_NO_EC */ +#endif /* !OPENSSL_NO_EC */ static int ssl_scan_clienthello_tlsext(SSL *s, unsigned char **p, unsigned char *d, int n, int *al) { @@ -1868,7 +1868,7 @@ static int ssl_scan_clienthello_tlsext(SSL *s, unsigned char **p, unsigned char #ifndef OPENSSL_NO_EC if (s->options & SSL_OP_SAFARI_ECDHE_ECDSA_BUG) ssl_check_for_safari(s, data, d, n); -#endif /* OPENSSL_NO_EC */ +#endif /* !OPENSSL_NO_EC */ /* Clear any signature algorithms extension received */ if (s->cert->peer_sigalgs) From 86a66deb7ed2e9cc0e61036b2c9accc05e239de3 Mon Sep 17 00:00:00 2001 From: Rob Stradling Date: Thu, 12 Sep 2013 22:08:07 +0100 Subject: [PATCH 5/6] Update CHANGES. --- CHANGES | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/CHANGES b/CHANGES index 943080944..fc3b96c70 100644 --- a/CHANGES +++ b/CHANGES @@ -4,6 +4,14 @@ Changes between 1.0.1e and 1.0.2 [xx XXX xxxx] + *) Add option SSL_OP_SAFARI_ECDHE_ECDSA_BUG (part of SSL_OP_ALL) which + avoids preferring ECDHE-ECDSA ciphers when the client appears to be + Safari on OS X. Safari on OS X 10.8..10.8.3 advertises support for + several ECDHE-ECDSA ciphers, but fails to negotiate them. The bug + is fixed in OS X 10.8.4, but Apple have ruled out both hot fixing + 10.8..10.8.3 and forcing users to upgrade to 10.8.4 or newer. + [Rob Stradling, Adam Langley] + *) New functions OPENSSL_gmtime_diff and ASN1_TIME_diff to find the difference in days and seconds between two tm or ASN1_TIME structures. [Steve Henson] From 6da498991c43b23f2f0abc36338593c5492185f7 Mon Sep 17 00:00:00 2001 From: Trevor Perrin Date: Sat, 27 Jul 2013 23:10:14 -0700 Subject: [PATCH 6/6] Various custom extension fixes. Force no SSL2 when custom extensions in use. Don't clear extension state when cert is set. Clear on renegotiate. Conflicts: ssl/t1_lib.c --- ssl/s23_clnt.c | 4 +++- ssl/ssl_rsa.c | 21 ++------------------- ssl/t1_lib.c | 8 ++++++++ 3 files changed, 13 insertions(+), 20 deletions(-) diff --git a/ssl/s23_clnt.c b/ssl/s23_clnt.c index 2c38b1a76..15da654bf 100644 --- a/ssl/s23_clnt.c +++ b/ssl/s23_clnt.c @@ -340,7 +340,9 @@ static int ssl23_client_hello(SSL *s) if (s->ctx->tlsext_opaque_prf_input_callback != 0 || s->tlsext_opaque_prf_input != NULL) ssl2_compat = 0; #endif - if (s->ctx->tlsext_authz_server_audit_proof_cb != NULL) + if (s->ctx->tlsext_authz_server_audit_proof_cb != NULL) + ssl2_compat = 0; + if (s->ctx->custom_cli_ext_records_count != 0) ssl2_compat = 0; } #endif diff --git a/ssl/ssl_rsa.c b/ssl/ssl_rsa.c index 77abcfce8..2837624ae 100644 --- a/ssl/ssl_rsa.c +++ b/ssl/ssl_rsa.c @@ -463,23 +463,6 @@ static int ssl_set_cert(CERT *c, X509 *x) X509_free(c->pkeys[i].x509); CRYPTO_add(&x->references,1,CRYPTO_LOCK_X509); c->pkeys[i].x509=x; -#ifndef OPENSSL_NO_TLSEXT - /* Free the old authz data, if it exists. */ - if (c->pkeys[i].authz != NULL) - { - OPENSSL_free(c->pkeys[i].authz); - c->pkeys[i].authz = NULL; - c->pkeys[i].authz_length = 0; - } - - /* Free the old serverinfo data, if it exists. */ - if (c->pkeys[i].serverinfo != NULL) - { - OPENSSL_free(c->pkeys[i].serverinfo); - c->pkeys[i].serverinfo = NULL; - c->pkeys[i].serverinfo_length = 0; - } -#endif c->key= &(c->pkeys[i]); c->valid=0; @@ -1083,7 +1066,7 @@ int SSL_CTX_use_serverinfo(SSL_CTX *ctx, const unsigned char *serverinfo, if (!serverinfo_process_buffer(serverinfo, serverinfo_length, NULL)) { SSLerr(SSL_F_SSL_CTX_USE_SERVERINFO,SSL_R_INVALID_SERVERINFO_DATA); - return(0); + return 0; } if (!ssl_cert_inst(&ctx->cert)) { @@ -1110,7 +1093,7 @@ int SSL_CTX_use_serverinfo(SSL_CTX *ctx, const unsigned char *serverinfo, if (!serverinfo_process_buffer(serverinfo, serverinfo_length, ctx)) { SSLerr(SSL_F_SSL_CTX_USE_SERVERINFO,SSL_R_INVALID_SERVERINFO_DATA); - return(0); + return 0; } return 1; } diff --git a/ssl/t1_lib.c b/ssl/t1_lib.c index f13ca4948..17ce8b3f8 100644 --- a/ssl/t1_lib.c +++ b/ssl/t1_lib.c @@ -1860,6 +1860,14 @@ static int ssl_scan_clienthello_tlsext(SSL *s, unsigned char **p, unsigned char s->s3->next_proto_neg_seen = 0; #endif + /* Clear observed custom extensions */ + s->s3->tlsext_custom_types_count = 0; + if (s->s3->tlsext_custom_types != NULL) + { + OPENSSL_free(s->s3->tlsext_custom_types); + s->s3->tlsext_custom_types = NULL; + } + #ifndef OPENSSL_NO_HEARTBEATS s->tlsext_heartbeat &= ~(SSL_TLSEXT_HB_ENABLED | SSL_TLSEXT_HB_DONT_SEND_REQUESTS);