Nginx-ru mailing list archive (nginx-ru@sysoev.ru)
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH] Disable SSL renegotiation (CVE-2009-3555).
Hello!
On Fri, Nov 06, 2009 at 11:35:56PM +0300, Maxim Dounin wrote:
> Hello!
>
> On Fri, Nov 06, 2009 at 07:54:01PM +0300, Igor Sysoev wrote:
>
> > On Fri, Nov 06, 2009 at 05:41:11PM +0300, Maxim Dounin wrote:
> >
> > > Hello!
> > >
> > > On Fri, Nov 06, 2009 at 04:49:18PM +0300, Igor Sysoev wrote:
> > >
> > > > On Fri, Nov 06, 2009 at 04:28:52PM +0300, Maxim Dounin wrote:
> > > >
> > > > > Hello!
> > > > >
> > > > > Патч, запрещающий ssl renegotiation для старых версий openssl
> > > > > (ссылок по теме есть в самом патче если кто ещё не читал, ну и в
> > > > > гугле наливают).
> > > > >
> > > > > Я проверил это на openssl 0.9.7e (из freebsd 6.2), и текущем
> > > > > 0.9.8l (где renegotiation недоступна по умолчанию). Делает вид
> > > > > что работает.
> > > > >
> > > > > Если кто-то потестирует - будет замечательно.
> > > >
> > > > Спасибо. У меня renegotiating в openssl клиенте не происходит и клиент
> > > > ничего не хочет принимать. Насколько я понимаю, это проблема клианта
> > > > openssl:
> > >
> > > [...]
> > >
> > > Оно игнорирует ClientHello пакет, в результате клиент ждёт
> > > таймаута. По хорошему там надо бы послать обратно alert про
> > > NO_RENEGOTIATION, но я не нашёл способа это нормально сделать.
> > >
> > > В моих тестах openssl 0.9.8l (с запрещённым из коробки
> > > renegotiation) ведёт себя точно так же (без моего патча).
> >
> > По идее, это должна делать сама OpenSSL по флагу
> > SSL3_FLAGS_NO_RENEGOTIATE_CIPHERS, но судя по исходникам,
> > она в этом случае просто ничего не делает.
>
> Именно так, должен. Но не.
>
> Вообще по здравому размышлению и дополнительному
> инструментированию - патч негодный, равно как и собственно openssl
> 0.9.8l. Оно не позволяет случится renegotiation только потому что
> openssl утыкается в EAGAIN на чтении. А утыкается - судя по всему из-за
> того что разваливает свой стейт в неконсистентное состояние.
Да, при отключённом negotiation'е (0.9.8l или старые версии с
SSL3_FLAGS_NO_RENEGOTIATE_CIPHERS) openssl попадает в
ssl3_accept() с состоянием включающим SSL_ST_BEFORE, и в первых же
строках чистит s->s3, в том числе стирая информацию о
свежепрочитанном пакете. А дальше утыкается в попытку этот самый
пакет прочитать.
> Думаю, правильным решением будет просто дропать соединение в
> ngx_ssl_info_callback() по прописанному в патче условию.
Новый патч, который делает это.
Maxim Dounin
# HG changeset patch
# User Maxim Dounin <mdounin@xxxxxxxxxx>
# Date 1257605230 -10800
# Node ID a9c0099a86c0360b5229161ac86ddb4252f191ad
# Parent 8da5668048b423cb58a77b9d496956e9cad96709
Disable SSL renegotiation (CVE-2009-3555).
It was recently discovered that SSL and TLS are vulnerable to
man-in-the-middle attacks due to renegotiation feature (see
http://extendedsubset.com/?p=8).
Most recent version of openssl (0.9.8l) disables renegotiation unless
explicitly enabled by application (not recommended though). Since nginx
doesn't require renegotiation to work - try to disable it for older
openssl versions too.
Note that openssl (up to most recent 0.9.8l) doesn't handle disabled
renegotiation gracefully (i.e. goes to an unconsistent state when client
asks for renegotiation instead of sending back NO_RENEGOTIATION alert)
and we have to explicitly drop connection if we detect renegotiation
attempt.
diff --git a/src/event/ngx_event_openssl.c b/src/event/ngx_event_openssl.c
--- a/src/event/ngx_event_openssl.c
+++ b/src/event/ngx_event_openssl.c
@@ -15,6 +15,8 @@ typedef struct {
static int ngx_http_ssl_verify_callback(int ok, X509_STORE_CTX *x509_store);
+static void ngx_ssl_info_callback(const ngx_ssl_conn_t *ssl_conn, int where,
+ int ret);
static void ngx_ssl_handshake_handler(ngx_event_t *ev);
static ngx_int_t ngx_ssl_handle_recv(ngx_connection_t *c, int n);
static void ngx_ssl_write_handler(ngx_event_t *wev);
@@ -175,6 +177,8 @@ ngx_ssl_create(ngx_ssl_t *ssl, ngx_uint_
SSL_CTX_set_read_ahead(ssl->ctx, 1);
+ SSL_CTX_set_info_callback(ssl->ctx, ngx_ssl_info_callback);
+
return NGX_OK;
}
@@ -349,6 +353,30 @@ ngx_http_ssl_verify_callback(int ok, X50
return 1;
}
+static void
+ngx_ssl_info_callback(const ngx_ssl_conn_t *ssl_conn, int where, int ret)
+{
+ ngx_connection_t *c;
+
+ if (where & SSL_CB_HANDSHAKE_START) {
+ c = ngx_ssl_get_connection((ngx_ssl_conn_t *) ssl_conn);
+
+ if (c->ssl->handshaked) {
+ ngx_log_debug0(NGX_LOG_DEBUG_EVENT, c->log, 0,
+ "unexpected SSL renegotiation detected");
+
+ c->ssl->renegotiation = 1;
+
+ /*
+ * rearm disable flag as openssl (as of 0.9.8l at least) loses
+ * it due to bug; this doesn't really matter since we will close
+ * this connection anyway, but just to be sure
+ */
+
+ c->ssl->connection->s3->flags |= SSL3_FLAGS_NO_RENEGOTIATE_CIPHERS;
+ }
+ }
+}
ngx_int_t
ngx_ssl_generate_rsa512_key(ngx_ssl_t *ssl)
@@ -587,6 +615,10 @@ ngx_ssl_handshake(ngx_connection_t *c)
c->recv_chain = ngx_ssl_recv_chain;
c->send_chain = ngx_ssl_send_chain;
+ /* initial handshake done, disable renegotiation (CVE-2009-3555) */
+
+ c->ssl->connection->s3->flags |= SSL3_FLAGS_NO_RENEGOTIATE_CIPHERS;
+
return NGX_OK;
}
@@ -789,6 +821,21 @@ ngx_ssl_handle_recv(ngx_connection_t *c,
int sslerr;
ngx_err_t err;
+ if (c->ssl->renegotiation) {
+ /*
+ * openssl (at least up to 0.9.8l) doesn't handle disabled
+ * renegotiation gracefully, so drop connection here
+ */
+
+ ngx_log_error(NGX_LOG_ALERT, c->log, 0,
+ "unexpected SSL renegotiation");
+
+ c->ssl->no_wait_shutdown = 1;
+ c->ssl->no_send_shutdown = 1;
+
+ return NGX_ERROR;
+ }
+
if (n > 0) {
if (c->ssl->saved_write_handler) {
diff --git a/src/event/ngx_event_openssl.h b/src/event/ngx_event_openssl.h
--- a/src/event/ngx_event_openssl.h
+++ b/src/event/ngx_event_openssl.h
@@ -41,6 +41,7 @@ typedef struct {
ngx_event_handler_pt saved_write_handler;
unsigned handshaked:1;
+ unsigned renegotiation:1;
unsigned buffer:1;
unsigned no_wait_shutdown:1;
unsigned no_send_shutdown:1;
|
