Skip to content

Comments

Feature tls 13 draft 23#1167

Closed
Pirabarlen-Cheenaramen wants to merge 3 commits intoaria2:masterfrom
Pirabarlen-Cheenaramen:feature-tls-13-draft-23
Closed

Feature tls 13 draft 23#1167
Pirabarlen-Cheenaramen wants to merge 3 commits intoaria2:masterfrom
Pirabarlen-Cheenaramen:feature-tls-13-draft-23

Conversation

@Pirabarlen-Cheenaramen
Copy link

@Pirabarlen-Cheenaramen Pirabarlen-Cheenaramen commented Mar 18, 2018

tatsuhiro-t
tatsuhiro-t reviewed Mar 21, 2018
View reviewed changes
src/LibsslTLSContext.cc
good_ = false;
A2_LOG_ERROR(fmt("SSL_CTX_set_cipher_list() failed. Cause: %s",
ERR_error_string(ERR_get_error(), nullptr)));
}
Copy link
Collaborator

@tatsuhiro-t tatsuhiro-t Mar 21, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't this block still required if TLS version other than TLSv1.3 is negotiated?

Copy link
Author

@Pirabarlen-Cheenaramen Pirabarlen-Cheenaramen Mar 21, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi Tatsuhiro,
From the draft, https://tools.ietf.org/html/draft-ietf-tls-tls13-23#page-133
I quote:

In the absence of an application profile standard specifying
   otherwise, a TLS-compliant application MUST implement the
   TLS_AES_128_GCM_SHA256 [GCM] cipher suite and SHOULD implement the
   TLS_AES_256_GCM_SHA384 [GCM] and TLS_CHACHA20_POLY1305_SHA256
   [RFC7539] cipher suites.  (see Appendix B.4)

By not setting the cipher list, we chose to use TLS 1.3's default cipher list, which gives us exactly what the draft proposes and ensures that it will work with all TLS 1.3 servers which is compliant.

Sure enough, if I remove that ifndef TLS1_3_VERSION, the handshake works with all servers I have tested with, but I won't then be able to guarantee that it will work for other implementations.

See the handshake part (under Cipher Suites), we got the proper cipher that is required:

Secure Sockets Layer
    TLSv1.3 Record Layer: Handshake Protocol: Client Hello
        Content Type: Handshake (22)
        Version: TLS 1.0 (0x0301)
        Length: 512
        Handshake Protocol: Client Hello
            Handshake Type: Client Hello (1)
            Length: 508
            Version: TLS 1.2 (0x0303)
            Random: a39aa06e4815fd0aed82f1d25ae7faf3927028dd25716809...
            Session ID Length: 32
            Session ID: 449cb70e2e7a21dfeb529b527853069a5574ce8aad1809e8...
            Cipher Suites Length: 8
            Cipher Suites (4 suites)
                Cipher Suite: TLS_AES_256_GCM_SHA384 (0x1302)
                Cipher Suite: TLS_CHACHA20_POLY1305_SHA256 (0x1303)
                Cipher Suite: TLS_AES_128_GCM_SHA256 (0x1301)
                Cipher Suite: TLS_EMPTY_RENEGOTIATION_INFO_SCSV (0x00ff)
            Compression Methods Length: 1
            Compression Methods (1 method)
            Extensions Length: 427
            Extension: server_name (len=29)
                Type: server_name (0)
                Length: 29
                Server Name Indication extension
            Extension: ec_point_formats (len=4)
                Type: ec_point_formats (11)
                Length: 4
                EC point formats Length: 3
                Elliptic curves point formats (3)
            Extension: supported_groups (len=4)
                Type: supported_groups (10)
                Length: 4
                Supported Groups List Length: 2
                Supported Groups (1 group)
            Extension: SessionTicket TLS (len=0)
                Type: SessionTicket TLS (35)
                Length: 0
                Data (0 bytes)
            Extension: encrypt_then_mac (len=0)
                Type: encrypt_then_mac (22)
                Length: 0
            Extension: extended_master_secret (len=0)
                Type: extended_master_secret (23)
                Length: 0
            Extension: signature_algorithms (len=28)
                Type: signature_algorithms (13)
                Length: 28
                Signature Hash Algorithms Length: 26
                Signature Hash Algorithms (13 algorithms)
                    Signature Algorithm: ecdsa_secp256r1_sha256 (0x0403)
                    Signature Algorithm: ecdsa_secp384r1_sha384 (0x0503)
                    Signature Algorithm: ecdsa_secp521r1_sha512 (0x0603)
                    Signature Algorithm: ed25519 (0x0807)
                    Signature Algorithm: rsa_pss_pss_sha256 (0x0809)
                    Signature Algorithm: rsa_pss_pss_sha384 (0x080a)
                    Signature Algorithm: rsa_pss_pss_sha512 (0x080b)
                    Signature Algorithm: rsa_pss_rsae_sha256 (0x0804)
                    Signature Algorithm: rsa_pss_rsae_sha384 (0x0805)
                    Signature Algorithm: rsa_pss_rsae_sha512 (0x0806)
                    Signature Algorithm: rsa_pkcs1_sha256 (0x0401)
                    Signature Algorithm: rsa_pkcs1_sha384 (0x0501)
                    Signature Algorithm: rsa_pkcs1_sha512 (0x0601)

