Skip to content
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

Consider using SHA-256 over MD5 for the config file #56

Closed
silverwind opened this issue Jun 8, 2020 · 5 comments · Fixed by #57
Closed

Consider using SHA-256 over MD5 for the config file #56

silverwind opened this issue Jun 8, 2020 · 5 comments · Fixed by #57

Comments

@silverwind
Copy link
Contributor

/~https://github.com/gulpjs/v8flags/blob/master/index.js#L19 contains a MD5 usage. When Node.js is built with FIPS compliance, that line triggers below error because MD5 is disabled in those builds:

Error: error:060800A3:digital envelope routines:EVP_DigestInit_ex:disabled for fips
    at new Hash (internal/crypto/hash.js:48:19)
    at Object.createHash (crypto.js:109:10)
    at Object.<anonymous> (node_modules/v8flags/index.js:19:89)

Consider using something more collision-resistant like SHA-256 instead.

@phated
Copy link
Member

phated commented Jun 10, 2020

@silverwind I'm down for that. I also assume our cacheing is fragile here because we are supposed to continue even if a cache error occurs, but that's a different problem.

@silverwind
Copy link
Contributor Author

It should be a simple swap of createHash('md5') to createHash('sha256'). SHA-256 hashes are a bit longer but you could just trim it down to the same size of MD5 if necessary using .substring(0,32) or similar.

@yocontra
Copy link
Member

yocontra commented Jun 11, 2020

This seems like a reasonable request - want to send a PR @silverwind ? Should be non-breaking.

silverwind added a commit to silverwind/v8flags that referenced this issue Jun 13, 2020
Fixed a maximum line length eslint warning on this line as well.

Fixes: gulpjs#56
silverwind added a commit to silverwind/v8flags that referenced this issue Jun 13, 2020
Fixed a maximum line length eslint warning on this line as well. This
change doubles the length of the file name from a previous 32 hex
characters to 64.

Fixes: gulpjs#56
@silverwind
Copy link
Contributor Author

#57

@tgflet
Copy link

tgflet commented Feb 18, 2021

Bump, I could really use this fix in a project. It is breaking the project when enforcing FIPS when enforcing FIPS because MD5 is not FIPS compliant @silverwind @phated

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants