The Puppet Labs Issue Tracker has Moved: https://tickets.puppetlabs.com

This issue tracker is now in read-only archive mode and automatic ticket export has been disabled. Redmine users will need to create a new JIRA account to file tickets using https://tickets.puppetlabs.com. See the following page for information on filing tickets with JIRA:

Bug #6663

puppet.conf says keylength defaults to 1024 -- should be 2048

Added by micah - about 5 years ago. Updated about 4 years ago.

Status:ClosedStart date:03/10/2011
Priority:HighDue date:
Assignee:-% Done:

0%

Category:SSL
Target version:2.7.12
Affected Puppet version: Branch:https://github.com/puppetlabs/puppet/pull/498
Keywords:

We've Moved!

Ticket tracking is now hosted in JIRA: https://tickets.puppetlabs.com


Description

puppet.conf(5) says that the keylength parameter defaults to 1024 bits for new RSA keys.

It should default to 2048, not 1024, there are a number of reasons for this:

  • many free software crypto tools are defaulting to 2048-bit keys now (e.g. OpenSSH, GnuPG)

  • NIST has recommended avoiding reliance on 1024-bit keys after the end of 2010

  • you can compare other comparable standards at http://keylength.com/

Considering that generated certificates are expected to be around for at least the lifetime of the server itself, setting a reasonable bit-length key from the beginning is pretty important, especially if the server might be expected to be around for some years from now…

You might argue that this is a feature request, but I would like to pre-empt that argument. Now that we are well beyond the NIST recommendation, this is a bug now days.


Related issues

Related to Puppet - Feature #5529: Allow configuration of SSL ciphers Accepted 12/13/2010
Related to Puppet - Feature #8120: Let user change hashing algorithm, to avoid crashing on F... Code Insufficient 06/28/2011

History

#1 Updated by Ben Hughes about 5 years ago

  • Status changed from Unreviewed to Investigating
  • Affected Puppet version deleted (2.6.6rc1)

While not a fix as you ask for. You can add keylength = however-big-you-want in the “main” stanza in your puppet.conf, though of course, this will not help with the current certificates you have, not deploying that chance to new hosts.

#2 Updated by micah - about 5 years ago

Not only is the default keylength for the CA 1024 bits, the default hash is MD5.

The german BSI1 produces a yearly document2 that defines which algorithms should be save for usage over the next five years. This document rules out MD5, SHA-1 and RIPEMD-160 for hashing and key sizes < 1976 bits for RSA keys right now.

Please update the default settings to something save for the time of the default TTL (five years).

#3 Updated by Nigel Kersten about 5 years ago

Micah, can you start a community thread on this?

I’m in agreement, but am worried we’re not considering some real world factors if we end up mixing key-lengths and hash formats, especially on older distributions that we need to support.

#4 Updated by Nigel Kersten almost 5 years ago

ping?

#5 Updated by micah - almost 5 years ago

  • Assignee set to micah -

Nigel Kersten wrote:

ping?

oh hi.

To be honest, I dont understand the workflow here. The last time I subscribed was to start a community thread about #5604, which I did do, and some people responded, but I dont understand how this flows back into the issue itself. I’m sure this is somehow my fault, but I dont really understand what I’m supposed to do and now that issue, annoying as it is for debian users, is stagnating and I’m not sure what to do about it. I’m a little hesitant to end up causing that with this issue, becuase I’m not sure how I should handle it.

also, just a side note, please assign tickets to me if you want me to respond :)

#6 Updated by Nigel Kersten almost 5 years ago

micah – wrote:

Nigel Kersten wrote:

ping?

oh hi.

To be honest, I dont understand the workflow here. The last time I subscribed was to start a community thread about #5604, which I did do, and some people responded, but I dont understand how this flows back into the issue itself. I’m sure this is somehow my fault, but I dont really understand what I’m supposed to do and now that issue, annoying as it is for debian users, is stagnating and I’m not sure what to do about it. I’m a little hesitant to end up causing that with this issue, becuase I’m not sure how I should handle it.

I’ll follow that up now.

It is difficult, and I welcome suggestions for how to improve it.

Sometimes I just don’t feel like I have enough information about the potential impact of a change on our community, and it’s good to at least seek feedback, as I don’t want to be too prescriptive when I’m lacking insight.

I’ve been behind on ticket gardening, but usually I follow up from the thread and update the ticket.

also, just a side note, please assign tickets to me if you want me to respond :)

erk. sorry.

#7 Updated by Anonymous almost 5 years ago

I am strongly of the view that we should follow the most restrictive of the current sets of government advice (eg: BSI, NSA/NIST, etc) and advice from the experts in the field. If this requires addressing the question of how to achieve compatibility then we had better solve this, before someone genuinely breaks MD5, or RSA, or whatever in a way that matters to us, and we end up in more serious trouble: having to solve this in zero time, rather than with the relatively luxury of time.

Larger keys, better hashing (probably by adding them as well as md5, rather than just replacing it, etc.)

(Oh, and we absolutely have the capabilities to inspect the client version and make intelligent decisions about what we ship in terms of checksums, etc, as part of our compatibility story. As long as the master leads the agent in version we should be fine.)

#8 Updated by Mark Stanislav almost 5 years ago

There will of course be a trade-off for security versus performance, which is why being reasonable about strength used should be also considered. 2048 bit RSA keys are ‘good’ until ~2030 at this time (according to NIST). Considering a default CA cert is five years for Puppet, this is a very reasonable way to go. There shouldn’t be a compatibility issue to solve unless there’s some interesting crypto voodoo occurring in Puppet ;)

I really don’t know of any reason to implement MD5 at all. It is broken and we do have better algorithms to implement. Even if SHA-1 is on its last leg, it’s still a step-up. SHA-256 is preferred, though.

Again, a great discussion to be having and very forward thinking.

#9 Updated by Nigel Kersten almost 5 years ago

  • Status changed from Investigating to Accepted
  • Assignee deleted (micah -)
  • Priority changed from Normal to Urgent
  • Target version set to 2.7.x

Micah, we’re going to do this. I’ll respond to the thread.

#10 Updated by James Turnbull over 4 years ago

  • Status changed from Accepted to In Topic Branch Pending Review
  • Branch set to https://github.com/jamtur01/puppet/commit/b929aa19330bab42421190e60099ac2406f975c3

Sent pull request:

https://github.com/puppetlabs/puppet/pull/69

#11 Updated by Josh Cooper over 4 years ago

  • Status changed from In Topic Branch Pending Review to Code Insufficient

Hi James, from the puppet-dev discussion http://groups.google.com/group/puppet-dev/browse_frm/thread/415f43b03d328efd/c897d70e0185e23d:

Telly must support RHEL4, as PE supports RHEL4.

And:

RHEL4, which has a version of OpenSSL old enough that SHA256 wasn't supported upstream. Given that we know the Ruby OpenSSL bindings to fail, rather than anything else, if you ask for an algorithm that doesn't exist in the underlying libraries

I don’t believe the code as it currently exists can be merged. However, we should be able to programmatically check which digests the underlying openssl provider supports and select appropriate defaults, e.g.

:ca_md => [ defined?(OpenSSL::Digest::SHA256) ? "sha256" : "md5", "The type of hash used in certificates."],

Before making such a change though we should verify that calling defined?(OpenSSL::Digest::SHA256) on an older RHEL4 box doesn’t segfault.

#12 Updated by James Turnbull over 4 years ago

  • Status changed from Code Insufficient to Needs Decision
  • Assignee set to Jason McKerr

Jason – as my patch is a no-go I’ll need to hand this over to Eng.

#13 Updated by Jason McKerr over 4 years ago

  • Assignee changed from Jason McKerr to Josh Cooper

OK, let’s go ahead on the fix that Josh recommends. Josh, feel free to kick this over to Partrick or Jacob unless you think you can fix it quick.

#14 Updated by Josh Cooper over 4 years ago

  • Status changed from Needs Decision to Accepted
  • Assignee changed from Josh Cooper to Anonymous

Seeing as how it’s been a month and I haven’t gotten anywhere with this, I am reassigning to the platform team.

#15 Updated by Anonymous over 4 years ago

  • Assignee deleted (Anonymous)
  • Priority changed from Urgent to Normal

Thanks.

#16 Updated by Patrick Mohr over 4 years ago

I’m curious why the priority on this was dropped. People have said advantages for it, but I have yet to hear a problem. This is also security related, and leaving this in, is a tacit statement from Puppet Labs that 1024 bits is safe enough about 6 years from now. (That’s 5 years for the cert to expire, and 1 year for the patch to get written, released, and sent downstream to the faster updating repositories)

Do we really want to recommend users keep using this 8 years (2012+6 years) after the NIST recommends we stop, considering that the ability to force a cert will basically invalidate the whole puppet security model?[1] I don’t see this as critical, but I also think, this should be urgent.

[1] Impersonating any computer, as a client will usually give you access to sensitive information, and allow fact injection that might cause problems other security problems. It also allows anyone who can hijack DNS or do a man-in-the-middle attack to gain root access to most puppet clients.

#17 Updated by Nigel Kersten over 4 years ago

  • Priority changed from Normal to High

micah – wrote:

Not only is the default keylength for the CA 1024 bits, the default hash is MD5.

Is this actually true in 2.7.x? I only see SHA1.

#18 Updated by Nigel Kersten over 4 years ago

Patrick Mohr wrote:

I’m curious why the priority on this was dropped. People have said advantages for it, but I have yet to hear a problem.

It’s not normal priority, you’re right.

We’re looking to solve this without breaking backwards compatibility on platforms we still support.

#19 Updated by Anonymous about 4 years ago

  • Branch changed from https://github.com/jamtur01/puppet/commit/b929aa19330bab42421190e60099ac2406f975c3 to https://github.com/puppetlabs/puppet/pull/498

https://github.com/puppetlabs/puppet/pull/498 raises the default key size to 4096 bits for client and CA certificates. This should give us a default that will be secure until the algorithm is broken, rather than anything else, which seems a good investment for people who don’t know why they should turn down the key length only within sane limits, in return for the performance.

It doesn’t address any of the checksum changes, which we have substantial compatibility issues around since they are passed without reference to type in most of the code, and where we can’t just drop older clients. The ticket is remaining open as a consequence.

#20 Updated by Anonymous about 4 years ago

  • Status changed from Accepted to In Topic Branch Pending Review

Daniel Pittman wrote:

It doesn’t address any of the checksum changes, which we have substantial compatibility issues around since they are passed without reference to type in most of the code, and where we can’t just drop older clients. The ticket is remaining open as a consequence.

Scratch that; #8120 already captures the hash issue, so this can close off and that ticket take over that role. If you also care about the problem of using the MD5 hash, please watch that ticket also.

#21 Updated by Matthaus Owens about 4 years ago

  • Status changed from In Topic Branch Pending Review to Merged - Pending Release
  • Target version changed from 2.7.x to 2.7.12

#22 Updated by Matthaus Owens about 4 years ago

  • Status changed from Merged - Pending Release to Closed

Released in puppet 2.7.12rc1

Also available in: Atom PDF