Fixes a few issues with redirects#462
Merged
phillip-stephens merged 9 commits intozmap:masterfrom May 13, 2025
Merged
Conversation
First, this prevents a DNS lookup from happening when we encounter a redirect, *even if we don't intend to follow it*. This likely addresses some part of zmap#452 Second, if we aren't following redirects, don't have the scan fail in an 'application-error'. We are succeeding in what we intended to do, which is to scan without following redirects
Properly handle no redirects wanted to return success
phillip-stephens
added a commit
that referenced
this pull request
Jun 18, 2025
Cherry-pick: 8981417 * Fixes a few issues with redirects First, this prevents a DNS lookup from happening when we encounter a redirect, *even if we don't intend to follow it*. This likely addresses some part of #452 Second, if we aren't following redirects, don't have the scan fail in an 'application-error'. We are succeeding in what we intended to do, which is to scan without following redirects * Make sure to check redirects before we loop through and parse the host Properly handle no redirects wanted to return success * Handle 0 indexing * Pull out the original checkRedirectCode so we deal with consistently * lint * add redirect fix to ipp module --------- Co-authored-by: Phillip Stephens <phillip@cs.stanford.edu> Co-authored-by: Zakir Durumeric <zakird@gmail.com>
phillip-stephens
added a commit
that referenced
this pull request
Sep 19, 2025
* added http1.1 and 2.0 test dockers * fix read requests tests * small fix for client tests * pull in the Go1.23 net/http without modifications (will not compile) Commit on go repo: 6885bad7dd86880be6929c02085e5c7a67ff2887 * new http module tests are compiling, not passing yet * all http tests passing * all http tests in all http/* dirs passing * merge client/server header list (#134) Cherry-pick 71315 * merge client/server header list * fix formatting of header.go * Add BodyHash field with hash type prefix Cherry-pick: 30e6 * Capture BodyTextLength Cherry-pick: fc82b6 * Deal with non-RFC compliant servers (#375) Cherry-pick 73639 There are hundreds of thousands of servers on the internet where the "INTENT" is to be an HTTP service, but in reality they do not properly conform to the HTTP RFC. In many cases, these servers will send back a simple: HTTP/1.1 404 Not Found With no `Connection: close` header, and terminates the established connection. While it's understood these are not RFC compliant, I would still like to see the servers that are intending to be HTTP servers. * http: allow raw header capture (#347) (#349) Cherry-pick 1e97dd8 The golang textproto library does a few things when parsing the HTTP headers: * consume some whitespace characters (e.g. \r\n) * canonicalizes the header keys (e.g. "content-type" => "Content-Type") * moves the headers into a map This all makes sense when parsing HTTP, but for a scanner some may want to have the exact headers, to match on order, non-canonical keys, etc. This adds that option, if '--raw-headers' is specified during an HTTP scan. This is accomplished by implementing a tee reader on the pconn interface, that tees before the bufio reader is put in place. The tee copy can be disabled once the headers have been read, so as to not waste memory while consuming the HTTP body. While denoted as "raw headers", this will also capture the raw status line as well. (cherry picked from commit 83e55e0) Signed-off-by: Jeff Cody <jcody@censys.io> * Fix http test error where RawHeaderBuffer wasn't copied on Clone * Fix tests and boundary condition (#410) Cherry-pick 990c9c3 Fix two failing tests Write a bunch more tests to expand coverage and check for errors Fix a bug where we can have no response and trigger a success * import sort * Don't disable `http` `tee` after first header, so that headers of re-directs can be captured and we don't panic (#504) Cherry-pick: bc9733c * 463 - add end bounds checking in TeeConn.Bytes * don't stop tee'ing after the first header, it means anything with a re-direct can't have its raw headers dumped and was causing a runtime panic if re-directs were used * Update the zcrypto dependency to bring in TLS 1.3 support (#507) Cherry-pick: 73944fc * build(deps): bump github.com/zmap/zcrypto * refactor(tests): update CipherSuites to use CipherSuiteID in MySQL tests * build(deps): bump zcrypto * build: static link zgrab in docker; ignore zgrab2 executable in dockerignore * fix(mssql): Read logic for TDS connection The old Read method was incorrectly implemented as ReadFull, causing it to wait indefinitely for a new packet that may never arrive under certain conditions. * build: sync latest zcrypto version --------- Co-authored-by: devStorm <59678453+developStorm@users.noreply.github.com> Co-authored-by: Zakir Durumeric <zakird@gmail.com> * Fixes a few issues with redirects (#462) Cherry-pick: 8981417 * Fixes a few issues with redirects First, this prevents a DNS lookup from happening when we encounter a redirect, *even if we don't intend to follow it*. This likely addresses some part of #452 Second, if we aren't following redirects, don't have the scan fail in an 'application-error'. We are succeeding in what we intended to do, which is to scan without following redirects * Make sure to check redirects before we loop through and parse the host Properly handle no redirects wanted to return success * Handle 0 indexing * Pull out the original checkRedirectCode so we deal with consistently * lint * add redirect fix to ipp module --------- Co-authored-by: Phillip Stephens <phillip@cs.stanford.edu> Co-authored-by: Zakir Durumeric <zakird@gmail.com> * import sort * Fix compile errors by new http library not having redirect function of appropriate signature * Fix test failures in lib/http * make test passes * make test passes and zgrab compiles * Add JSON marshalling to http lib Most output fields are matching master, still need to output the tls log * Update to latest zcrypto with HandshakeContext and NetConn * swap crypto/tls for zgrab2/tls and add parsing support for the tls handshake -> tlslog * add testing for lib/http to unit tests make target * add better http2 only tests * add docs for http 1/2 containers * extra caddy logging on the http2 file * got http2.0 scannign working....at the cost of removing the Dialers. We'll need to be able to handle both * added http2 library with imports for tls and http swapped to our forks * added a scratch folder with working http2 and http1 -> http2 upgraded minimal product transports as PoC's. * custom dialer with H2 transport * making http2 requests in zgrab...but without custom dialers * http2 working with custom dialers in zgrab * cleanup: remove debugging comments * remove debugging print msg * extract TLS handshake log with HTTP2 * clean up todos, remove scratch workspace * add default http2 and http1.1 nextprotos to TLS flags * remove todo and commented out function * fix merge mistake * working h2c test * lint issues in http and http2 fixed * formatting in http scanner * removed deprectaed dialer in ipp scanner * http2 prior knowledge working * made h2c test exclusive to http2, will error if it receives a http1 request * clean up h2c main.go * use http.Do to preserve the headers set by zgrab * remove logging code for debugging * lint issue * fixed up http tests * added http2 tests to makefile and got tests passing * automated http version tests * uncomment tests and set IP correctly * Hardcode request protocol field in HTTP request for http h2c. Gives more accurate info to user * lint * Update file paths in the http smoke test * update protocol structure in the request/response for http module in zschema * lint * remove default to h2 on TLS since other libraries aren't expecting those NextProtos. We'll default it in the http scanner itslef * lint * added back our redirect error handling to give the user back the full redirect page even if there weren't enough redirects to reach the page * remove http tests, don't pass in master * fix incorrect merge conflict resolution that kept redirects from being returned correctly * remove unused TLS config, missed removing it from testing * Remove http2-prior-knowledge and add no-http-2 and no-http1.1 flags * revise help text * use new flags for tests * added a few redirect endpoints on our docker files to add more tests * fixed up http tests * add manual test details and a note on the TLS/h2 redirect to non-TLS/http1.1 edge case * python lint * lint http_reading_test * suppress logs from http2, too chatty --------- Signed-off-by: Jeff Cody <jcody@censys.io> Co-authored-by: devStorm <59678453+developStorm@users.noreply.github.com> Co-authored-by: Zakir Durumeric <zakird@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
First, this prevents a DNS lookup from happening when we encounter a redirect, even if we don't intend to follow it. This likely addresses some part of #452
Second, if we aren't following redirects, don't have the scan fail in an 'application-error'. We are succeeding in what we intended to do, which is to scan without following redirects
How to Test
I tested by scanning targets that do redirects both with and without this change, using wireshark to monitor for DNS lookups and reviewing output to ensure that we are the same (within reason for randomized items like cookies and dates)