This repository has been archived by the owner on Dec 17, 2021. It is now read-only.
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.
Add support for client authentication #286
Add support for client authentication #286
Changes from 19 commits
c35efea
324054b
0f516cf
99f5eb9
03866d5
8eb43b8
c89e956
05b3083
cf81846
652d6a2
95aa0f5
333d23e
a8b66d1
d6e1970
133a5fc
80d27f1
aeb980d
10458ca
07be179
050e26b
860392e
15bd499
4240b17
369f91b
c676313
af596de
cf8a291
0848d11
e513aa7
b812928
2019622
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These two blocks could be simplified to:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, that is better.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This could be simplified to:
data['certs']['expired_certificate'] = not(leaf.not_valid_before < now < leaf.not_valid_after)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This could simply be:
if reneg.accepts_client_renegotiation and not reneg.supports_secure_renegotiation:
And then further, the whole block could be:
data['config']['insecure_renegotiation'] = reneg.accepts_client_renegotiation and not reneg.supports_secure_renegotiation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FWIW, I think with the logging calls, it might actually be preferred to use formatting similar to:
logging.debug("\t\t%s scan.", scan_type)
Which matches more what is done in the documentation, where the parameters (
scan_type
in this case) are passed tologging.debug
as arguments, rather than doing the substitution first: https://docs.python.org/3/library/logging.html#logging.Logger.debugThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not at all opposed to this change, but I think we should make it wholesale, across all scanners, in a separate pull request.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this be
logging.error(...)
?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This can be simplified to:
sslv2 = sslv3 = tlsv1 = tlsv1_1 = tlsv1_2 = tlsv1_3 = certs = reneg = None
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should just be
if options.get("sslyze-reneg", True):
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🦅 👁️