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

Permissive TLS parsing #400

Merged
merged 2 commits into from
Feb 18, 2024
Merged

Permissive TLS parsing #400

merged 2 commits into from
Feb 18, 2024

Conversation

Seanstoppable
Copy link
Contributor

@Seanstoppable Seanstoppable commented Nov 28, 2023

This is driven by #378, zmap/zcrypto#364 and #334

This allows a number of scans to actually succeed, rather than fail out when parsing the certificate

Example without permissive parsing:

echo FAILING_IP | ./zgrab2 http -p 443 --use-https
INFO[0000] started grab at 2023-09-21T21:25:29-05:00
{"ip":"FAILING_IP","data":{"http":{"status":"unknown-error","protocol":"http","result":{},"timestamp":"2023-09-21T21:25:29-05:00","error":"tls: failed to parse certificate from server: asn1: structure error: explicitly tagged member didn't match"}}}
INFO[0001] finished grab at 2023-09-21T21:25:29-05:00
{"statuses":{"http":{"successes":0,"failures":1}},"start":"2023-09-21T21:25:29-05:00","end":"2023-09-21T21:25:29-05:00","duration":"987.606886ms"}

With Permissive parsing:

echo FAILING_IP | ./zgrab2 http -p 443 --use-https
INFO[0000] started grab at 2023-09-21T21:25:34-05:00
{"ip":"FAILING_UP","data":{"http":{"status":"application-error","protocol":"http","result":{"response":{"status_line":"302 Found","status_code":302,"protocol":{"name":"HTTP/1.1","major":1,"minor":1},"headers":{"content_length":["0"],
... all the HTTP and TLS handshake log data

This is driven by  zmap#378, zmap/zcrypto#364 and
zmap#334

This allows a number of scans to actually succeed, rather than fail out
when parsing the certificate

Example without permissive parsing:

```
echo FAILING_IP | ./zgrab2 http -p 443 --use-https
INFO[0000] started grab at 2023-09-21T21:25:29-05:00
{"ip":"FAILING_IP","data":{"http":{"status":"unknown-error","protocol":"http","result":{},"timestamp":"2023-09-21T21:25:29-05:00","error":"tls: failed to parse certificate from server: asn1: structure error: explicitly tagged member didn't match"}}}
INFO[0001] finished grab at 2023-09-21T21:25:29-05:00
{"statuses":{"http":{"successes":0,"failures":1}},"start":"2023-09-21T21:25:29-05:00","end":"2023-09-21T21:25:29-05:00","duration":"987.606886ms"}
```

With Permissive parsing:

```
echo FAILING_IP | ./zgrab2 http -p 443 --use-https --permissive-parsing
INFO[0000] started grab at 2023-09-21T21:25:34-05:00
{"ip":"FAILING_UP","data":{"http":{"status":"application-error","protocol":"http","result":{"response":{"status_line":"302 Found","status_code":302,"protocol":{"name":"HTTP/1.1","major":1,"minor":1},"headers":{"content_length":["0"],
... all the HTTP and TLS handshake log data
```
@Seanstoppable Seanstoppable changed the title Fox over TLS Permissive TLS parsing Nov 28, 2023
@mzpqnxow
Copy link
Contributor

mzpqnxow commented Dec 5, 2023

@dadrian can you consider this for merging if you have a moment? Literally a moment, probably - it's a very small change, but an impactful one in large collections, hence the nudge 😊

Thanks @Seanstoppable for submitting this, it fell completely off my radar

@dadrian
Copy link
Member

dadrian commented Dec 5, 2023

We could probably default to this tbh

@mzpqnxow
Copy link
Contributor

mzpqnxow commented Dec 6, 2023

We could probably default to this tbh

That would be awesome- I wasn't sure how it might impact the output schemas that y'all have, and if it might break any test-cases you have, so I didn't want to be presumptuous

Probably the vast majority of users would be appreciative of it as a default

@Seanstoppable
Copy link
Contributor Author

I'm happy to change this to be default behavior if that is the desire. Just opted to do behavioral backwards compatibility.

@Seanstoppable
Copy link
Contributor Author

Updated to just be the default

@zakird zakird merged commit 413c2ce into zmap:master Feb 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants