Thanks for this good work! I like the idea of specific checks for the 3 SSLv2-related vulnerabilities, but I have 2 concerns with including them in the sslv2 script directly:

1. Users are much less likely to use a feature of a script that requires using a script-arg to activate than if they can just add another term or wildcard to their --script option.
2. Removing the "safe" category will prevent many users who currently restrict themselves to "safe" scripts from detecting SSLv2 at all.

I would like to see how this PR would look with the following changes:

• Only simple enhancements to sslv2.nse if you have discovered problems with how it currently works.
• A new script, sslv2-drown, for checking the forced-cipher and clear-key vulnerabilities. The regular sslv2 script could store discovered ciphers in the host registry, with sslv2-drown using dependencies to indicate that sslv2 should run first if it is selected, in order to avoid repeating the basic handshake if possible.
• Use the vulns NSE library for reporting vulnerability info.

@bbc2 Be sure to let us know when you think this is ready for review again.

OK! It's not ready yet.

@dmiller-nmap It's ready for review. Thank you for your help!

I've hopefully addressed all your concerns. I don't know what is the best practice for NSE libraries but we could win a lot on code deduplication if the two scripts shared common SSLv2 structures and functions.

Working on integrating this. Partially applied changes to sslv2.nse, along with some of my own improvements. Expect to have your DROWN script and possibly a sslv2 library committed in the next few days. Thanks so much for this great work!

@bbc2 You list these ciphers as "weak enough to enable" DROWN:

-- Those ciphers are weak enough to enable a "General DROWN" attack.
local GENERAL_DROWN_CIPHERS = {
  [SSL_CK.RC2_128_CBC_EXPORT40_WITH_MD5] = true,
  [SSL_CK.RC4_128_EXPORT40_WITH_MD5] = true,
  [SSL_CK.DES_64_CBC_WITH_MD5] = true,
}

Why these and not SSL2_RC4_64_WITH_MD5? It also has an encrypted-key length of 8 (64 bits) like SSL2_DES_64_CBC_WITH_MD5.

On the other hand, the DROWN paper only mentions EXPORT ciphers (40 bits) as being vulnerable, so should SSL2_DES_64_CBC_WITH_MD5 be removed from the list?

I took the list of weak enough ciphers from an early version of the DROWN paper which listed them explicitly. In the published version it's less obvious but please see the last paragraph of section 4.1 (The SSLv2 export padding oracle):

If the server does not support 40-bit export ciphers, the attack can also be mounted in feasible computation time by choosing DES as the symmetric cipher.

Even though DES requires 64-bit keys, it only uses 56 bits from them, making it a lot weaker against bruteforce than other ciphers with 64-bit keys.

Thanks for the review and the integration, it's awesome!