Use bodyTextLen instead of readLen for FailHTTPToHTTPS logic #345
Conversation
This file contains 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
respContentLength can be -1 in certain cases, in which case readLen will be maxReadLen for the current scan. This will, however, then cause the FailHTTPToHTTPS if-condition to fail as the readLen is > 1024, even though the body content length can be in this range. So let us use the actual body length for the check to avoid this issue.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Replace readLen condition with bodyTextLen for the FailHTTPToHTTPS logic.
How to Test
Setup a Docker Server with TLS, run zgrab against it with FailHTTPToHTTPS & RetryHTTPS enabled. In my case, zgrab will still return "success" with HTTP 400 and "Client sent an HTTP request to an HTTPS server.\n" as the body with the current master. With this fix, zgrab will actually retry HTTPS and get the requested content from the Docker API.
Notes & Caveats
This fixes a bug where the FailHTTPToHTTPS logic would not trigger when the Go's HTTP library cannot determine the correct ContentLength (resp.ContentLength is -1), which causes readLen to be maxReadLen, which is 262144 by default.
This is way above the 1024 bytes hard limit in the if condition which contains the FailHTTPToHTTPS logic, thus it is never entered even if the body is actually short with one of the expected message and falls inside this limit.
Issue Tracking
Have not created an issue for this, as the fix is easy.