[BUGFIX] Set SNI on each redirect to avoid handshake failures / incorrect server name

[mzpqnxow]

This is a fix for issue #300, where the SNI server name was only set once, at the start of an HTTP scan. This caused incorrect SNI server name values when not using --no-sni or --server-name. The example case is in #300

Details follow, the initial commit was a botched patch but the latest commit fixes it. It was a little more involved than I thought it would be, though still a pretty small change

[mzpqnxow]

@Rich5 my first patch was bad. This version should fully fix your issues. Can you please test it when you have a moment? It's working for me on your sample hosts and does not break --no-sni or --server-name if you're depending on those for other uses

@dadrian, @elliot-censys, @zakird I think this patch is important as it may impact a non-negligible amount of HTTPS endpoints. Would you take a look at it if you have time? It's a small patch- I included comments that you can remove if you like. The only thing that a mistake in this patch could impact is SNI-related functionality- specifically --no-sni and --server-name. I tested both and they seem to be working as expected

I needed to make a minor change to function parameters for the function returned by getTLSDialer() because there was a namespace collision with the net module and a parameter- I just renamed the parameter from net to network

The gist (behaviorally) can be seen using the example @Rich5 provided in #300:

echo granite-logistics.com | zgrab2 http --max-redirects 10`

For this endpoint, the behavior before the patch looked like this:

http://granite-logistics.com 302 -> https://granite-logistics.com (SNI=granite-logistics.com) 302 --> https://www.granite-logistics.com (SNI=granite-logistics.com) : SNI FAIL

This patch just updates the SNI server name on every request in a redirect chain (unless --no-sni or --server-name are set) so that in the example, the final request to https://www.granite-logistics.com has the correct SNI server name value set, allowing the negotiation to complete. This can be confirmed by checking that the correct content was grabbed and/or by checking the protocol negotiation on the wire to see the SNI server name value

The important point here is that the old behavior causes servers that have mandatory SNI to either fail negotiation entirely or return a fallback certificate and fallback virtual host configuration

This specific case happened to be Cloudflare, which obviously makes up a big chunk of HTTPS on the web and is only getting bigger. I believe Cloudflare customers need to explicitly opt-in to SNI- they definitely need to opt-in to an SNI-only configuration- so it may not be too much of a problem now, but it's going to get bigger over time as they are pushing SNI pretty hard. They'are also pushing ESNI but I believe this will just be available opportunistically alongside SNI because hardly any browsers or client libraries support it.

While we're here- I want to point out two other behaviors I noticed, both with and without this change

1. Use of --server-name sets the SNI server name on all HTTPS connections, not just the initial one. This may be intentional / well-known, but it caught me off guard and I think in general it's not how most users would want it to behave. That said, use of --server-name is probably not very widespread so it's not a big deal. I'm happy to open it as an issue if there's any agreement that it's something that should be changed
2. A failure anywhere in the redirect chain causes the --retry-https logic to fire, even if the first request elicited a successful plaintext HTTP response. I think it would make sense to either change the default behavior to not retry as https in this case, or maybe add a flag (somewhat similar to --redirects-succeed) to prevent it. It seems to be that in 99% of cases this is just wasted bandwidth and/or a very confusing error message when the SSL handshake tries to interpret P/ as an SSL message length. I opened up Mark request as successful (short-circuit retry-https) if redirect chain fails due to SSL handshake #307 to get an opinion on that behavior

Thanks, I appreciate your patience and help with everything- I can imagine that features/bugfixes aside, just "keeping the lights on" for zcrypto, zmap and zgrab2 probably takes up a lot of your time

BTW- If anyone would like to take a look at #302 that would be great too. It's a much simpler change :)

[engn33r]

I encountered no issues when testing this PR in my use cases 👍