-
Notifications
You must be signed in to change notification settings - Fork 40
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
crypto.createHash('md5') fails in a FIPS-enabled environment #272
Comments
This was referenced Oct 7, 2020
Closed
Per discussion in the fix PR, it looks like a simple swap from #!/usr/bin/env bash
################################################################################
# This script is a workaround for a bug in the `constructs` library which
# causes the operator to fail to boot in a FIPS cluster. It is intended to be
# used during the build process inside the docker container to patch the
# version of the constructs javascript library that gets packaged along with
# the python package.
#
# https://github.com/aws/constructs/issues/272
################################################################################
# Find the tarball for constructs
cd /usr/local/lib/python3.6/site-packages
constructs_tar=$(find . -name "constructs*.tgz")
# Patch the constructs library tarball
tar_lib=$(dirname "$constructs_tar")
tar_name=$(basename "$constructs_tar")
cd "$tar_lib"
mkdir tmp
cd tmp
tar -xzvf "../$tar_name"
pushd package/lib/private/
sed -i'' 's/md5/sha1/g' uniqueid.js
popd
tar -czvf "$tar_name" package
mv "$tar_name" ..
cd ..
rm -rf tmp
|
mergify bot
pushed a commit
that referenced
this issue
Oct 26, 2020
The `uniqueId` property has a few issues: 1. It uses MD5 which is not FIPS-compliant (fixes #272) 2. It has a variable length of up to 255 chars which may not be suitable in certain domains (e.g. k8s label names are limited to 63 characters). 3. The human-readable portion of `uniqueId` is sometimes confusing and in many cases does not offer sufficient value. 4. Its charset might be restrictive or too broad for certain domains. To that end, we introduce `node.addr` as an alternative to `uniqueId`. It returns a 42 character hexadecimal, lowercase and opaque address for a construct which is unique within the tree (app). The address is calculated using SHA-1 which is FIPS-compliant. For example: c83a2846e506bcc5f10682b564084bca2d275709ee Similar to `uniqueId`, `addr` intentionally skips any path components called `Default` to allow refactoring of construct trees which preserve names within the tree. *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Description
Our team is building an
operator
based oncdk8s
and one of our deployment targets is a FIPS-enabled kubernetes cluster. When runningcdk8s
(and by proxy theconstructs
library), we encounter the following error when attempting to to run asynth
:Root Cause
The root-cause of this bug is this line where
crypto.createHash('md5')
is invoked. According to the internet themd5
crypto hash algorithm is considered insecure by FIPS standards and is thus disabled in the underlyingopenssl
libraries when the operating system is running in FIPS mode.Repro
(In a FIPS-enabled cluster. Not sure how to easily reproduce this)
Proposed Solution
I believe that the point of the offending line is simply to compute a unique value for the given string
path
, so I think we can simply replace'md5'
with a FIPS-approved algorithm likesha1
orsha256
. I've tested both and they both produce valid hash values.Volunteering!
I'd be happy to try to fix this one and submit a PR
The text was updated successfully, but these errors were encountered: