Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature request: Implement Zmap's 'zcrypto/tls' libary as opposed to 'crypto/tls' to increase scanning versatility #1231

Closed
noskcajflor opened this issue Nov 8, 2021 · 4 comments · Fixed by #1529
Assignees
Labels
Priority: Medium This issue may be useful, and needs some attention. Status: Completed Nothing further to be done with this issue. Awaiting to be closed. Type: Enhancement Most issues will probably ask for additions or changes.
Milestone

Comments

@noskcajflor
Copy link

Please describe your feature request:

Requesting that Nuclei implement the 'zcrypto/tls' library developed by the Zmap project, instead of the current 'crypto/tls' library. 'Zcrypto/tls' makes use of larger pool of cipher suites than 'crypto/tls' and would enable more versatile scanning.

Describe the use case of this feature:

The default cipher suites used by golang's crypto/tls are fairly limited, and various hosts are likely to reject tcp handshakes with scanners that don't offer a large enough pool of cipher suites for client/server negotiation. Zcrypto/tls circumvents this issue by adding a significant number of additional cipher suites to 'cipher_suites.go' in GOPATH/src/crypto/tls/.

The process for adding a single cipher suite is fairly straightforward as done in this fork: phuslu/go@3533723

@noskcajflor noskcajflor added the Type: Enhancement Most issues will probably ask for additions or changes. label Nov 8, 2021
@ehsandeep ehsandeep added the Priority: Medium This issue may be useful, and needs some attention. label Nov 9, 2021
@ehsandeep
Copy link
Member

This applies to both HTTP and new SSL protocol support has been added here.

@forgedhallpass forgedhallpass added the Type: Discussion Some ideas need to be planned and disucssed to come to a strategy. label Nov 23, 2021
@Mzack9999 Mzack9999 self-assigned this Nov 26, 2021
@Mzack9999 Mzack9999 added the Status: In Progress This issue is being worked on, and has someone assigned. label Nov 26, 2021
@Mzack9999
Copy link
Member

This is the comparison between crypto/tls and https://github.com/zmap/zcrypto:

  • SSLV1 (not supported)
  • SSLV2 (not supported)
  • SSLV3 (crypto/tls: deprecated in go1.18, zmap/zcrypto: supported up to go 1.16)
  • TLS10 (crypto/tls: deprecated in go1.18, zmap/zcrypto: supported up to go 1.16)
  • TLS11 (crypto/tls: deprecated in go1.18, zmap/zcrypto: supported up to go 1.16)
  • TLS12 (crypto/tls: supported with safe ciphers, zmap/zcrypto: supported with safe+unsafe ciphers up to go 1.16)
  • TLS13 (crypto/tls: supported with cipher auto-selection, zmap/zcrypto: seems not supported yet)

https://github.com/zmap/zcrypto is not a ready drop-in replacement of crypto/tls and requires using their fork of net/http. A solution that integrates and works with upstream crypto/tls would be preferred. Marking it as blocked and researching a way to extend the standard library or quickly diff/patch without requiring a fork (monkey/hot patching? diff files?)

@Mzack9999 Mzack9999 removed the Status: In Progress This issue is being worked on, and has someone assigned. label Dec 3, 2021
@ehsandeep ehsandeep removed the Type: Discussion Some ideas need to be planned and disucssed to come to a strategy. label Dec 16, 2021
@Mzack9999 Mzack9999 added the Status: In Progress This issue is being worked on, and has someone assigned. label Jan 24, 2022
@Mzack9999 Mzack9999 linked a pull request Jan 24, 2022 that will close this issue
4 tasks
@Mzack9999 Mzack9999 added Status: Review Needed The issue has a PR attached to it which needs to be reviewed and removed Status: In Progress This issue is being worked on, and has someone assigned. labels Jan 27, 2022
@ehsandeep ehsandeep added Status: Completed Nothing further to be done with this issue. Awaiting to be closed. and removed Status: Review Needed The issue has a PR attached to it which needs to be reviewed labels Feb 5, 2022
@ehsandeep
Copy link
Member

@J-rolf this is now supported into dev version and implemented here #1529, zcrypto/tls is enabed with -ztls flag, as default crypto/tls is used as before.

@ehsandeep ehsandeep added this to the v2.6.0 milestone Feb 5, 2022
@noskcajflor
Copy link
Author

@ehsandeep Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Priority: Medium This issue may be useful, and needs some attention. Status: Completed Nothing further to be done with this issue. Awaiting to be closed. Type: Enhancement Most issues will probably ask for additions or changes.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants