Opened 4 years ago

Closed 3 years ago

#15760 closed defect (fixed)

tortls.c fails to compile with OpenSSL 1.1.0-dev

Reported by: yancm Owned by:
Priority: Medium Milestone: Tor: 0.2.7.x-final
Component: Core Tor/Tor Version: Tor: 0.2.7
Severity: Keywords: openssl, tor-tls 026-backport
Cc: Actual Points:
Parent ID: #9476 Points:
Reviewer: Sponsor:

Description (last modified by nickm)

On NetBSD 6.1_Stable/i386 trying to compile15760
Tor v0.2.7.0-alpha-dev (git-06939551f4c081c4)
with
Libevent 2.1.5-beta, Zlib 1.2.3
AND
OpenSSL 1.1.0-dev

Since I was already compiling a development version of tor, thought I would compile against a dev version of openssl...

I used autogen and configure thusly:
./autogen.sh --with-libevent-dir=/usr/local --enable-static-openssl=1 --with-openssl-dir=/usr/local/ssl
./configure --with-libevent-dir=/usr/local --enable-static-openssl=1 --with-openssl-dir=/usr/local/ssl

The build started ok for the first 30-40 files then I received:

CC src/common/tortls.o

src/common/tortls.c: In function 'tor_tls_context_new':
src/common/tortls.c:1375:18: error: dereferencing pointer to incomplete type
src/common/tortls.c:1376:16: error: dereferencing pointer to incomplete type
src/common/tortls.c: In function 'find_cipher_by_id':
src/common/tortls.c:1529:13: error: dereferencing pointer to incomplete type
src/common/tortls.c:1535:10: error: dereferencing pointer to incomplete type
src/common/tortls.c:1537:7: error: dereferencing pointer to incomplete type
src/common/tortls.c:1541:13: error: dereferencing pointer to incomplete type
src/common/tortls.c:1541:30: error: dereferencing pointer to incomplete type
src/common/tortls.c:1547:22: error: dereferencing pointer to incomplete type
src/common/tortls.c:1548:12: error: dereferencing pointer to incomplete type
src/common/tortls.c:1549:18: error: dereferencing pointer to incomplete type
src/common/tortls.c: In function 'tor_tls_classify_client_ciphers':
src/common/tortls.c:1626:27: error: dereferencing pointer to incomplete type
src/common/tortls.c: In function 'tor_tls_client_is_using_v2_ciphers':
src/common/tortls.c:1676:54: error: dereferencing pointer to incomplete type
src/common/tortls.c: In function 'tor_tls_server_info_callback':
src/common/tortls.c:1695:11: error: dereferencing pointer to incomplete type
src/common/tortls.c:1696:11: error: dereferencing pointer to incomplete type
src/common/tortls.c: In function 'rectify_client_ciphers':
src/common/tortls.c:1826:7: error: invalid application of 'sizeof' to incomplete type 'SSL_CIPHER'
src/common/tortls.c:1828:7: error: invalid use of undefined type 'struct ssl_cipher_st'
src/common/tortls.c:1828:28: error: dereferencing pointer to incomplete type
src/common/tortls.c:1831:7: error: invalid use of undefined type 'struct ssl_cipher_st'
src/common/tortls.c:1831:28: error: dereferencing pointer to incomplete type
src/common/tortls.c:1832:7: error: invalid use of undefined type 'struct ssl_cipher_st'
src/common/tortls.c:1832:28: error: dereferencing pointer to incomplete type
src/common/tortls.c:1841:7: error: dereferencing pointer to incomplete type
src/common/tortls.c:1841:7: error: dereferencing pointer to incomplete type
src/common/tortls.c:1854:29: error: dereferencing pointer to incomplete type
src/common/tortls.c:1857:9: error: dereferencing pointer to incomplete type
src/common/tortls.c:1857:9: error: dereferencing pointer to incomplete type
src/common/tortls.c:1862:25: error: dereferencing pointer to incomplete type
src/common/tortls.c:1864:9: error: dereferencing pointer to incomplete type
src/common/tortls.c:1868:7: error: invalid use of undefined type 'struct ssl_cipher_st'
src/common/tortls.c:1868:47: error: dereferencing pointer to incomplete type
src/common/tortls.c:1872:9: error: invalid use of undefined type 'struct ssl_cipher_st'
src/common/tortls.c:1872:9: error: dereferencing pointer to incomplete type
src/common/tortls.c:1873:1: error: invalid use of undefined type 'struct ssl_cipher_st'
src/common/tortls.c:1873:9: error: dereferencing pointer to incomplete type
src/common/tortls.c: In function 'tor_tls_new':
src/common/tortls.c:1939:40: error: dereferencing pointer to incomplete type
src/common/tortls.c: In function 'tor_tls_unblock_renegotiation':
src/common/tortls.c:2033:13: error: dereferencing pointer to incomplete type
src/common/tortls.c: In function 'tor_tls_block_renegotiation':
src/common/tortls.c:2048:11: error: dereferencing pointer to incomplete type
src/common/tortls.c: In function 'tor_tls_assert_renegotiation_unblocked':
src/common/tortls.c:2056:5: error: dereferencing pointer to incomplete type
src/common/tortls.c: In function 'tor_tls_handshake':
src/common/tortls.c:2193:22: error: dereferencing pointer to incomplete type
src/common/tortls.c:2203:27: error: dereferencing pointer to incomplete type
src/common/tortls.c: In function 'tor_tls_finish_handshake':
src/common/tortls.c:2239:13: error: dereferencing pointer to incomplete type
src/common/tortls.c: In function 'tor_tls_get_tlssecrets':
src/common/tortls.c:2842:3: error: dereferencing pointer to incomplete type
src/common/tortls.c:2843:3: error: dereferencing pointer to incomplete type
src/common/tortls.c:2848:3: error: dereferencing pointer to incomplete type
src/common/tortls.c:2848:3: error: dereferencing pointer to incomplete type
src/common/tortls.c:2849:3: error: dereferencing pointer to incomplete type
src/common/tortls.c:2849:3: error: dereferencing pointer to incomplete type
src/common/tortls.c:2853:37: error: dereferencing pointer to incomplete type
src/common/tortls.c:2854:30: error: dereferencing pointer to incomplete type
src/common/tortls.c: In function 'tor_tls_get_buffer_sizes':
src/common/tortls.c:2870:15: error: dereferencing pointer to incomplete type
src/common/tortls.c:2871:30: error: dereferencing pointer to incomplete type
src/common/tortls.c:2874:15: error: dereferencing pointer to incomplete type
src/common/tortls.c:2875:30: error: dereferencing pointer to incomplete type
src/common/tortls.c:2878:25: error: dereferencing pointer to incomplete type
src/common/tortls.c:2879:25: error: dereferencing pointer to incomplete type
src/common/tortls.c: In function 'tor_tls_client_is_using_v2_ciphers':
src/common/tortls.c:1677:1: warning: control reaches end of non-void function
src/common/tortls.c: In function 'find_cipher_by_id':
src/common/tortls.c:1557:1: warning: control reaches end of non-void function
* Error code 1

Stop.
make: stopped in /usr/local/src/tor
* Error code 1

Fixable? Unsupported?

thanks,
gene

Child Tickets

Change History (26)

comment:1 in reply to:  description Changed 4 years ago by yawning

Keywords: tor-tls added

Replying to yancm:

Fixable? Unsupported?

Probably fixable, but unsupported for now since it appears that the API has changed.

IIRC, there hasn't been an official 1.1.x release made by the OpenSSL people ever, so changing the code now might end up as chasing a moving target (though I would still be in favor of supporting this to the best of our ability sooner rather than later).

comment:2 Changed 4 years ago by yawning

Priority: normalminor

Setting it to minor since snapshots are snapshots. One of us (maybe me) will poke on this as time allows.

comment:3 Changed 4 years ago by yancm

This is the top of the change log for openssl:

OpenSSL CHANGES
_

Changes between 1.0.2 and 1.1.0 [xx XXX xxxx]

*) RAND_pseudo_bytes has been deprecated. Users should use RAND bytes instead.

*) Added support for TLS extended master secret from

draft-ietf-tls-session-hash-03.txt. Thanks for Alfredo Pironti for an
initial patch which was a great help during development.
[Steve Henson]

*) All libssl internal structures have been removed from the public header

files, and the OPENSSL_NO_SSL_INTERN option has been removed (since it is
now redundant). Users should not attempt to access internal structures
directly. Instead they should use the provided API functions.
[Matt Caswell]

