ServerHello

[martind1111]

I am trying to create a ServerHello with a supported_version extension. Here is how I am trying to create this message (the syntax may not be exact because I transcripted the command before copying it here):

pkt = Ether()/IP()/TCP(sport=54988, dport=4433)/TLSRecord(version='TLS_1_2')/TLSHandshakes(handshakes=[TLSHandshake()/TLSServerHello(session_id_length=32, session_id='\x30\xcf\x00\x021010101010101010101010101010', cipher_suite=TLSCipherSuite.ECDHE_RSA_WITH_AES_256_CBC_SHA, extensions=[TLSExtension(length=2)/TLSExtSupportedVersions(length=3, version=[0x0400])])])

When I write this packet to a PCAP and load it into Wireshark, Wireshark complains about the extensions portion. There is a dangling byte after the supported_version extension. Actually, the length of the supported versions extension is correctly set to 2 (although the command above sets it to 3) and the length field in the supported version extension is concatenated with the first byte of the version field to give a version of 0x0304. After the version field, there is an extra byte appearing in the PCAP file.

I am wondering if this is a known problem, or maybe I should create my message in a different manner to avoid the message problem.

I am using scapy 2.3.2 on a CentOS 7 server and scapy-ssl_tls 2.0.0.

[martind1111]

I have started playing with source code and was able to create a ServerHello with a supported version extension. I had to create a new copy of the supported version extension class as:
class TLSExtensionSupportedVersion(PacketNoPayload):
name = "TLS Extension Supported Version"
fields_desc = [ReprFieldListField("versions", [TLSVersion.TLS_1_3], XShortEnumField("version", None, TLS_VERSION), length_from=lambda x: x.length)]

Then, I can create a ServerHello with:
pkt = Ether()/IP()/TCP(sport=54988, dport=4433)/TLSRecord(version='TLS_1_2')/TLSHandshakes(handshakes=[TLSHandshake()/TLSServerHello(session_id_length=32, session_id='\x30\xcf\x00\x021010101010101010101010101010', cipher_suite=TLSCipherSuite.ECDHE_RSA_WITH_AES_256_CBC_SHA, extensions=[TLSExtension()/TLSExtSupportedVersion(versions=['TLS_1_2'])])])

Note that the supported version extension in case of ServerHello can only contain one version (the negotiated version).

It looks like the section that causes an issue is this code:
XFieldLenField("length", None, length_of="versions", fmt="B"),

I am not sure why this is causing an issue specifically for the ServerHello, because it does not create a problem with the ClientHello. It also does not make quite sense that removing the line that seems to be required to figure out the length of the extension would be OK to remove for ServerHello. However, using this specialized version for ServerHello seems to fix the problem. I could not just fix the original class, because then, the ClientHello would not build properly.

[alexmgr]

Hi @martind1111,

Scapy-ssl_tls only supports TLS 1.3 draft 18, until I find time to implement the final spec.
That change (having the server use TLSExtSupportedVersions instead of the SH version field) happened later in the spec, so the implementation is not there for the server side.

If you look at the spec: you'll see that the client struct is a vector, whilst the server side is a plain version (which makes sense). That byte difference causes the parsing to fail in the server case.

To fix this, you'd need to split the extension in 2 (one server, one client) and add some heuristic parsing logic in guess_payload_class of the TLSExtSupportedVersions.

Cheers,
Alex

[martind1111]

Alex, Thanks for your response. I have seen some examples of extension in the code that split between client and server and used the guess_payload_class. I will try to do something similar and will send you my modification for review. Martin

…

On Wed, Sep 12, 2018 at 6:29 PM, alexmgr ***@***.***> wrote: Hi @martind1111 <https://github.com/martind1111>, Scapy-ssl_tls only supports TLS 1.3 draft 18, until I find time to implement the final spec. That change (having the server use TLSExtSupportedVersions instead of the SH version field) happened later in the spec, so the implementation is not there for the server side. If you look at the spec <https://tools.ietf.org/html/draft-ietf-tls-tls13-28#section-4.2.1>: you'll see that the client struct is a vector, whilst the server side is a plain version (which makes sense). That byte difference causes the parsing to fail in the server case. To fix this, you'd need to split the extension in 2 (one server, one client) and add some heuristic parsing logic in guess_payload_class of the TLSExtSupportedVersions. Cheers, Alex — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#141 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AFHLOXhsgDb7H-RA3eosy2WYaKPyvpGsks5uaYqtgaJpZM4WkbFF> .

[martind1111]

Alex, I am trying to do the modifications to the code to be able to encode a supported_versions extension for client and server. I have something that works for cases where only one supported_version is present in the ClientHello, but have a hard time figuring out if it works for multiple supported_versions. Since my code is very similar to key_share, I was wondering if you could show me how we could implement a ClientHello message that would have multiple KeyShareEntry in the TLSExtKeyShare extension. I cee there is a cleint_share parameter, but I don`t know how to assigna list of KeyShareEntry to that parameter or if it is necessary to specify the client_shares parameter. Cheers, Martin

…

On Wed, Sep 12, 2018 at 6:29 PM, alexmgr ***@***.***> wrote: Hi @martind1111 <https://github.com/martind1111>, Scapy-ssl_tls only supports TLS 1.3 draft 18, until I find time to implement the final spec. That change (having the server use TLSExtSupportedVersions instead of the SH version field) happened later in the spec, so the implementation is not there for the server side. If you look at the spec <https://tools.ietf.org/html/draft-ietf-tls-tls13-28#section-4.2.1>: you'll see that the client struct is a vector, whilst the server side is a plain version (which makes sense). That byte difference causes the parsing to fail in the server case. To fix this, you'd need to split the extension in 2 (one server, one client) and add some heuristic parsing logic in guess_payload_class of the TLSExtSupportedVersions. Cheers, Alex — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#141 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AFHLOXhsgDb7H-RA3eosy2WYaKPyvpGsks5uaYqtgaJpZM4WkbFF> .

[martind1111]

I have clone the master branch and created a branch that adds some support for TLS 1.3. I can at least now create ServerHello. I am wondering how I can contribute back to the project, but I don't know how to commit the work back. When I tried to push my branch, I got the following error:

git push origin tls-1.3

Username for 'https://github.com': martind1111
Password for 'https://martind1111@github.com':
remote: Permission to tintinweb/scapy-ssl_tls.git denied to martind1111.
fatal: unable to access 'https://github.com/tintinweb/scapy-ssl_tls.git/': The requested URL returned error: 403