mirleft / ocaml-tls

TLS in pure OCaml
BSD 2-Clause "Simplified" License
304 stars 68 forks source link

wget on nqsb.io fails on Debian (GnuTLS) #293

Closed edwintorok closed 9 years ago

edwintorok commented 9 years ago

Not sure where to report this, curl (OpenSSL) works fine but wget (GnuTLS) gives an error:

$ wget https://nqsb.io/nqsbtls-usenix-security15.pdf
--2015-07-08 00:09:28--  https://nqsb.io/nqsbtls-usenix-security15.pdf
Resolving nqsb.io (nqsb.io)... 198.167.222.201
Connecting to nqsb.io (nqsb.io)|198.167.222.201|:443... connected.
GnuTLS: A TLS fatal alert has been received.
GnuTLS: received alert [47]: Illegal parameter
Unable to establish SSL connection.
$ wget --version
GNU Wget 1.16 built on linux-gnu.

+digest +https +ipv6 +iri +large-file +nls +ntlm +opie +psl +ssl/gnutls
[...]

Tried some wget flags but no good:

wget --secure-protocol=TLSv1_2 https://nqsb.io/nqsbtls-usenix-security15.pdf
--2015-07-08 00:12:21--  https://nqsb.io/nqsbtls-usenix-security15.pdf
Resolving nqsb.io (nqsb.io)... 198.167.222.201
Connecting to nqsb.io (nqsb.io)|198.167.222.201|:443... connected.
Aborted (core dumped)

Update: this works:

wget --secure-protocol=TLSv1 https://nqsb.io/nqsbtls-usenix-security15.pdf
amirmc commented 9 years ago

Just for info, I tried on a Mac (10.10.3) and it worked first time.

$ wget https://nqsb.io/nqsbtls-usenix-security15.pdf
--2015-07-07 22:26:24--  https://nqsb.io/nqsbtls-usenix-security15.pdf
Resolving nqsb.io... 198.167.222.201
Connecting to nqsb.io|198.167.222.201|:443... connected.
HTTP request sent, awaiting response... 200 OK
Length: unspecified [application/pdf]
Saving to: 'nqsbtls-usenix-security15.pdf'

nqsbtls-usenix-security15.pdf                            [   <=>                                                                                                                   ] 248.26K   486KB/s   in 0.5s

2015-07-07 22:26:26 (486 KB/s) - 'nqsbtls-usenix-security15.pdf' saved [254218]
$ wget --version
GNU Wget 1.16.3 built on darwin14.1.0.

+digest +https +ipv6 -iri +large-file -nls +ntlm +opie -psl +ssl/openssl

[...]
edwintorok commented 9 years ago

thanks, we can rule out wget+openssl then, only wget+gnutls is a problem. However gnutls-cli on its own works and established a TLSv1.2 connection, here is gnutls-cli-debug nqsb.io if it helps:

Checking for SSL 3.0 support... no
Checking whether %COMPAT is required... no
Checking for TLS 1.0 support... yes
Checking for TLS 1.1 support... yes
Checking fallback from TLS 1.1 to... N/A
Checking for TLS 1.2 support... yes
Checking whether we need to disable TLS 1.2... N/A
Checking whether we need to disable TLS 1.1... N/A
Checking whether we need to disable TLS 1.0... N/A
Checking for Safe renegotiation support... yes
Checking for Safe renegotiation support (SCSV)... no
Checking for HTTPS server name... not checked
Checking for version rollback bug in RSA PMS... dunno
Checking for version rollback bug in Client Hello... no
Checking whether the server ignores the RSA PMS version... yes
Checking whether the server can accept Hello Extensions... yes
Checking whether the server can accept HeartBeat Extension... no
Checking whether the server can accept small records (512 bytes)... yes
Checking whether the server can accept cipher suites not in SSL 3.0 spec... yes
Checking whether the server can accept a bogus TLS record version in the client hello... yes
Checking for certificate information... N/A
Checking for trusted CAs... N/A
Checking whether the server understands TLS closure alerts... yes
Checking whether the server supports session resumption... no
Checking for anonymous authentication support... no
Checking anonymous Diffie-Hellman group info... N/A
Checking for ephemeral Diffie-Hellman support... yes
Checking ephemeral Diffie-Hellman group info... N/A
Checking for ephemeral EC Diffie-Hellman support... no
Checking ephemeral EC Diffie-Hellman group info... N/A
Checking for AES-128-GCM cipher support... no
Checking for AES-128-CBC cipher support... yes
Checking for CAMELLIA-128-GCM cipher support... no
Checking for CAMELLIA-128-CBC cipher support... no
Checking for 3DES-CBC cipher support... yes
Checking for ARCFOUR 128 cipher support... no
Checking for MD5 MAC support... no
Checking for SHA1 MAC support... yes
Checking for SHA256 MAC support... yes
Checking for ZLIB compression support... no
Checking for max record size... no
Checking for OpenPGP authentication support... no
pqwy commented 9 years ago

