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

ServerHello #141

Open
martind1111 opened this issue Sep 12, 2018 · 5 comments
Open

ServerHello #141

martind1111 opened this issue Sep 12, 2018 · 5 comments

Comments

@martind1111
Copy link

martind1111 commented Sep 12, 2018

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
Copy link
Author

martind1111 commented Sep 12, 2018

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
Copy link
Collaborator

alexmgr commented Sep 12, 2018

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
Copy link
Author

martind1111 commented Sep 13, 2018 via email

@martind1111
Copy link
Author

martind1111 commented Sep 18, 2018 via email

@martind1111
Copy link
Author

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://[email protected]':
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

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

No branches or pull requests

2 participants