*) config has been changed so that by default OPENSSL_NO_DEPRECATED is used.

Access to deprecated functions can be re-enabled by running config with
"enable-deprecated". In addition applications wishing to use deprecated
functions must define OPENSSL_USE_DEPRECATED. Note that this new behaviour
will, by default, disable some transitive includes that previously existed
in the header files (e.g. ec.h will no longer, by default, include bn.h)
[Matt Caswell]

*
Do you think that enabling deprecated in openssl might be enough? I can try that when I get some spare time...

comment:4 Changed 4 years ago by yawning

I'd be willing to bet that the internal structure related removal requires code changes on our part, since there's a few places in the code that do lots of nastiness. Personally I'd be more in favor of fixing it correctly as well rather than continue to depend on deprecated routines if at all possible.

comment:5 Changed 4 years ago by yawning

It occurs to me that this is dup-ish of #14188, except the reason for the breakage is that a whole bunch of stuff has been made totally opaque to application code. I'm inclined to keep discussion here since it's a different issue.

Ok, so I spent the time and looked into this a bit. First the good news:

Apr 23 12:16:59.017 [notice] Tor v0.2.7.0-alpha-dev (git-ad9a71ed70d0f439) running on Linux with Libevent 2.0.22-stable, OpenSSL 1.1.0-dev and Zlib 1.2.8.
Apr 23 12:16:59.017 [notice] Tor can't help you if you use it wrong! Learn how to be safe at https://www.torproject.org/download/download#warning
Apr 23 12:16:59.017 [notice] This version is not a stable Tor release. Expect more bugs than usual.

...

Apr 23 12:17:10.000 [notice] Bootstrapped 100%: Done