Thanks for the report!

I compiled a version of wget that uses GnuTLS (3.4.2) on my Arch Linux machine:

GNU Wget 1.16.3 built on linux-gnu.

+digest +https +ipv6 +iri +large-file +nls +ntlm +opie -psl +ssl/gnutls

Using this binary, I cannot replicate the behavior you get on Debian. It always connects successfully.

While I'm digging around for Debian source packages to check if they do something interesting to the default options, could you please run another test:

Check out ocaml-tls, and ./configure --enable-lwt --enable-tests && make. This should produce echo_server.native. Could you please run it, connect to it using wget (like wget https://localhost:4433), and paste the echo_server's output? By default it will dump copious tracing information to the terminal, and this should make it clear what ocaml-tls thinks of the other side.

edwintorok commented 9 years ago
[server] -> start @ 4433
(record-in
 (((content_type HANDSHAKE) (version SSL_3))
   "\001\000\002\002\003\003U\158&7K#\194+^\213\"\1792\149\241\2071\179aS\022\003\190\129h-\021\224\007\166\235\223\000\000\132\192+\192,\192\134\192\135\192\t\192#\192\
  \n\192$\192r\192s\192\b\192\007\192/\1920\192\138\192\139\192\019\192'\192\020\192(\192v\192w\192\018\192\017\000\156\000\157\192z\192{\000/\000<\0005\000=\000A\000\186\000\132\000\192\000\
  \n\000\005\000\004\000\158\000\159\192|\192}\0003\000g\0009\000k\000E\000\190\000\136\000\196\000\022\000\162\000\163\192\128\192\129\0002\000@\0008\000j\000D\000\189\000\135\000\195\000\019\000f\001\000\001U\000\005\000\005\001\000\000\000\000\000\000\000\014\000\012\000\000\tlocalhost\255\001\000\001\000\000#\000\000\000\
  \n\000\012\000\
  \n\000\019\000\021\000\023\000\024\000\025\000\011\000\002\001\000\000\r\000\028\000\026\004\001\004\002\004\003\005\001\005\003\006\001\006\003\003\001\003\002\003\003\002\001\002\002\002\003\000\021\000\247\000\245\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000"))

(fail-alert-out (FATAL ILLEGAL_PARAMETER))

(failure (Fatal (ReaderError (Unknown "bad padding in padding extension"))))

[server] server socket: (Fatal (ReaderError (Unknown "bad padding in padding extension")))
edwintorok commented 9 years ago

Also reproduces with (this is what wget on debian sets):

gnutls-cli --priority='NORMAL:%COMPAT:-VERS-SSL3.0' localhost -p 4433

The problem is when using %COMPAT about which the manual says:

will enable compatibility mode. It might mean that violations of the protocols are allowed as long as maximum compatibility with problematic clients and servers is achieved. More specifically this string would disable TLS record random padding, tolerate packets over the maximum allowed TLS record, and add a padding to TLS Client Hello packet to prevent it being in the 256-512 range which is known to be causing issues with a commonly used firewall.
edwintorok commented 9 years ago

Looking at a wireshark trace GnuTLS %COMPAT sends:

Extension: Padding
 Type: Padding (0x0015)
 Length: 247
 Padding Data: 00f500...
   Padding length: 245
   Padding data: <MISSING>

Whereas OpenSSL sends:

Extension: Padding
 Type: Padding (0x0015)
 Length: 218
 Padding Data: 000000...
   Padding length: 0
   Padding data: <MISSING>

It looks like ocaml-tls rejects padding that has a length, shouldn't it parse the length and check just that the data is all zeroes?

hannesm commented 9 years ago

edwin, thanks for your research. according to padding extension draft https://tools.ietf.org/html/draft-ietf-tls-padding-01#section-3 the data must be all 0. (we mentioned this behaviour, although we didn't knew it was GnuTLS in 7.1 of our paper).

there are two things to do: inform the GnuTLS people that they should not insert a length field in the padding data, and evaluate whether we should be more lose in what we accept.

hannesm commented 9 years ago

which version of GnuTLS are you using?

edwintorok commented 9 years ago

gnutls-cli 3.3.8

I don't know much about how to read ASN.1 specifications (or encoding), is the field extension_data usually prefixed with its length in ASN.1/DER?

      struct {
          ExtensionType extension_type;
          opaque extension_data<0..2^16-1>;
      } Extension;
hannesm commented 9 years ago

the example in section 3 is pretty clear: extension_type 0x00 0x15 followed by length 0x00 0x06 followed by length amount of 0s..

edwintorok commented 9 years ago

Looks like wireshark looks for extension length, then padding length, then padding data, and GnuTLS adds the length twice (once for extension, once for the padding itself). I agree that it'd be useful to discuss with them to see why they chose to implement it that way.

hannesm commented 9 years ago

looking through GnuTLS git source code, I cannot find the padding data extension. maybe it already got removed!?

edwintorok commented 9 years ago

Its called dumbfw because its used to work around dumb firewalls that have trouble in the 256-511 range with ClientHello :)

