Skip to content

Commit

Permalink
Issue shellinabox#384: compatibility with OpenSSL 1.1.0
Browse files Browse the repository at this point in the history
* Direct usage of BIO struct members is removed for new versions of
  OpenSSL.
* Workaround for double BIO free in SSL_free() was updated to work
  with new and old OpenSSL versions.
* Note that this patch only fixes compatibilty when building with
  configure option "--disable-runtime-loading" (like it is done
  for Debia package.).
  • Loading branch information
KLuka committed Oct 8, 2016
1 parent d4bd77c commit d0d8c58
Show file tree
Hide file tree
Showing 2 changed files with 25 additions and 16 deletions.
39 changes: 23 additions & 16 deletions libhttp/ssl.c
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,7 @@ BIO_METHOD * (*BIO_f_buffer)(void);
void (*BIO_free_all)(BIO *);
BIO * (*BIO_new)(BIO_METHOD *);
BIO * (*BIO_new_socket)(int, int);
BIO * (*BIO_next)(BIO *);
BIO * (*BIO_pop)(BIO *);
BIO * (*BIO_push)(BIO *, BIO *);
#if defined(HAVE_OPENSSL_EC)
Expand Down Expand Up @@ -280,6 +281,7 @@ static void loadSSL(void) {
{ { &BIO_free_all }, "BIO_free_all" },
{ { &BIO_new }, "BIO_new" },
{ { &BIO_new_socket }, "BIO_new_socket" },
{ { &BIO_next }, "BIO_next" },
{ { &BIO_pop }, "BIO_pop" },
{ { &BIO_push }, "BIO_push" },
{ { &ERR_clear_error }, "ERR_clear_error" },
Expand Down Expand Up @@ -1013,31 +1015,38 @@ int sslPromoteToSSL(struct SSLSupport *ssl, SSL **sslHndl, int fd,
#endif
}

BIO *sslGetNextBIO(BIO *b) {
#if OPENSSL_VERSION_NUMBER <= 0x10100000L
return b->next_bio;
#else
return BIO_next(b);
#endif
}

void sslFreeHndl(SSL **sslHndl) {
#if defined(HAVE_OPENSSL)
if (*sslHndl) {
// OpenSSL does not always correctly perform reference counting for stacked
// BIOs. This is particularly a problem if an SSL connection has two
// different BIOs for the read and the write end, with one being a stacked
// derivative of the other. Unfortunately, this is exactly the scenario
// that we set up.
// that we set up with call to "BIO_push(readBIO, writeBIO)" in function
// "sslPromoteToSSL()".
// As a work-around, we un-stack the BIOs prior to freeing the SSL
// connection.
debug("[ssl] Freeing SSL handle.");
ERR_clear_error();
BIO *writeBIO, *readBIO;
check(writeBIO = SSL_get_wbio(*sslHndl));
check(readBIO = SSL_get_rbio(*sslHndl));
if (writeBIO != readBIO) {
if (readBIO->next_bio == writeBIO) {
// OK, that's exactly the bug we are looking for. We know how to
// fix it.
if (sslGetNextBIO(readBIO) == writeBIO) {
// OK, that's exactly the bug we are looking for. We know that
// writeBIO needs to be removed from readBIO chain.
debug("[ssl] Removing stacked write BIO!");
check(BIO_pop(readBIO) == writeBIO);
check(readBIO->references == 1);
check(writeBIO->references == 1);
check(!readBIO->next_bio);
check(!writeBIO->prev_bio);
} else if (readBIO->next_bio == writeBIO->next_bio &&
writeBIO->next_bio->prev_bio == writeBIO) {
check(!sslGetNextBIO(readBIO));
} else if (sslGetNextBIO(readBIO) == sslGetNextBIO(writeBIO)) {
// Things get even more confused, if the SSL handshake is aborted
// prematurely.
// OpenSSL appears to internally stack a BIO onto the read end that
Expand All @@ -1046,15 +1055,13 @@ void sslFreeHndl(SSL **sslHndl) {
// reading and one for writing). In this case, not only is the
// reference count wrong, but the chain of next_bio/prev_bio pairs
// is corrupted, too.
warn("[ssl] Removing stacked socket BIO!");
BIO *sockBIO;
check(sockBIO = BIO_pop(readBIO));
check(sockBIO == BIO_pop(writeBIO));
check(readBIO->references == 1);
check(writeBIO->references == 1);
check(sockBIO->references == 1);
check(!readBIO->next_bio);
check(!writeBIO->next_bio);
check(!sockBIO->prev_bio);
check(!sslGetNextBIO(readBIO));
check(!sslGetNextBIO(writeBIO));
check(!sslGetNextBIO(sockBIO));
BIO_free_all(sockBIO);
} else {
// We do not know, how to fix this situation. Something must have
Expand Down
2 changes: 2 additions & 0 deletions libhttp/ssl.h
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,7 @@ extern BIO_METHOD *(*x_BIO_f_buffer)(void);
extern void (*x_BIO_free_all)(BIO *);
extern BIO *(*x_BIO_new)(BIO_METHOD *);
extern BIO *(*x_BIO_new_socket)(int, int);
extern BIO *(*x_BIO_next)(BIO *);
extern BIO *(*x_BIO_pop)(BIO *);
extern BIO *(*x_BIO_push)(BIO *, BIO *);
#if defined(HAVE_OPENSSL_EC)
Expand Down Expand Up @@ -131,6 +132,7 @@ extern void *(*x_SSL_COMP_get_compression_methods)(void);
#define BIO_free_all x_BIO_free_all
#define BIO_new x_BIO_new
#define BIO_new_socket x_BIO_new_socket
#define BIO_next x_BIO_next
#define BIO_pop x_BIO_pop
#define BIO_push x_BIO_push
#define EC_KEY_free x_EC_KEY_free
Expand Down

0 comments on commit d0d8c58

Please sign in to comment.