Issue #8120 has been updated by Daniel Pittman.
I have taken a look at the code, and it generally looks reasonable enough. There are some small concerns that would need to be addressed if we were going to merge this, but my strong advise is that we do not: The change to SHA1 from MD5 in the SSL area is pretty trivial, and should be fine. We don't really have any great compatibility concerns there because the SSL libraries take care of handling multiple digests for us. Unfortunately, this patchset doesn't account for any compatibility on the Puppet side. It is still support for one, and only one, digest, and will fail hard if anything else is used. That means that you have to change the setting on every node in your network, all at once, to the same digest, or it will not work. It also fails if, say, you had a bunch of MD5 hashed files in a file bucket, then change the digest. The previous files are not upgraded or changed in any way, and there are no migration tools supplied. That makes it easy to lose files, and so break, eg, links between the Dashboard and the historic files that were inspected. Technically, it doesn't actually break that, because there is no change to the digest stuff in the Dashboard, which will now silently truncate (thanks, MySQL) or fail to import reports, but the older values will still refer to the older hash, and that would work, except that the FileBucket will reject a hash where the digest length doesn't match the new setting... I would happily advise people who needed to avoid MD5 digests right now to apply this, provided they were in a green-fields deployment, or could scrap their existing history in Puppet. I wouldn't put it into the main product repository without planning for the extra two to eight weeks designing and implementing compatibility into the changes, so that we don't have to upgrade every node, and so that we can migrate our older data (where possible) to the new content. (eg: support both old and new hash to access a file in the bucket, upgrade Dashboard and other API to handle multiple digest lengths, and to type them, handle compatibility fallbacks when agent and master have different digests configured, perhaps publish the digest configuration from the master to the agent, etc.) So, again, this is a good change from the PoV of just getting away from MD5, but a bad change in terms of our compatibility story at this point in time. It would serve as the heart of building the rest of the changes, but we shouldn't ship it until we are invested in doing that, because of the ability to blow your own foot off that it presently adds. ---------------------------------------- Feature #8120: Let user change hashing algorithm, to avoid crashing on FIPS-compliant hosts https://projects.puppetlabs.com/issues/8120 Author: jared jennings Status: Needs Decision Priority: Normal Assignee: Jason McKerr Category: security Target version: Affected Puppet version: Keywords: Branch: https://github.com/puppetlabs/puppet/pull/195 I'm using Puppet in part to ensure [Federal Information Processing Standard 140-2](http://csrc.nist.gov/publications/fips/fips140-2/fips1402.pdf) (FIPS 140-2) compliance on my network. Part of this compliance for the system underlying Puppet is to make sure that only [FIPS Approved](http://csrc.nist.gov/publications/fips/fips140-2/fips1402annexa.pdf) algorithms are used. OpenSSL does this by ensuring that any attempts to run an unapproved algorithm result in either a SIGSEGV or a SIGABRT. MD5 has been broken enough that it is no longer a FIPS Approved algorithm. The consequence for Puppet is that, if it tries to use MD5 on a FIPS-compliant system, it will crash. Here is where I have seen Puppet crash for this reason: 1. the puppet/util/checksums.rb, used by File resources; 2. the puppet/parser/functions/md5.rb, implementation of the md5 DSL function; 3. certificate signature in puppet/ssl/certificate_request.rb; 4. certificate fingerprinting in puppet/ssl/base.rb; 5. outside Puppet, in the session ID code in openssl/ssl-internal.rb, class OpenSSL::SSLServer, due to using WEBrick. It was easy enough to replace MD5 with SHA256 in all those places - and, in case 4, it appears I may not have needed to change the code; but the DSL function is still called md5, and MD5 is still named in some of the messages. My changes lack the refinement necessary to be useful to others. What I think I need is to be able to say, in one place like puppet.conf, "use SHA256, not MD5," and algorithms and messages alike will change. I think the `md5` DSL function would need to be replaced with a `digest` function which uses the configured algorithm, and there should also be a way in the DSL to find out which digest is being used, like a `digestname` function. Then, in some years when SHA2 is decertified, I can tell Puppet, "use SHA3, not SHA2," instead of filing an issue like this one and doing code changes. (I don't know what migration issues this scenario may pose.) [How can I make Red Hat Enterprise Linux 5 FIPS 140-2 compliant?](https://access.redhat.com/kb/docs/DOC-39230) (Red Hat login required) -- You have received this notification because you have either subscribed to it, or are involved in it. To change your notification preferences, please click here: http://projects.puppetlabs.com/my/account -- You received this message because you are subscribed to the Google Groups "Puppet Bugs" group. To post to this group, send email to [email protected]. To unsubscribe from this group, send email to [email protected]. For more options, visit this group at http://groups.google.com/group/puppet-bugs?hl=en.