Now the bad news:

  • V2 link handshake support is totally broken and is not compiled at all (but, we're planning on killing it eventually anyway, so this may be ok one day, see #9476). With enough persistence it may be possible to fix this, but I don't have more time to burn on this.
  • Reporting per-connection SSL buffer usage statistics is broken since the data structure is opaque (Oh well, not great but we can live with this).
  • AUTHENTICATE cell processing with the RSA-SHA256-TLSSecret AuthType is broken since it's not possible (at least easily) to get the TLS pre-master secret. While this isn't used often in practice, this needs to be fixed somehow. (All v3 uses this per nickm).

Before I link my branch:

  • DO NOT USE MY BRANCH (Except as a starting point for fixing the stuff that I broke)
  • DO NOT EXPECT THIS TO BE CORRECT, OR SAFE
  • If people ignore the warnings, and it breaks in any way, shape, or form, resulting the men in suits coming for a visit and dragging them off in the van, I will point and laugh.

My branch: https://github.com/Yawning/tor/compare/bug15760

Last edited 4 years ago by yawning (previous) (diff)

comment:6 Changed 4 years ago by yawning

Priority: minornormal

Because getting this done correctly is non-trivial now that we know more, bumping the priority back up. At least we know what we're up against.

comment:7 Changed 4 years ago by yawning

Parent ID: #9476

comment:8 Changed 4 years ago by nickm

I wonder if we can enumerate what we need to the openssl people so they don't go and break tor.

comment:9 in reply to:  8 Changed 4 years ago by yawning

Replying to nickm:

I wonder if we can enumerate what we need to the openssl people so they don't go and break tor.

We should spend time looking into seeing if we can do the V2 link handshake stuff with 1.1 before poking them about it. I sidestepped the issue when initially looking into things under the assumption that the old link protocol versions no one should use will die soon-ish, but that seems up in the air at this point.

The AUTHENTICATE stuff would definitely require changes on their end, since I don't think we can come up with a better alternative faster and get it deployed wide enough.

comment:10 Changed 4 years ago by nickm

RFC 5705 (implemented as SSL_export_keying_material) seems like it would be a good thing to use in future AUTHENTICATE cell types. But for now, we need to keep implementing the old kind. We should be adding a new ticket to do a design there.

comment:11 Changed 4 years ago by nickm

I've hacked this some more in bug15760_easy_026 -- it's not done , since we still need a way to get the master secret and stuff, but it's progress.

I'm a tiny bit nervous here; this needs testing with legacy Tor versions IMO maybe.

comment:12 Changed 4 years ago by nickm

(this takes lots of ideas and stuff from yawning's code)

comment:13 Changed 4 years ago by nickm

Description: modified (diff)
Status: newneeds_review

The patch series mentioned above isn't complete, but I believe that merging it is better than not.

comment:14 Changed 4 years ago by yawning

The branch looks ok, with the fixup from the #16034 applied, though you know more about all the various inter-version quirks than I do, and I'm not sure how much my review is worth since I wrote a chunk of this.

We're not going to backport immediately right? It's probably ok to merge.

comment:15 Changed 4 years ago by nickm

Milestone: Tor: unspecifiedTor: 0.2.6.x-final

bug16034_no_more_openssl_098_squashed is what I merged. Some of it might be mergeable to 0.2.6

comment:16 Changed 4 years ago by nickm

Keywords: 026-backport added

I have the remaining fixes in "bug15760_hard_026" . It requires a couple of openssl patches currently in review. Those are in https://github.com/nmathewson/openssl.git , in the branches:

secret_accessors
ssl_session_get_ciphers

comment:17 Changed 4 years ago by nickm

They merged my patches but changed the API a little. Waiting to hear more before I update the Tor branch. Still, probably reviewable/testable.

comment:18 Changed 4 years ago by nickm

And they've fixed the bugs in their API changes. bug15760_hard_026 should now IMO be considered for merge.

comment:19 Changed 4 years ago by yawning

NACK 1651dd57154a4375e7338a5022ee46b93611bf62

+ const size_t server_random_len = SSL_get_server_random(ssl, NULL, -1);
+ const size_t client_random_len = SSL_get_client_random(ssl, NULL, -1);
+ const size_t master_key_len = SSL_SESSION_get_master_key(session, NULL, -1);

The len parameter to each of those is a size_t now, all of those should pass in 0 as the 3rd argument, rest looks ok upon casual inspection.

comment:20 Changed 4 years ago by yawning

Ok, lgtm now after the fixup.

comment:21 Changed 4 years ago by nickm

Resquashed to bug15760_hard_026_v2. Merged to master; backport candidate to 0.2.6

comment:22 Changed 4 years ago by yawning

https://github.com/Yawning/tor/compare/bug15760_redux

This should be the last of the fixes (as in, it compiles on my box, though linking fails due to disagreement about -fPIC that's orthogonal to this and entirely to do with how I built OpenSSL/tor, the changes should be trivially correct anyway).

comment:23 Changed 4 years ago by nickm

Thanks for the patch! I believe I merged it to 0.2.7 and things are well now. Leaving in 0.2.6 for possible backport.

comment:24 Changed 4 years ago by yancm

yawning and nickm - Really cool! I've been building successfully against OpenSSL 1.1.0-dev for a couple of days now. Thank you for your efforts!

comment:25 Changed 4 years ago by nickm

Cool; please let us know if it breaks again -- the OpenSSL team may find something else to change between now and when they release. :)

comment:26 Changed 3 years ago by nickm

Milestone: Tor: 0.2.6.x-finalTor: 0.2.7.x-final
Resolution: fixed
Status: needs_reviewclosed

This is fixed in 0.2.7, and 0.2.7 will be stable before Openssl 1.1 is released. Marking it closed there.

Note: See TracTickets for help on using tickets.