and Yes, this block is required with other TLS versions with an older server version. The patch is backwards compatible as well. See the testing results, more specifically the cipher suite section for other TLS versions. I did a rather full testing of it with the handshake detailed.

Thanks again for going through the patch.
Apologies for any grammatical errors.

Copy link
Author

@Pirabarlen-Cheenaramen Pirabarlen-Cheenaramen Mar 21, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry if i seem to be spamming,

Here's a test against a TLS 1.2 server, notice the cipher suites

##TLSv1.2 against a tls1.2 server

selven@thiruchendur:~/code/installs/aria/bin$ LD_PRELOAD=/home/selven/code/installs/openssl/lib/libssl.so.1.1:/home/selven/code/installs/openssl/lib/libcrypto.so.1.1 ./aria2c  https://hackers.mu --check-certificate=false --min-tls-version=TLSv1.2

03/17 19:41:29 [NOTICE] Downloading 1 item(s)
[#54db8f 0B/0B CN:1 DL:0B]                                                                                                                                                                                         
03/17 19:41:30 [NOTICE] File already exists. Renamed to /home/selven/code/installs/aria/bin/index.21.html.

03/17 19:41:30 [NOTICE] Download complete: /home/selven/code/installs/aria/bin/index.21.html

Download Results:
gid   |stat|avg speed  |path/URI
======+====+===========+=======================================================
54db8f|OK  |   177KiB/s|/home/selven/code/installs/aria/bin/index.21.html

Status Legend:
(OK):download completed.

##wireshark clienthello tlsv1.2 against tls1.2 server

Secure Sockets Layer
    TLSv1.2 Record Layer: Handshake Protocol: Client Hello
        Content Type: Handshake (22)
        Version: TLS 1.0 (0x0301)
        Length: 512
        Handshake Protocol: Client Hello
            Handshake Type: Client Hello (1)
            Length: 508
            Version: TLS 1.2 (0x0303)
            Random: c2fd424dbd2951ac2e8a7bfe2353224a146566d50605163e...
            Session ID Length: 32
            Session ID: 038f12decf8dcf74f289b4d395f6e67c5073292e00d8a31b...
            Cipher Suites Length: 62
            Cipher Suites (31 suites)
                Cipher Suite: TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384 (0xc02c)
                Cipher Suite: TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (0xc030)
                Cipher Suite: TLS_DHE_RSA_WITH_AES_256_GCM_SHA384 (0x009f)
                Cipher Suite: TLS_ECDHE_ECDSA_WITH_CHACHA20_POLY1305_SHA256 (0xcca9)
                Cipher Suite: TLS_ECDHE_RSA_WITH_CHACHA20_POLY1305_SHA256 (0xcca8)
                Cipher Suite: TLS_DHE_RSA_WITH_CHACHA20_POLY1305_SHA256 (0xccaa)
                Cipher Suite: TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256 (0xc02b)
                Cipher Suite: TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256 (0xc02f)
                Cipher Suite: TLS_DHE_RSA_WITH_AES_128_GCM_SHA256 (0x009e)
                Cipher Suite: TLS_ECDHE_ECDSA_WITH_AES_256_CBC_SHA384 (0xc024)
                Cipher Suite: TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA384 (0xc028)
                Cipher Suite: TLS_DHE_RSA_WITH_AES_256_CBC_SHA256 (0x006b)
                Cipher Suite: TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA256 (0xc023)
                Cipher Suite: TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256 (0xc027)
                Cipher Suite: TLS_DHE_RSA_WITH_AES_128_CBC_SHA256 (0x0067)
                Cipher Suite: TLS_ECDHE_ECDSA_WITH_AES_256_CBC_SHA (0xc00a)
                Cipher Suite: TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA (0xc014)
                Cipher Suite: TLS_DHE_RSA_WITH_AES_256_CBC_SHA (0x0039)
                Cipher Suite: TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA (0xc009)
                Cipher Suite: TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA (0xc013)
                Cipher Suite: TLS_DHE_RSA_WITH_AES_128_CBC_SHA (0x0033)
                Cipher Suite: TLS_RSA_WITH_AES_256_GCM_SHA384 (0x009d)
                Cipher Suite: TLS_RSA_WITH_AES_128_GCM_SHA256 (0x009c)
                Cipher Suite: TLS_AES_256_GCM_SHA384 (0x1302)
                Cipher Suite: TLS_CHACHA20_POLY1305_SHA256 (0x1303)
                Cipher Suite: TLS_AES_128_GCM_SHA256 (0x1301)
                Cipher Suite: TLS_RSA_WITH_AES_256_CBC_SHA256 (0x003d)
                Cipher Suite: TLS_RSA_WITH_AES_128_CBC_SHA256 (0x003c)
                Cipher Suite: TLS_RSA_WITH_AES_256_CBC_SHA (0x0035)
                Cipher Suite: TLS_RSA_WITH_AES_128_CBC_SHA (0x002f)
                Cipher Suite: TLS_EMPTY_RENEGOTIATION_INFO_SCSV (0x00ff)
            Compression Methods Length: 1
            Compression Methods (1 method)
            Extensions Length: 373
            Extension: server_name (len=15)
                Type: server_name (0)
                Length: 15
                Server Name Indication extension
            Extension: ec_point_formats (len=4)
                Type: ec_point_formats (11)
                Length: 4
                EC point formats Length: 3
                Elliptic curves point formats (3)
            Extension: supported_groups (len=4)
                Type: supported_groups (10)
                Length: 4
                Supported Groups List Length: 2
                Supported Groups (1 group)
            Extension: SessionTicket TLS (len=0)
                Type: SessionTicket TLS (35)
                Length: 0
                Data (0 bytes)
            Extension: encrypt_then_mac (len=0)
                Type: encrypt_then_mac (22)
                Length: 0
            Extension: extended_master_secret (len=0)
                Type: extended_master_secret (23)
                Length: 0
            Extension: signature_algorithms (len=46)
                Type: signature_algorithms (13)
                Length: 46
                Signature Hash Algorithms Length: 44
                Signature Hash Algorithms (22 algorithms)
            Extension: supported_versions (len=5)
                Type: supported_versions (43)
                Length: 5
                Supported Versions length: 4
                Supported Version: TLS 1.3 (draft 23) (0x7f17)
                Supported Version: TLS 1.2 (0x0303)
            Extension: psk_key_exchange_modes (len=2)
                Type: psk_key_exchange_modes (45)
                Length: 2
                PSK Key Exchange Modes Length: 1
                PSK Key Exchange Mode: PSK with (EC)DHE key establishment (psk_dhe_ke) (1)
            Extension: key_share (len=71)
                Type: key_share (51)
                Length: 71
                Key Share extension
            Extension: padding (len=182)
                Type: padding (21)
                Length: 182
                Padding Data: 000000000000000000000000000000000000000000000000...

Copy link
Author

@Pirabarlen-Cheenaramen Pirabarlen-Cheenaramen Mar 21, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

and a TLS 1.2 handshake against TLS1.3 server
##TLSv1.2 on against a tls1.3 server

selven@thiruchendur:~/code/installs/aria/bin$ LD_PRELOAD=/home/selven/code/installs/openssl/lib/libssl.so.1.1:/home/selven/code/installs/openssl/lib/libcrypto.so.1.1 ./aria2c  https://tls13.crypto.mozilla.org/ --check-certificate=false --min-tls-version=TLSv1.2

03/17 19:19:00 [NOTICE] Downloading 1 item(s)

03/17 19:19:00 [NOTICE] File already exists. Renamed to /home/selven/code/installs/aria/bin/index.7.html.

03/17 19:19:00 [NOTICE] Download complete: /home/selven/code/installs/aria/bin/index.7.html

Download Results:
gid   |stat|avg speed  |path/URI
======+====+===========+=======================================================
af225d|OK  |   3.2MiB/s|/home/selven/code/installs/aria/bin/index.7.html

Status Legend:
(OK):download completed.

##wireshark clienthello tlsv1.2

Secure Sockets Layer
    TLSv1.3 Record Layer: Handshake Protocol: Client Hello
        Content Type: Handshake (22)
        Version: TLS 1.0 (0x0301)
        Length: 512
        Handshake Protocol: Client Hello
            Handshake Type: Client Hello (1)
            Length: 508
            Version: TLS 1.2 (0x0303)
            Random: 9a7b336328f5b78052ab73fabc90fde4d118fe6c785da8e6...
            Session ID Length: 32
            Session ID: c1188811625630ce7a3f31dac372ec2eae5ed84b995fa2b1...
            Cipher Suites Length: 62
            Cipher Suites (31 suites)
                Cipher Suite: TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384 (0xc02c)
                Cipher Suite: TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (0xc030)
                Cipher Suite: TLS_DHE_RSA_WITH_AES_256_GCM_SHA384 (0x009f)
                Cipher Suite: TLS_ECDHE_ECDSA_WITH_CHACHA20_POLY1305_SHA256 (0xcca9)
                Cipher Suite: TLS_ECDHE_RSA_WITH_CHACHA20_POLY1305_SHA256 (0xcca8)
                Cipher Suite: TLS_DHE_RSA_WITH_CHACHA20_POLY1305_SHA256 (0xccaa)
                Cipher Suite: TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256 (0xc02b)
                Cipher Suite: TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256 (0xc02f)
                Cipher Suite: TLS_DHE_RSA_WITH_AES_128_GCM_SHA256 (0x009e)
                Cipher Suite: TLS_ECDHE_ECDSA_WITH_AES_256_CBC_SHA384 (0xc024)
                Cipher Suite: TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA384 (0xc028)
                Cipher Suite: TLS_DHE_RSA_WITH_AES_256_CBC_SHA256 (0x006b)
                Cipher Suite: TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA256 (0xc023)
                Cipher Suite: TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256 (0xc027)
                Cipher Suite: TLS_DHE_RSA_WITH_AES_128_CBC_SHA256 (0x0067)
                Cipher Suite: TLS_ECDHE_ECDSA_WITH_AES_256_CBC_SHA (0xc00a)
                Cipher Suite: TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA (0xc014)
                Cipher Suite: TLS_DHE_RSA_WITH_AES_256_CBC_SHA (0x0039)
                Cipher Suite: TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA (0xc009)
                Cipher Suite: TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA (0xc013)
                Cipher Suite: TLS_DHE_RSA_WITH_AES_128_CBC_SHA (0x0033)
                Cipher Suite: TLS_RSA_WITH_AES_256_GCM_SHA384 (0x009d)
                Cipher Suite: TLS_RSA_WITH_AES_128_GCM_SHA256 (0x009c)
                Cipher Suite: TLS_AES_256_GCM_SHA384 (0x1302)
                Cipher Suite: TLS_CHACHA20_POLY1305_SHA256 (0x1303)
                Cipher Suite: TLS_AES_128_GCM_SHA256 (0x1301)
                Cipher Suite: TLS_RSA_WITH_AES_256_CBC_SHA256 (0x003d)
                Cipher Suite: TLS_RSA_WITH_AES_128_CBC_SHA256 (0x003c)
                Cipher Suite: TLS_RSA_WITH_AES_256_CBC_SHA (0x0035)
                Cipher Suite: TLS_RSA_WITH_AES_128_CBC_SHA (0x002f)
                Cipher Suite: TLS_EMPTY_RENEGOTIATION_INFO_SCSV (0x00ff)
            Compression Methods Length: 1
            Compression Methods (1 method)
            Extensions Length: 373
            Extension: server_name (len=29)
                Type: server_name (0)
                Length: 29
                Server Name Indication extension
            Extension: ec_point_formats (len=4)
                Type: ec_point_formats (11)
                Length: 4
                EC point formats Length: 3
                Elliptic curves point formats (3)
            Extension: supported_groups (len=4)
                Type: supported_groups (10)
                Length: 4
                Supported Groups List Length: 2
                Supported Groups (1 group)
            Extension: SessionTicket TLS (len=0)
                Type: SessionTicket TLS (35)
                Length: 0
                Data (0 bytes)
            Extension: encrypt_then_mac (len=0)
                Type: encrypt_then_mac (22)
                Length: 0
            Extension: extended_master_secret (len=0)
                Type: extended_master_secret (23)
                Length: 0
            Extension: signature_algorithms (len=46)
                Type: signature_algorithms (13)
                Length: 46
                Signature Hash Algorithms Length: 44
                Signature Hash Algorithms (22 algorithms)
                    Signature Algorithm: ecdsa_secp256r1_sha256 (0x0403)
                    Signature Algorithm: ecdsa_secp384r1_sha384 (0x0503)
                    Signature Algorithm: ecdsa_secp521r1_sha512 (0x0603)
                    Signature Algorithm: ed25519 (0x0807)
                    Signature Algorithm: rsa_pss_pss_sha256 (0x0809)
                    Signature Algorithm: rsa_pss_pss_sha384 (0x080a)
                    Signature Algorithm: rsa_pss_pss_sha512 (0x080b)
                    Signature Algorithm: rsa_pss_rsae_sha256 (0x0804)
                    Signature Algorithm: rsa_pss_rsae_sha384 (0x0805)
                    Signature Algorithm: rsa_pss_rsae_sha512 (0x0806)
                    Signature Algorithm: rsa_pkcs1_sha256 (0x0401)
                    Signature Algorithm: rsa_pkcs1_sha384 (0x0501)
                    Signature Algorithm: rsa_pkcs1_sha512 (0x0601)
                    Signature Algorithm: SHA224 ECDSA (0x0303)
                    Signature Algorithm: ecdsa_sha1 (0x0203)
                    Signature Algorithm: SHA224 RSA (0x0301)
                    Signature Algorithm: rsa_pkcs1_sha1 (0x0201)
                    Signature Algorithm: SHA224 DSA (0x0302)
                    Signature Algorithm: SHA1 DSA (0x0202)
                    Signature Algorithm: SHA256 DSA (0x0402)
                    Signature Algorithm: SHA384 DSA (0x0502)
                    Signature Algorithm: SHA512 DSA (0x0602)

Again apologies for so many test logs in the comment section.

Copy link
Collaborator

@tatsuhiro-t tatsuhiro-t Apr 11, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry for the delay. The intention of my question is that if the block is removed, it may enable unsafe cipher suite if TLSv1.2 or lower is negotiated. It looks it is not an issue at this time.

tatsuhiro-t
tatsuhiro-t reviewed Apr 11, 2018
View reviewed changes
src/LibsslTLSContext.cc
long ver_opts = 0;
switch (minVer) {
case TLS_PROTO_TLS13:
ver_opts |= SSL_OP_NO_TLSv1_2;
Copy link
Collaborator

@tatsuhiro-t tatsuhiro-t Apr 11, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be nice to add comment "// fall through" here

Copy link
Author

@Pirabarlen-Cheenaramen Pirabarlen-Cheenaramen Jul 4, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good to hear from you! I thought you were dropping the pr, don't merge it yet then, I'll try to add some comments and update this PR with the changes from draft 28 in the coming 2 weeks. Myself I Apologize for the late reply.

@tatsuhiro-t tatsuhiro-t mentioned this pull request Apr 7, 2019
Merged
@tatsuhiro-t
Copy link
Collaborator

tatsuhiro-t commented Apr 7, 2019

To modernize TLS protocol in aria2, I made a PR #1385 which also enables TLSv1.3 for OpenSSL and GNUTLS.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@tatsuhiro-t tatsuhiro-t tatsuhiro-t left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@Pirabarlen-Cheenaramen @tatsuhiro-t