hannesm commented 9 years ago

yes, I found the dumbfw code as well, and it indeed still prefixes the padding data with a length. do you want to report an issue at gnutls.org, or should I?

edwintorok commented 9 years ago

Please go ahead and report it, you actually know about TLS implementation details, I'm just a user :)

edwintorok commented 9 years ago

On 07/09/2015 11:44 AM, Hannes Mehnert wrote:

evaluate whether we should be more lose in what we accept.

I think it'd be consistent with the 'we thereby capture the practical de facto standard' mentioned in your paper to accept the GnuTLS behaviour (first two bytes either 00 00, or the length of padding).

Once an updated gnutls or wget is sufficiently widely deployed in major distributions you could revert to the strict checking you have now. The extension draft says 'Servers MAY verify that the extension is either empty or contains only zero bytes', so you're not required to reject all paddings that are not conforming.

hannesm commented 9 years ago

fixed in GnuTLS https://gitlab.com/gnutls/gnutls/commit/5955025e72a9cd2b1eadcd5191a28e3c05ad60f1

@edwintorok sure, but not being strict in checking introduces a covert channel (as mentioned in section 5 of the draft).. still unclear to me @pqwy what's your opinion?

pqwy commented 9 years ago

@edwintorok Has a pretty reasonable position, esp. given that what TLS is is more-or-less an empirical fact. And the "covert channel" mention in the draft sounds a little hand-wavy, it is unclear to me how this could be used for anything.

Then again, I'm reminded of this. GnuTLS is constantly causing small headache, and I'm not inclined to have any code that specifically caters to their many historical quirks. If anything, I would rather adopt a no-exceptions policy for that library: if GnuTLS is the only dissenting voice on something, I would ignore it on principle.

It's also worth noting that this problem is encountered only when using GnuTLS' own "quirks mode." Adding our quirks mode to adapt to their quirks mode sounds like an arms race in arbitrariness... :smile:

hannesm commented 9 years ago

in the traces from https://tls.openmirage.org we encountered 7 times (out of > 22000) this bad padding extension. this is such a minority that I don't think we need to make our checks more lose.

hannesm commented 9 years ago

giving the small percentage of systems which are affected, closing this with WONTFIX. If I misjudged the cardinality of affected systems, please reopen..

UltimateByte commented 8 years ago

Hi there,

I guess a fix on that could still be appreciated for some Debian 7 users. :) http://steamcommunity.com/groups/linuxgsm/discussions/0/364041517010932228/

hannesm commented 8 years ago

@UltimateByte two things: a) it is a bug in GnuTLS, fixed upstream - debian people should poke their maintainers to get fixed packages b) I don't believe OCaml-TLS is running on github or cloudflare servers, thus it is likely another TLS stack which has the same behaviour as ours.

UltimateByte commented 8 years ago

@hannesm Thanks for bothering answering me. :) Pardon my ignorance, i thought this was related but it seems like i know nothing :p