[sr-dev] git:andrei/tcp_tls_changes: tls: fix wrong wbio usage
Andrei Pelinescu-Onciul
andrei at iptel.org
Fri Jun 4 18:37:10 CEST 2010
Module: sip-router
Branch: andrei/tcp_tls_changes
Commit: 01c803a6c081ab84ca0b4faf9d16575b23de1b20
URL: http://git.sip-router.org/cgi-bin/gitweb.cgi/sip-router/?a=commit;h=01c803a6c081ab84ca0b4faf9d16575b23de1b20
Author: Andrei Pelinescu-Onciul <andrei at iptel.org>
Committer: Andrei Pelinescu-Onciul <andrei at iptel.org>
Date: Fri Jun 4 18:30:48 2010 +0200
tls: fix wrong wbio usage
The openssl library sometimes (after write operations) sets a
buffering bio "over" the wbio that we set. In this case one can no
longer rely on the wbio returned by SSL_get_wbio() (it might be the
buffering bio).
Now the BIOs are set at connection initialization time (and not on
their first use) and are stored inside the tls_extra_data
structure attached to the tcp connection. This way we are sure we
are always controlling our wbio and not something that openssl
might have stacked on top of it.
---
modules/tls/tls_bio.c | 5 +++--
modules/tls/tls_server.c | 45 ++++++++++++++++++---------------------------
modules/tls/tls_server.h | 3 +++
3 files changed, 24 insertions(+), 29 deletions(-)
diff --git a/modules/tls/tls_bio.c b/modules/tls/tls_bio.c
index c82fe64..3581ef0 100644
--- a/modules/tls/tls_bio.c
+++ b/modules/tls/tls_bio.c
@@ -115,7 +115,7 @@ int tls_BIO_mbuf_set(BIO* b, struct tls_mbuf* rd, struct tls_mbuf* wr)
{
struct tls_bio_mbuf_data* d;
- TLS_BIO_DBG("tls_BIO_muf_set called (%p, %p)\n", rd, wr);
+ TLS_BIO_DBG("tls_BIO_mbuf_set called (%p => %p, %p)\n", b, rd, wr);
if (unlikely(b->ptr == 0)){
BUG("null BIO ptr\n");
return 0;
@@ -195,7 +195,8 @@ static int tls_bio_mbuf_read(BIO* b, char* dst, int dst_len)
as a shortcut when no data is available =>
simulate EAGIAN/WANT_READ */
TLS_BIO_DBG("read (%p, %p, %d) called with null read buffer"
- " => simulating EAGAIN/WANT_READ\n", b, dst, dst_len);
+ "(%p->%p) => simulating EAGAIN/WANT_READ\n",
+ b, dst, dst_len, d, d->rd);
BIO_set_retry_read(b);
}
return -1;
diff --git a/modules/tls/tls_server.c b/modules/tls/tls_server.c
index c5c36de..981cf41 100644
--- a/modules/tls/tls_server.c
+++ b/modules/tls/tls_server.c
@@ -126,11 +126,16 @@ static int tls_complete_init(struct tcp_connection* c)
}
memset(data, '\0', sizeof(struct tls_extra_data));
data->ssl = SSL_new(dom->ctx[process_no]);
+ data->rwbio = tls_BIO_new_mbuf(0, 0);
data->cfg = cfg;
data->state = state;
- if (data->ssl == 0) {
- TLS_ERR("Failed to create SSL structure:");
+ if (unlikely(data->ssl == 0 || data->rwbio == 0)) {
+ TLS_ERR("Failed to create SSL or BIO structure:");
+ if (data->ssl)
+ SSL_free(data->ssl);
+ if (data->rwbio)
+ BIO_free(data->rwbio);
goto error;
}
#ifdef TLS_KSSL_WORKARROUND
@@ -140,6 +145,7 @@ static int tls_complete_init(struct tcp_connection* c)
data->ssl->kssl_ctx=0;
}
#endif
+ SSL_set_bio(data->ssl, data->rwbio, data->rwbio);
c->extra_data = data;
return 0;
@@ -182,25 +188,9 @@ static int tls_set_mbufs(struct tcp_connection *c,
struct tls_mbuf* rd,
struct tls_mbuf* wr)
{
- SSL *ssl;
BIO *rwbio;
- /* if (unlikely(tls_fix_connection(c) < 0))
- return -1;
- */
-
- ssl = ((struct tls_extra_data*)c->extra_data)->ssl;
- if (unlikely(((rwbio=SSL_get_rbio(ssl))==0) ||
- ((rwbio=SSL_get_wbio(ssl))==0))) {
- rwbio = tls_BIO_new_mbuf(rd, wr);
- if (unlikely(rwbio == 0)) {
- ERR("new mbuf BIO creation failure\n");
- return -1;
- }
- /* use the same bio for both read & write */
- SSL_set_bio(ssl, rwbio, rwbio);
- return 0;
- }
+ rwbio = ((struct tls_extra_data*)c->extra_data)->rwbio;
if (unlikely(tls_BIO_mbuf_set(rwbio, rd, wr)<=0)) {
/* it should be always 1 */
ERR("failed to set mbufs");
@@ -875,7 +865,7 @@ redo_read:
*/
if (unlikely(tls_c->enc_rd_buf)) {
/* use queued data */
- /* safe to use without locks, because only read changes it and
+ /* safe to use without locks, because only read changes it and
there can't be parallel reads on the same connection */
enc_rd_buf = tls_c->enc_rd_buf;
tls_c->enc_rd_buf = 0;
@@ -980,8 +970,8 @@ ssl_read_skipped:
goto error_send;
}
}
- /* quickly catch bugs: segfault if accessed and not set */
- tls_set_mbufs(c, 0, 0);
+ /* quickly catch bugs: segfault if accessed and not set */
+ tls_set_mbufs(c, 0, 0);
lock_release(&c->write_lock);
switch(ssl_error) {
case SSL_ERROR_NONE:
@@ -1027,11 +1017,12 @@ ssl_read_skipped:
if (unlikely(n < 0))
/* here n should always be >= 0 */
BUG("unexpected value (n = %d)\n", n);
- else if (unlikely(n < bytes_free))
- BUG("read buffer not exhausted (rbio still has %d bytes,"
- "last SSL_read %d / %d)\n",
- rd.used - rd.pos, n, bytes_free);
- else if (n == bytes_free) {
+ else {
+ if (unlikely(n < bytes_free))
+ BUG("read buffer not exhausted (rbio still has %d bytes,"
+ "last SSL_read %d / %d)\n",
+ rd.used - rd.pos, n, bytes_free);
+ /* n <= bytes_free */
/* queue read data if not fully consumed by SSL_read()
* (very unlikely situation)
*/
diff --git a/modules/tls/tls_server.h b/modules/tls/tls_server.h
index 66eb962..2a324bd 100644
--- a/modules/tls/tls_server.h
+++ b/modules/tls/tls_server.h
@@ -55,6 +55,9 @@ struct tls_rd_buf {
struct tls_extra_data {
tls_domains_cfg_t* cfg; /* Configuration used for this connection */
SSL* ssl; /* SSL context used for the connection */
+ BIO* rwbio; /* bio used for read/write
+ (openssl code might add buffering BIOs so
+ it's better to remember our original BIO) */
tls_ct_q* ct_wq;
struct tls_rd_buf* enc_rd_buf;
unsigned int flags;
More information about the sr-dev
mailing list