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

Make the digest algorithm used for file checksums configurable #2452

Closed
wants to merge 4 commits into
base: master
from

Conversation

Projects
None yet
3 participants
@jaredjennings
Copy link
Contributor

jaredjennings commented Mar 19, 2014

These changes add a digest_algorithm setting, settable in puppet.ini, and use the algorithm indicated by it to perform checksums for file resources and file bucket operations. The digest_algorithm setting defaults to md5 so that if it is not set, Puppet will still act the same as it always has.

Nothing clever is done in these changes to deal with what happens if Puppet was checksumming files using one algorithm, and you change the digest_algorithm setting, i.e. migration concerns.

Note that the patch to add the SHA-256 checksum type, from #2451, is included in this pull request. It is a prerequisite for the configurable digest algorithm changes, because it adds the Puppet::Util::Checksums#known_checksum_types method.

@puppetcla

This comment has been minimized.

Copy link

puppetcla commented Mar 19, 2014

CLA signed by all contributors.

@jaredjennings

This comment has been minimized.

Copy link
Contributor

jaredjennings commented Mar 20, 2014

The Travis CI failure appears to be the same failure that @peterhuene said in #2451 could likely be safely ignored.

@jaredjennings

This comment has been minimized.

Copy link
Contributor

jaredjennings commented Apr 4, 2014

I rebased the fix/master/configurable-digest-algorithm-20140319 branch on the latest HEAD of the master, and moved the spec helper code out of spec_helper.rb, and force-pushed the (now different) changes up here, on the advice of http://codeinthehole.com/writing/pull-requests-and-other-good-practices-for-teams-using-github/ . Comments made on the old commits can be seen at jaredjennings@94bca18 and parents.

jaredjennings added some commits Mar 18, 2014

(PUP-1840) Add a digest_algorithm Puppet setting, defaulting to md5
Without this patch applied, Puppet only performs checksums of files
using the MD5 algorithm. MD5 is not available on hosts configured for
FIPS 140-2 compliance, so Puppet fails. This patch adds a setting,
settable in puppet.conf, whose name is digest_algorithm and whose
value can be any checksum type known to Puppet::Util::Checksums. The
setting indicates which digest algorithm should be used for performing
checksums. (This patch does not contain the code that pays attention
to the setting.) The value defaults to md5, so that if you do not set
this setting yourself, Puppet will act like it always has before.
(PUP-1840) Ease testing under different digest_algorithm settings
Without this patch, spec tests for pieces of the code having to do
with checksums would have to be duplicated twice and contain repeated
code.

The patch adds the `using_checksums_describe` and `using_checksums_it`
methods, which can be used instead of the `describe` and `it` methods
to enclose spec tests where checksums are involved. The methods are
only defined inside describe blocks where the `:uses_checksums =>
true` user metadata is given, to mitigate the risk of unintended
changes to the behavior of spec tests not having to do with checksums.

`using_checksums_it` defines a group of examples, each having the
given block as body. Each example happens with a different value set
for `digest_algorithm`. Many `let`s are defined for use inside the
spec test; foremost are `plaintext`, `checksum`, and
`algo`. `plaintext` is some text; `checksum` is the checksum for this
text under the present setting of `digest_algorithm`. `algo` contains
the name of the digest algorithm in use; if `digest_algorithm` is
unset, `algo` is 'md5'.
(PUP-1840) Use configured digest_algorithm as default checksum_type f…
…or file resources

Without this patch, file resources will always try to checksum
themselves using MD5. On FIPS 140-2 compliant hosts, this will
fail. This patch adds sha256 as a permissible value for the File
resource's checksum parameter, and makes the checksum parameter
default to using the digest_algorithm, as set in the puppet.conf.
(PUP-1840) Use configured digest_algorithm for file bucketing and fet…
…ching

Without this patch the filebucket works using MD5, which may fail on
FIPS 140-2 compliant hosts. This patch makes the filebucket use the
digest algorithm indicated by the digest_algorithm setting in
puppet.conf.
@adrienthebo

This comment has been minimized.

Copy link
Contributor

adrienthebo commented Apr 15, 2014

I've refactored this code in GH-2537 to remove duplication and make the tests a bit more clear to read. Please check it over and make sure this still captures all the changes that you wanted to see made in this pull request. In the mean time I'm going to close this pull request in favor of GH-2537; let me know if this is in error. Thanks!

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