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

Add parser function digest, that calculates a checksum using the configured `digest_algorithm` #2453

Closed
wants to merge 5 commits into
base: master
from

Conversation

Projects
None yet
3 participants
@jaredjennings
Copy link
Contributor

jaredjennings commented Mar 19, 2014

This patch adds the digest function to the Puppet language. The md5 function exists in Puppet presently, but fails on FIPS 140-2 compliant systems. digest uses whatever digest algorithm puppet.conf says should be used for files to calculate a checksum.

Just as with #2452, migration concerns are not dealt with: if the digest_algorithm setting changes from md5 to sha256, and the manifest specifies that the digest of something should be equal to some literal MD5 checksum, it will suddenly be unequal. But if the manifest were to compare the digests of two things instead, that would continue to work seamlessly across transitions, where comparing the md5s would begin to cause errors when FIPS 140-2 compliance is enforced.

The changes in #2451 and #2452 are included in this pull request; the only new thing here is the last change, "Add parser function digest."

@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 Apr 4, 2014

Until now, fix/master/digest-parser-function-20140319 consisted of all the same commits as fix/master/configurable-digest-algorithm-20140319, plus the one commit that adds the digest parser function. But now I have changed fix/master/configurable-digest-algorithm-20140319 according to some of the comments you made on commits in fix/master/digest-parser-function-20140319 (see #2452), so the histories are no longer common.

@jaredjennings

This comment has been minimized.

Copy link
Contributor

jaredjennings commented Apr 4, 2014

I have rebased fix/master/digest-parser-function-20140319, the branch which is the subject of this pull request, on master.

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.
(PUP-1840) Add parser function digest: uses digest_algorithm to hash,…
… not strictly md5

Puppet has an md5 parser function, which returns the MD5 digest of its
argument. On hosts configured for compliance with U.S. Federal
Information Processing Standard (FIPS) 140-2, attempts to use the MD5
algorithm cause errors, because MD5 is no longer FIPS Approved. This
patch adds a parser function called digest, which returns the digest
of its argument using the algorithm named by the digest_algorithm
setting in puppet.conf. Therefore, where md5 may fail on some hosts,
the digest function should always return a value; but the value may
vary if the digest_algorithm setting is changed.
@joshcooper

This comment has been minimized.

Copy link
Member

joshcooper commented Apr 23, 2014

@jaredjennings thanks for your contribution, we haven't forgotten about this. We are still working on resolving #2537. After that we can rebase this and merge

@joshcooper

This comment has been minimized.

Copy link
Member

joshcooper commented Apr 30, 2014

@jaredjennings we've merged in #2537, could you rebase this PR as many of the commits should no longer be needed. Thanks!

@jaredjennings

This comment has been minimized.

Copy link
Contributor

jaredjennings commented May 1, 2014

Rather than deal with all the conflicts between my old configurable digest algorithm code and what #2537 resulted in, I've made a new branch and a new pull request, #2603; accordingly I close this one.

@jaredjennings jaredjennings deleted the jaredjennings:fix/master/digest-parser-function-20140319 branch May 1, 2014

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