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

Feature/portable suites #302

Merged
merged 4 commits into from
Apr 6, 2021
Merged

Conversation

mzpqnxow
Copy link
Contributor

@mzpqnxow mzpqnxow commented Mar 4, 2021

This makes use of the PortableCiphers that was just merged into zcrypto

The original issue for this on the zgrab2 side is #285

Usage is zgrab2 http --cipher-suite portable ...

I tested it against a large swath of HTTPS services and got a 1.5% increase in successful handshakes. I didn't see any evidence of problems

@mzpqnxow
Copy link
Contributor Author

CI is busted but seems that's being worked on.

@zakird you may want to take a look at this PR- now that the change in zcrypto has been merged, this is really a very minor change but it can have a really nice benefit at scale

@engn33r
Copy link
Contributor

engn33r commented Mar 20, 2021

@mzpqnxow Thanks for your work on this topic. FYI I tried to include this PR into my fork, but I got the following error when I tried to build my fork. I get the same error when I try to build the feature/portable-suites branch of your fork. It's probably due to a mistake on my part, but I thought I'd mention it:

../../tls.go:94:22: undefined: "github.com/zmap/zcrypto/tls".PortableCiphers
make: *** [Makefile:24: zgrab2] Error 2

@mzpqnxow
Copy link
Contributor Author

mzpqnxow commented Mar 21, 2021

@mzpqnxow Thanks for your work on this topic. FYI I tried to include this PR into my fork, but I got the following error when I tried to build my fork. I get the same error when I try to build the feature/portable-suites branch of your fork. It's probably due to a mistake on my part, but I thought I'd mention it:

../../tls.go:94:22: undefined: "github.com/zmap/zcrypto/tls".PortableCiphers
make: *** [Makefile:24: zgrab2] Error 2

Interesting.. I don't know too much about the go build system, it's possible it has a zcrypto master locally that's not up to date. If that's the case, using go build -a ./... && go install ./... may work for you (though probably not, I'm not sure it will actually pull down the modules, it may just rebuild the local ones)

If that doesn't help, the following should work- this just clones the zcrypto master and then points your local fork at it. The -a tells it to force rebuild iirc

$ mkdir ~/zportable && cd ~/zportable
$ git clone https://github.com/zmap/zcrypto
$ cp -r ~/your/zgrab/fork .
$ go mod edit -replace github.com/zmap/zcrypto=../zcrypto
$ go get all  # You may or may not need this
$ go install ./...

Hope that's helpful. I did confirm that the zcrypto master branch does have the PR merged- https://github.com/zmap/zcrypto/blob/master/tls/cipher_suites.go#L1122

So it must be using an older branch/tag. One other thing you can do is take a look at the go.mod file in your zgrab2 fork to see if it's pointing somewhere odd for zcrypto (maybe another branch, some specific release tag instead of master?)

EDIT, @engn33r I forgot to tag you, not sure if you would see this otherwise. Also I mixed up what you were trying to do so I updated my comment...

@engn33r
Copy link
Contributor

engn33r commented Mar 26, 2021

@mzpqnxow the issue was on my end, and right where you suggested it would be. I too observed a noticeable improvement in HTTPS responses after using the portable cipher suite. No issues with this PR!

The issue I had was that the dependencies in the go.mod file did not have an updated zmap/zcrypto version from 2021. Instead, it was pulling the 2020 release in the current go.mod file. The following commands solved my issue:

$ cd <zgrab2 directory with this PR>
$ go get all
$ go get -u ./...
$ make

A couple unrelated additional comments that may also (or may not) help with increasing successful handshakes:

  • The dependencies in go.mod are outdated. I'm not sure if updating these with go get -u ./... makes a big difference, but if it doesn't break anything, it's probably a good idea to update. I will let others judge if this is worth creating a separate issue for, or perhaps it is implied that users should update before building.
  • When I run the same zgrab2 HTTP scan multiple times, I get (substantially) different results, even testing with this PR. I am not sure why this is - perhaps someone can provide some insight. One issue I encountered when providing zgrab with domain names instead of IPs was that DNS lookup process ended up being a bottleneck. This old zmap issue echos this. I'm currently testing massdns to perform the dns lookup first and passing that data in a file to zgrab. I don't have data if using massdns offers a substantial improvement in successful handshakes, but it does reduce the network load when zgrab is running if the DNS info is provided, and massdns allows for multiple DNS resolvers to be used.

@mzpqnxow
Copy link
Contributor Author

mzpqnxow commented Apr 6, 2021

  • When I run the same zgrab2 HTTP scan multiple times, I get (substantially) different results, even testing with this PR. I am not sure why this is - perhaps someone can provide some insight

What does the log say for the causes? Handshake errors, TCP failures, ...?

@dadrian dadrian merged commit 0eb497e into zmap:master Apr 6, 2021
@dadrian
Copy link
Member

dadrian commented Apr 6, 2021

I'll bump go.mod in a follow-up.

@dadrian
Copy link
Member

dadrian commented Apr 6, 2021

Done in 4e04784

@engn33r
Copy link
Contributor

engn33r commented Apr 9, 2021

  • When I run the same zgrab2 HTTP scan multiple times, I get (substantially) different results, even testing with this PR. I am not sure why this is - perhaps someone can provide some insight

What does the log say for the causes? Handshake errors, TCP failures, ...?

To wrap up the last item here, the issue I experienced with inconsistent results was due to my DNS config, not that of any ZGrab2 code. A tip for any future readers encountering DNS resolution issues is to try using different DNS providers (Google, Quad9, etc.). Cloudflare was quicker to throttle my DNS requests than others and my results are much better after switching to 8.8.8.8. I also switched to dnsmasq, which appears to work better than what I had before, but I expect that I'm only scratching the surface of DNS improvements here.

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