Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Caddy-tris: ERR_SSL_VERSION_INTERFERENCE #126

Closed
high3eam opened this issue Aug 18, 2018 · 23 comments
Closed

Caddy-tris: ERR_SSL_VERSION_INTERFERENCE #126

high3eam opened this issue Aug 18, 2018 · 23 comments
Assignees
Labels

Comments

@high3eam
Copy link

I've built caddy with the tris crypto library of go and thus tried out to use tls1.3 with caddy.

I added/modified three lines to the caddytls/config.go: "TLS1-3": TLS_CHACHA20_POLY1305_SHA256 as an additional cipher and to only use this one for TLS1.3 negotiation and "tls1.3": tls.VersionTLS13 for the available TLS versions.

Additionally I stated in the same file, that the max version is tls1.3:

if config.ProtocolMaxVersion == 0 {
                config.ProtocolMaxVersion = tls.VersionTLS13
        }

But still dev.ssllabs.com/ssltest says, that my server negotiates TLS1.3 with a blacklisted HTTP/2 ciphersuite, which is why probably chrome throws the error mentioned in the title.

Any ideas?

@kriskwiatkowski kriskwiatkowski self-assigned this Aug 18, 2018
@kriskwiatkowski
Copy link
Contributor

It seems to me like TLS draft version mismatch. On current master branch we support draft 28 and draft 23, which is not a final version of the TLS 1.3. I've no idea which version of the draft chrome supports. In order to debug this issue, I would start with wireshark and check if client hello contains 0x7f1c (draft 28) or 0x7f17 (draft 23) in "supported_versions" extensions.
Please notice that there is a branch 'tls13', which is ongoing effort to implement final version of TLS 1.3. It may be worth trying to run caddy with this version, but it's still early beta

@high3eam
Copy link
Author

Attached are screenshots from the clientHello of chrome and the alert of my webserver:

chrome

And here the alert message.
changecipherspec

@kriskwiatkowski
Copy link
Contributor

kriskwiatkowski commented Aug 19, 2018

I don't think error comes from tls-tris. Tris never sends "Unsupported extension". In fact we don't even define alert 110 (see alerts.go):

@high3eam
Copy link
Author

high3eam commented Aug 19, 2018

You could be right, because, as seen here, Chrome and Firefox both connect to my server using TLS1.3, but HTTP/2 throws the error: Blacklisted cipher suite.

Maybe HTTP/2 implementation in go needs to be updated?

EDIT: Alert 110 is not shown in my screenshots, it is alert 21. Unsupported extensions has number 110, which is not an alert. alert.go says: errdecryptionfailure

@kriskwiatkowski
Copy link
Contributor

Hmm... I'm not sure. I've just noticed that it's a browser that sends "unsupported extension" not a server.
I'm running tls-tris server on gotls13-t2.amongbytes.com - it is TLS 1.3 Draft 28 only. The results from ssllabs looks like that:
https://www.ssllabs.com/ssltest/analyze.html?d=gotls13-t2.amongbytes.com

@high3eam
Copy link
Author

high3eam commented Aug 20, 2018

Tested your domain also on dev.ssllabs.com, but still: Assessment failed: Unexpected failure. SSLLabs states, they support full Draft 28 support, which seems to work properly...
Maybe they stop continuing testing and throw an error, because handshake simulation requires TLS1.2 with at least one enabled AEAD cipher ?

@kriskwiatkowski
Copy link
Contributor

kriskwiatkowski commented Aug 20, 2018

I think server had some problem over night.
Here you go new results: https://dev.ssllabs.com/ssltest/analyze.html?d=gotls13-t2.amongbytes.com&hideResults=off

@high3eam
Copy link
Author

Okay, I've now built the exact same configuration as you did, but with caddy. You can see here: https://dev.ssllabs.com/ssltest/analyze.html?d=h-neef.de&s=185.101.92.192, I support the same ciphers, and get the same message as you: Blacklisted HTTP/2 cipher suite.

@high3eam
Copy link
Author

Okay, so far, so good, that means, that ssllabs need to fox that, but still the problem persists, that connection via chrome/firefox is not possible to your/my server. When I connect to your server via https in chrome (https://gotls13-t2.amongbytes.com/), it says: ERR_SSL_VERSION_OR_CIPHER_MISMATCH.

Any idea?

@kriskwiatkowski
Copy link
Contributor

kriskwiatkowski commented Aug 20, 2018

Ok, so I've changed my configuration to use Poly1305-Chacha20. SSL labs still says that this cipher suite is "blacklisted", but I don't think this is correct. Cipher suite is neither blacklisted by TLS 1.2 (see apendx A in RFC 5246) nor by TLS 1.3 (quite opposite - it's recommended as per RFC 8446, 9.1)

I think we are good here. I'm not sure why ssllabs displays "blacklisted cipher". I think they shouldn't.

TIP: report says "ECDH x25519", but in fact it negotiates ECDHE (TLS 1.3 uses ephemeral ECDH only). This may be a root of problem

@kriskwiatkowski
Copy link
Contributor

kriskwiatkowski commented Aug 20, 2018

yeah, that's something I need to look into. We do tests against boringssl, nss and some others - all are OK. Hard for me to explain this problem at the moment. But I agree - it's an issue. Debugging ongoing... (but may take some time)

@high3eam
Copy link
Author

Okay, thank you for the clarification.

Yes, I agree, there might be a problem with ssllabs displaying the correct key exchange.

Looking forward to your investigations! 👍

@kriskwiatkowski
Copy link
Contributor

I've been advised that ALPN in SH must be part of EE.

@high3eam
Copy link
Author

Okay, so that means both serverhello and Encrypted extensions must contain alpn versions? Or just one of them? And does it mean tls-tris must implement ALPN into EE?

@kriskwiatkowski
Copy link
Contributor

kriskwiatkowski commented Aug 20, 2018

the latter, ALPN needs to go to EE. server hello must contain ALPN in EE (and must contain not-encrypted).

@high3eam
Copy link
Author

Alright, seems to be pretty clear now.

Are you able to tell me how long the fixing process is going to be? Just a few lines to add or does it take longer time? I'm asking because I am relatively new to the topic but want to participate in any way possible.

@kriskwiatkowski
Copy link
Contributor

kriskwiatkowski commented Aug 20, 2018

So, first of all - if you want to propose a fix - just go ahead.
I think the problem is a bit broader than just ALPN. It seems to me that tris doesn't handle EE correctly. There is a bit digging needed.

The task would be to go over https://tools.ietf.org/html/rfc8446#page-37 and check (test) which extensions should be encrypted and then check if it is really done (for TLS 1.3 only).

I would start with looking at SH marshalling, it should be done somewhere here:
https://github.com/cloudflare/tls-tris/blob/master/handshake_server.go#L472

My wild guess is that ALPN is actually marshaled to EE, but it is also marshalled to SH directly. My guess comes from the fact this thing exists:
https://github.com/cloudflare/tls-tris/blob/master/handshake_messages.go#L1221

For TLS 1.2 code must be unchanged. Obviously it needs to be tested properly. Let me know if you want to take it over. Otherwise I'll try to start it somewhere this week, as I would like to have it fixed rather quickly

Personally I think adding tests in a first place would be great help. We have many tests and non of them has caught it, which really sucks.

@high3eam
Copy link
Author

high3eam commented Aug 20, 2018

Okay, thank you very much for providing some information to begin with.

I must admit though, that is too much for me right now, and I guess I will concentrate more on first learning a bit more about go, then to seriously dig in this topic here, when you can probably fix it in like 100x times the speed of me.

SO: I am going to let you go for gold, and I watch and learn from it. In the end, I'm sitting on the user side, and will be more than happy to test the fixed version! :-)

Still thanks for transparently communicating!

@kriskwiatkowski
Copy link
Contributor

@henrocker Can you do a review for #136 and check if it fixes the problem on your side ?

@high3eam
Copy link
Author

high3eam commented Sep 16, 2018

Hi,

just tested to compile it, but I get the following error, while tris is building boringssl:

[319/411] Linking CXX executable crypto/crypto_test
[320/411] Building CXX object ssl/CMakeFiles/ssl.dir/bio_ssl.cc.o
[321/411] Building CXX object ssl/CMakeFiles/ssl.dir/d1_both.cc.o
[322/411] Building CXX object ssl/CMakeFiles/ssl_test.dir/ssl_test.cc.o
FAILED: ssl/CMakeFiles/ssl_test.dir/ssl_test.cc.o
/usr/bin/c++  -DBORINGSSL_IMPLEMENTATION -I../third_party/googletest/include -I.                                                                        ./ssl/../include -Werror -Wformat=2 -Wsign-compare -Wmissing-field-initializers                                                                         -Wwrite-strings -Wall -ggdb -fvisibility=hidden -fno-common -Wno-free-nonheap-ob                                                                        ject -Wmissing-declarations -std=c++11 -fno-exceptions -fno-rtti -Wshadow -MD -M                                                                        T ssl/CMakeFiles/ssl_test.dir/ssl_test.cc.o -MF ssl/CMakeFiles/ssl_test.dir/ssl_                                                                        test.cc.o.d -o ssl/CMakeFiles/ssl_test.dir/ssl_test.cc.o -c ../ssl/ssl_test.cc
c++: internal compiler error: Killed (program cc1plus)
Please submit a full bug report,
with preprocessed source if appropriate.
See <http://gcc.gnu.org/bugs.html> for instructions.
[323/411] Building CXX object ssl/CMakeFiles/ssl.dir/d1_lib.cc.o
ninja: build stopped: subcommand failed.
The command '/bin/sh -c cd boringssl && ninja -C build' returned a non-zero code                                                                        : 1
_dev/Makefile:59: recipe for target 'build-test-boring' failed
make: *** [build-test-boring] Error 1
root@h-neef:~/tls-tris#

@kriskwiatkowski
Copy link
Contributor

hmm... interesting. travis did this step and it looks OK. Late here, I'll check it out tomorrow. Thanks !!

@high3eam
Copy link
Author

Hi,

tested this again with GO version 1.10.4, and it compiled successfully!

TLS1.3 is working on caddy now! Thank you so much, great work!

After tris has finished compiling and running interop tests, this was the command to build caddy with tls1.3: GOROOT="/root/tls-tris/tls-tris/_dev/GOROOT/linux_amd64" go run build.go.

I have changed the caddy/caddytls/config.go according to the caddy.patch under _dev, with additionally, adding one TLS1.3 cipher as the supported ones in the config.go.

Works like a charm: https://henrock.net (Disabling QUIC in Chrome reveals negotiation with TLS1.3)

@kriskwiatkowski
Copy link
Contributor

Great!! Thanks a lot for your help !!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants