Skip to content

remove smaller DH modp's #3406

Description

@calvinmetcalf

based on the recent paper on new attacks against diffie-hellman it might be a good idea to remove 2 of the well known primes that node comes with from the defaults, namely modp1 and modp2 aka Oakley Group 1 and 2 which are mentioned specifically in the linked paper as being vulnerable to pre computation attacks with academic and state level resources respectively.

Activity

  1. mscdex commented on Oct 16, 2015

    @mscdex
    Contributor

    Couple of questions:

    1. Is this suggestion just for TLS or also for the crypto module?
    2. Just recently the default DH prime size for node TLS clients was set to 1024 (with the node TLS server rejecting anything less than 2048 by default), which is the same size as modp2. I guess this means we'd have to then bump this to 2048 or something? How would that affect existing TLS clients?
  2. added
    tlsIssues and PRs related to the tls subsystem.
    cryptoIssues and PRs related to the crypto subsystem.
    on Oct 16, 2015
  3. rvagg commented on Oct 16, 2015

    @rvagg
    Member

    Yeah, 1024 and 1536 are mentioned in the paper, so even though we've moved up to 1024 as a minimum we're still not at an ideal level, but thus is the compromise involved in choosing default parameters!

    I think it might be Java 7 based clients that are the biggest hurdle to moving beyond 1024 as a minimum and Java 7 is still in very wide use and not likely to disappear any time soon.

    This whole area is making me wonder if we shouldn't try a new approach to our crypto defaults. We are basically forced to select our settings and cipher suite based on lowest-common-denominator client capabilities while considering the whole Node.js ecosystem. However, many Node.js users are deploying to environments where their target clients are well above the minimum that we're considering. Therefore, perhaps we need to have at least two levels of crypto settings, a default and an ideal, maybe even an uber secure for those that can afford it (e.g. situations where they have full control over their clients and may even be doing Node.js <-> Node.js comms). I see two obvious ways of doing this:

    1. Documentation - create a new section in our documentation with details about what you can do to move beyond the default level of crypto by providing a custom cipher suite and other settings that harden your clients & servers.
    2. Options - create a simple option where you select the level of security you are opting for and that maps to our internal coded version of what we consider the best settings for that level of security are. Being able do something like tls.createServer({ key, cert, ca, securityLevel: 'ideal' }) would be fantastic. Beyond usability it has a benefit above documentation of letting the user trust us to update these settings as the landscape changes, so they don't need to keep on coming back to check what our latest recommendations are.

    /cc @nodejs/crypto

  4. calvinmetcalf commented on Oct 16, 2015

    @calvinmetcalf
    ContributorAuthor

    So this is more specifically about well known primes then small primes but
    the gist is more to remove the presets so those 2 specific ones can't be
    used by name.

    On Fri, Oct 16, 2015, 4:27 PM Brian White notifications@github.com wrote:

    Couple of questions:

    Is this suggestion just for TLS or also for the crypto module?
    2.

    Just recently
    f72e178
    the default DH prime size for node TLS clients was set to 1024 (with the
    node TLS server rejecting anything less than 2048 by default), which is the
    same size as modp2. I guess this means we'd have to then bump this to 2048
    or something? How would that affect existing TLS clients?

    —
    Reply to this email directly or view it on GitHub
    #3406 (comment).

  5. alexlamsl commented on Oct 20, 2015

    @alexlamsl

    Java 7 supports ECDHE, so I don't think it's fatal that it cannot handle DH keys > 1024 bits. Java 6 OTOH would be in trouble.

    I have tested that using Qualys SSL Server Test.

  6. bnoordhuis commented on Oct 20, 2015

    @bnoordhuis
    Member

    Do we know what other languages / frameworks do or are doing in the short term? My (admittedly limited) googling didn't really turn up anything.

    Removing the ability to use modp1 and modp2 by name seems reasonable at first glance. Alternatively, we can prefix them with insecure_ to get the message across but still leave people with an escape hatch?

    /cc @nodejs/crypto - you should chime in here.

  7. shigeki commented on Oct 22, 2015

    @shigeki
    Contributor

    Sorry to be late. I missed this.
    tls module does not use standard groups for DH key exchange and 5.0 is already limits its key size above 1024bits. The fix in LTS is in discussion of nodejs/Release#49. So we need not to care this in tls here.

    crypto module has a API of crypto.getDiffieHellman which uses these groups, however, it would be used not only on the wire but also in offline or internal network and my opinion is as the same of @rvagg 's comment of 1 to add documentation to notify users of caveats to use weak algorithms and key.
    This was discussed in nodejs/node-v0.x-archive#25564 and I will move the PR to here and resubmit it.

  8. indutny commented on Oct 22, 2015

    @indutny
    Member

    I'm +1 for prefix, or stderr warning on use.

  9. mscdex commented on Oct 22, 2015

    @mscdex
    Contributor

    FWIW modp1 is one of the groups required to be supported by ssh2 implementations, but so is modp14. I agree with @shigeki though, I think I'd prefer just a documentation change for now.

  10. jasnell commented on Apr 4, 2016

    @jasnell
    Member

    Is there reason to keep this open still?

  11. mscdex commented on Apr 4, 2016

    @mscdex
    Contributor

    @jasnell IMHO I don't think so, a documentation update was committed awhile back.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    cryptoIssues and PRs related to the crypto subsystem.securityIssues and PRs related to security.tlsIssues and PRs related to the tls subsystem.

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions