Skip to content
This repository has been archived by the owner on Dec 17, 2021. It is now read-only.

pshtt scan exception case drops record from report #117

Closed
egyptiankarim opened this issue Mar 28, 2017 · 3 comments
Closed

pshtt scan exception case drops record from report #117

egyptiankarim opened this issue Mar 28, 2017 · 3 comments

Comments

@egyptiankarim
Copy link

When the version of pshtt that is loaded by domain-scan lands into an ConnectionError or a RequestException exception case in its basic_check method, it dumps an error and returns an empty string where a JSON object representing the domain's test results should be. This ripples through sslyze and errors out of the domain-scan pshtt invocation altogether (i.e., "Bad news scanning"), and never writes an entry to the results. Here's an example (domain redacted; hit me up for a live example):

› docker-compose run scan domain.tld --scan=pshtt --debug --force
[domain.tld][pshtt]
	 /opt/pyenv/versions/2.7.11/bin/pshtt domain.tld
Failed to connect.
Certificate did not match expected hostname: domain.tld. Certificate: {[certificate chain information]}
Traceback (most recent call last):
  File "/opt/pyenv/versions/2.7.11/bin/pshtt", line 9, in <module>
    load_entry_point('pshtt==0.1.5', 'console_scripts', 'pshtt')()
  File "/opt/pyenv/versions/2.7.11/lib/python2.7/site-packages/pshtt/cli.py", line 54, in main
    results = pshtt.inspect_domains(domains, options)
  File "/opt/pyenv/versions/2.7.11/lib/python2.7/site-packages/pshtt/pshtt.py", line 882, in inspect_domains
    results.append(inspect(domain))
  File "/opt/pyenv/versions/2.7.11/lib/python2.7/site-packages/pshtt/pshtt.py", line 63, in inspect
    basic_check(domain.https)
  File "/opt/pyenv/versions/2.7.11/lib/python2.7/site-packages/pshtt/pshtt.py", line 173, in basic_check
    https_check(endpoint)
  File "/opt/pyenv/versions/2.7.11/lib/python2.7/site-packages/pshtt/pshtt.py", line 331, in https_check
    cert_plugin_result = cert_plugin.process_task(server_info, 'certinfo_basic')
  File "/opt/pyenv/versions/2.7.11/lib/python2.7/site-packages/sslyze/plugins/certificate_info_plugin.py", line 115, in process_task
    if scan_command.custom_ca_file:
AttributeError: 'str' object has no attribute 'custom_ca_file'
Error running eval "$(pyenv init -)" && pyenv shell 2.7.11 && /opt/pyenv/versions/2.7.11/bin/pshtt domain.tld --json --user-agent "github.com/18f/domain-scan, pshtt.py" --timeout 30 --preload-cache ./cache/preload-list.json.
	Bad news scanning, sorry!
Results written to CSV.

That last bit is a lie... Nothing is written because the raw data out of the pshtt invocation is None.

This ultimately is an issue with the way exception cases in pshtt are ordered in the overall flow of logic, and I'll open up an issue there and eventually offer a solution. The reason I'm bringing it up here, is because pshtt will produce a report with these exception cases properly reflected (i.e., as failing), but domain-scan just ends up dropping them from the report all together, which can be confusing as anything when your target list of 12K domains only results in a results.csv of 11,999 rows (gah!). So this is more just an "awareness" issue.

@konklone
Copy link
Contributor

@egyptiankarim Want to email me a live example so I can test? (eric.mill@gsa.gov)

The reason I'm bringing it up here, is because pshtt will produce a report with these exception cases properly reflected (i.e., as failing), but domain-scan just ends up dropping them from the report all together, which can be confusing as anything when your target list of 12K domains only results in a results.csv of 11,999 rows (gah!).

Totally get this. What do you think should be in the 12,000th row (the row representing the failed run) for such a scan?

@egyptiankarim
Copy link
Author

Want to email me a live example so I can test?

@konklone Will do. Email on its way.

What do you think should be in the 12,000th row (the row representing the failed run) for such a scan?

Well, just generally speaking, I think domain-scan ought to mimic whatever a regular run of the underlying scanner would give you. For the exception cases in question here, a regular pshtt scan eventually gives us a "Live" True with a failure on "Valid HTTPS" and related attributes, which is a fair enough glimpse of the situation.

I think there's probably room for improvement on how we handle RequestExceptions (especially weird code 500 and redirect loop situations), and I'll puzzle over it, but for now I know that I get some result back running a regular pshtt scan, so whatever domain-scan gives me should approximate that rather than just dropping it.

Also, I'll emphasize again that I think the "fix" for all of this is in revisiting the logic in pshtt, and I'll be working a pull request there. I just wanted to post an issue on domain-scan so that people would be aware of the gap in reports produced by each.

@konklone
Copy link
Contributor

@egyptiankarim I'm closing this because I think it's made moot on the domain-scan side by the refactor in #155.

Try using the --meta flag to capture local errors in scan output, and let me know if you're still seeing issues with this.

You may also want to try the Lambda pipeline, which definitely returns stack traces when errors are observed.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants