Opened 4 years ago

Last modified 7 months ago

#13815 new enhancement

Attempt to port tor to Google's BoringSSL

Reported by: teor Owned by:
Priority: Medium Milestone: Tor: unspecified
Component: Core Tor/Tor Version: Tor: 0.2.6.1-alpha
Severity: Normal Keywords: tor-relay openssl boringssl post-deprecation
Cc: nickm Actual Points:
Parent ID: #26639 Points:
Reviewer: Sponsor:

Description

Split from #13415:

nickm:

Interesting. It looks like building with BoringSSL will require some actual porting to detect all the APIs they've removed, and to figure out whether we can replace them.

teor:

BoringSSL is even worse - it doesn't even have an openssl executable, only builds static libraries, and is a pain to configure correctly under our current config scripts.

I can't seem to stop it finding the system-supplied SSL, even when I provide it the BoringSSL directories.

I get the following warnings when I manually install BoringSSL into include/lib/bin dirs, and fake the openssl executable using the bssl executable:

(I've cleaned up some warnings that were irrelevant or trivial.)

CC src/common/crypto.o

src/common/crypto.c:170:12: warning: implicit declaration of function

'ENGINE_get_name' is invalid in C99 [-Wimplicit-function-declaration]

name = ENGINE_get_name(e);

src/common/crypto.c:171:10: warning: implicit declaration of function

'ENGINE_get_id' is invalid in C99 [-Wimplicit-function-declaration]

id = ENGINE_get_id(e);

src/common/crypto.c:186:15: warning: implicit declaration of function

'ENGINE_by_id' is invalid in C99 [-Wimplicit-function-declaration]

ENGINE *e = ENGINE_by_id("dynamic");

src/common/crypto.c:188:10: warning: implicit declaration of function

'ENGINE_ctrl_cmd_string' is invalid in C99
[-Wimplicit-function-declaration]

if (!ENGINE_ctrl_cmd_string(e, "ID", engine, 0)

src/common/crypto.c:227:31: warning: implicit declaration of function

'SSLeay_version' is invalid in C99 [-Wimplicit-function-declaration]

const char *raw_version = SSLeay_version(SSLEAY_VERSION);

src/common/crypto.c:227:46: error: use of undeclared identifier 'SSLEAY_VERSION'

const char *raw_version = SSLeay_version(SSLEAY_VERSION);

src/common/crypto.c:241:51: error: use of undeclared identifier

'OPENSSL_VERSION_TEXT'

parse_openssl_version_str(OPENSSL_VERSION_TEXT);

src/common/crypto.c:251:7: warning: implicit declaration of function

'RAND_get_rand_method' is invalid in C99 [-Wimplicit-function-declaration]

if (RAND_get_rand_method() != RAND_SSLeay()) {

src/common/crypto.c:251:33: warning: implicit declaration of function

'RAND_SSLeay' is invalid in C99 [-Wimplicit-function-declaration]

if (RAND_get_rand_method() != RAND_SSLeay()) {

src/common/crypto.c:255:5: warning: implicit declaration of function

'RAND_set_rand_method' is invalid in C99 [-Wimplicit-function-declaration]

RAND_set_rand_method(RAND_SSLeay());

src/common/crypto.c:291:9: warning: implicit declaration of function 'SSLeay' is

invalid in C99 [-Wimplicit-function-declaration]

if (SSLeay() == OPENSSL_VERSION_NUMBER &&

src/common/crypto.c:292:32: error: use of undeclared identifier 'SSLEAY_VERSION'

!strcmp(SSLeay_version(SSLEAY_VERSION), OPENSSL_VERSION_TEXT)) {

src/common/crypto.c:292:49: error: use of undeclared identifier

'OPENSSL_VERSION_TEXT'

!strcmp(SSLeay_version(SSLEAY_VERSION), OPENSSL_VERSION_TEXT)) {

CC src/common/crypto_s2k.o

src/common/crypto.c:294:57: error: use of undeclared identifier 'SSLEAY_VERSION'

"(%lx: %s).", SSLeay(), SSLeay_version(SSLEAY_VERSION));

./src/common/../common/torlog.h:190:50: note: expanded from macro 'log_info'

log_fn_(LOG_INFO, domain, PRETTY_FUNCTION, args)

src/common/crypto.c:299:55: error: use of undeclared identifier

'OPENSSL_VERSION_TEXT'

(unsigned long)OPENSSL_VERSION_NUMBER, OPENSSL_VERSION_TEXT,

./src/common/../common/torlog.h:194:50: note: expanded from macro 'log_warn'

log_fn_(LOG_WARN, domain, PRETTY_FUNCTION, args)

src/common/crypto.c:300:41: error: use of undeclared identifier 'SSLEAY_VERSION'

SSLeay(), SSLeay_version(SSLEAY_VERSION));

./src/common/../common/torlog.h:194:50: note: expanded from macro 'log_warn'

log_fn_(LOG_WARN, domain, PRETTY_FUNCTION, args)

src/common/crypto.c:339:7: warning: implicit declaration of function

'ENGINE_load_builtin_engines' is invalid in C99
[-Wimplicit-function-declaration]
ENGINE_load_builtin_engines();

src/common/crypto.c:340:7: warning: implicit declaration of function

'ENGINE_register_all_complete' is invalid in C99
[-Wimplicit-function-declaration]
ENGINE_register_all_complete();

src/common/crypto.c:350:13: warning: incompatible integer to pointer conversion

assigning to 'ENGINE *' (aka 'struct engine_st *') from 'int'
[-Wint-conversion]

e = ENGINE_by_id(accelName);

~

src/common/crypto.c:363:9: warning: implicit declaration of function

'ENGINE_set_default' is invalid in C99 [-Wimplicit-function-declaration]

ENGINE_set_default(e, ENGINE_METHOD_ALL);

src/common/crypto.c:363:31: error: use of undeclared identifier

'ENGINE_METHOD_ALL'

ENGINE_set_default(e, ENGINE_METHOD_ALL);

src/common/crypto.c:367:25: warning: implicit declaration of function

'ENGINE_get_default_RSA' is invalid in C99
[-Wimplicit-function-declaration]
log_engine("RSA", ENGINE_get_default_RSA());

src/common/crypto.c:368:24: warning: implicit declaration of function

'ENGINE_get_default_DH' is invalid in C99
[-Wimplicit-function-declaration]
log_engine("DH", ENGINE_get_default_DH());

src/common/crypto.c:369:26: warning: implicit declaration of function

'ENGINE_get_default_ECDH' is invalid in C99
[-Wimplicit-function-declaration]
log_engine("ECDH", ENGINE_get_default_ECDH());

src/common/crypto.c:370:27: warning: implicit declaration of function

'ENGINE_get_default_ECDSA' is invalid in C99
[-Wimplicit-function-declaration]
log_engine("ECDSA", ENGINE_get_default_ECDSA());

src/common/crypto.c:371:26: warning: implicit declaration of function

'ENGINE_get_default_RAND' is invalid in C99
[-Wimplicit-function-declaration]
log_engine("RAND", ENGINE_get_default_RAND());

src/common/crypto.c:373:26: warning: implicit declaration of function

'ENGINE_get_digest_engine' is invalid in C99
[-Wimplicit-function-declaration]
log_engine("SHA1", ENGINE_get_digest_engine(NID_sha1));

src/common/crypto.c:374:30: warning: implicit declaration of function

'ENGINE_get_cipher_engine' is invalid in C99
[-Wimplicit-function-declaration]
log_engine("3DES-CBC", ENGINE_get_cipher_engine(NID_des_ede3_cbc));

src/common/crypto.c:408:3: warning: implicit declaration of function

'ERR_remove_state' is invalid in C99 [-Wimplicit-function-declaration]

ERR_remove_state(0);

src/common/crypto.c:691:25: error: incomplete definition of type

'struct buf_mem_st'

*dest = tor_malloc(buf->length+1);

~

./src/common/util.h:116:44: note: expanded from macro 'tor_malloc'
#define tor_malloc(size) tor_malloc_(size DMALLOC_ARGS)

/test/tor/boringssl-install/include/openssl/base.h:170:16: note: forward

declaration of 'struct buf_mem_st'

typedef struct buf_mem_st BUF_MEM;

src/common/crypto.c:692:20: error: incomplete definition of type

'struct buf_mem_st'

memcpy(*dest, buf->data, buf->length);

~

/usr/include/secure/_string.h:65:33: note: expanded from macro 'memcpy'

builtin_memcpy_chk (dest, src, len, darwin_obsz0 (dest))

/test/tor/boringssl-install/include/openssl/base.h:170:16: note: forward

declaration of 'struct buf_mem_st'

typedef struct buf_mem_st BUF_MEM;

src/common/crypto.c:692:31: error: incomplete definition of type

'struct buf_mem_st'

memcpy(*dest, buf->data, buf->length);

~

/usr/include/secure/_string.h:65:38: note: expanded from macro 'memcpy'

builtin_memcpy_chk (dest, src, len, darwin_obsz0 (dest))

/test/tor/boringssl-install/include/openssl/base.h:170:16: note: forward

declaration of 'struct buf_mem_st'

typedef struct buf_mem_st BUF_MEM;

src/common/crypto.c:693:14: error: incomplete definition of type

'struct buf_mem_st'

(*dest)[buf->length] = 0; /* nul terminate it */

~

/test/tor/boringssl-install/include/openssl/base.h:170:16: note: forward

declaration of 'struct buf_mem_st'

typedef struct buf_mem_st BUF_MEM;

src/common/crypto.c:694:13: error: incomplete definition of type

'struct buf_mem_st'

*len = buf->length;

~

/test/tor/boringssl-install/include/openssl/base.h:170:16: note: forward

declaration of 'struct buf_mem_st'

typedef struct buf_mem_st BUF_MEM;

src/common/crypto.c:695:3: warning: implicit declaration of function

'BUF_MEM_free' is invalid in C99 [-Wimplicit-function-declaration]

BUF_MEM_free(buf);

src/common/crypto.c:1783:19: warning: implicit declaration of function

'DH_generate_parameters' is invalid in C99
[-Wimplicit-function-declaration]

dh_parameters = DH_generate_parameters(DH_BYTES*8, DH_GENERATOR, NULL, NULL);

src/common/crypto.c:2118:12: error: no member named 'length' in 'struct dh_st'

res->dh->length = DH_PRIVATE_KEY_BITS;
~

src/common/crypto.c:3046:2: error: OpenSSL has been built without thread

support. Tor requires an OpenSSL library with thread support enabled.

#error OpenSSL has been built without thread support. Tor requires an \

src/common/crypto.c:3149:3: warning: implicit declaration of function

'ENGINE_cleanup' is invalid in C99 [-Wimplicit-function-declaration]

ENGINE_cleanup();

src/common/crypto.c:3152:3: warning: implicit declaration of function

'CONF_modules_unload' is invalid in C99 [-Wimplicit-function-declaration]

CONF_modules_unload(1);

Child Tickets

Change History (15)

comment:1 Changed 4 years ago by nickm

Milestone: Tor: unspecifiedTor: 0.2.7.x-final

Worth looking at during 0.2.7 triage IMO

comment:2 Changed 4 years ago by nickm

Status: newassigned

comment:3 Changed 4 years ago by nickm

Milestone: Tor: 0.2.7.x-finalTor: 0.2.8.x-final

comment:4 Changed 3 years ago by teor

Parent ID: #13415

Split this off by itself - LibreSSL now works and I want to close all those tickets.

comment:5 Changed 3 years ago by nickm

Partial success: I have crypto.c compiling! It's in a branch called "boringssl". Probably some cleanup is in order. I have no idea if it works.

Only partial success however; tor-gencert.c and tortls.c are still broken.

comment:6 Changed 3 years ago by nickm

Now tor-gencert.c works; only tortls.c is failing to compile.

comment:7 Changed 3 years ago by nickm

tortls.c is much closer to compiling in my boringssl branch, but it has a few warnings left:

src/common/tortls.c:1576:22: error: no member named 'ciphers' in
      'struct ssl_session_st'; did you mean 'cipher'?
  ciphers = session->ciphers;
                     ^~~~~~~
                     cipher
/Users/nickm/boringssl//include/openssl/ssl.h:1070:21: note: 'cipher' declared
      here
  const SSL_CIPHER *cipher;
                    ^
src/common/tortls.c:1576:11: error: incompatible pointer types assigning to
      'struct stack_st_SSL_CIPHER *' from 'const SSL_CIPHER *' (aka 'const
      struct ssl_cipher_st *') [-Werror,-Wincompatible-pointer-types]
  ciphers = session->ciphers;
          ^ ~~~~~~~~~~~~~~~~

I don't see a easy way around this.

comment:8 Changed 3 years ago by nickm

Note:

In an message (relayed here with permission), David Benjamin (BoringSSL hacker) informed me of a few things we should keep in mind if we tread this route:

  • BoringSSL isn't meant to maintain openssl compatibility, and might not be the library for us.
  • Neither SSL_renegotiate nor SSL_set_session_secret_cb will actually work here.
  • In place of the get_cipher_by_char craziness we could instead do SSL_get_cipher_by_value.
  • This seriously might not be the right library for the degree of shenanigans that Tor has tended to pull with the guts of the TLS implementation.

Together this would imply that BoringSSL compatibility simply can't happen until we drop the v2 version of our TLS handshake. And that we should probably consider the stuff we want to use BoringSSL for "supported by accident, at best."

comment:9 Changed 3 years ago by nickm

Milestone: Tor: 0.2.8.x-finalTor: 0.2.???

Move a few tickets out of 0.2.8. I would take a good patch for most of these if somebody writes one. (If you do, please make the ticket needs_review and move it back into maint-0.2.8 milestone. :) )

comment:10 Changed 2 years ago by teor

Milestone: Tor: 0.2.???Tor: 0.3.???

Milestone renamed

comment:11 Changed 2 years ago by nickm

Keywords: tor-03-unspecified-201612 added
Milestone: Tor: 0.3.???Tor: unspecified

Finally admitting that 0.3.??? was a euphemism for Tor: unspecified all along.

comment:12 Changed 20 months ago by nickm

Keywords: tor-03-unspecified-201612 removed

Remove an old triaging keyword.

comment:13 Changed 20 months ago by nickm

Status: assignednew

Change the status of all assigned/accepted Tor tickets with owner="" to "new".

comment:14 Changed 20 months ago by nickm

Keywords: openssl boringssl post-deprecation added; lorax removed
Severity: Normal

comment:15 Changed 7 months ago by teor

Parent ID: #26639
Note: See TracTickets for help on using tickets.