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

Make MD5 hash implementation configurable #1445

Merged
merged 1 commit into from
Jul 16, 2018
Merged

Conversation

rokek
Copy link
Contributor

@rokek rokek commented Jul 14, 2018

Overview

This aims to enable CouchDB to run in an environment where OpenSSL MD5 hash operations are explicitly disallowed by the operating system, for example: when running in "FIPS mode." Because CouchDB does not make use of MD5 hashes for cryptographic purposes, this workaround does not defeat the purpose of "FIPS mode," provided that the system owner is aware of and consents to its use.

Approach

  • Add a module that selects the library to use for MD5 hash operations depending on a configure flag. If the flag is used, the module's functions use erlang:md5. If not, the functions use crypto:hash(md5, ...).
  • Add to configure and rebar scripts to provide the configure flag and define an environment variable when it is used.
  • Replace crypto:hash(md5, ...) calls with calls to functions in the new module.

Testing recommendations

make check

Related Issues or Pull Requests

#1171

Checklist

@rnewson
Copy link
Member

rnewson commented Jul 16, 2018

What's the use case?

@nickva
Copy link
Contributor

nickva commented Jul 16, 2018

From what I understand it is to allow using CouchDB when FIPS mode is enabled. In that case computing md5 via OpenSSL (crypto:hash(md5, ...)) fails.

The other alternative is to switch CouchDB to use a different hash function, but that is a bigger change, so I suggested just using Erlang's built-in md5 for now as an option.

There was some discussion here about it: #1171

@rokek
Copy link
Contributor Author

rokek commented Jul 16, 2018

@rnewson Updated to include rationale, thank you for the reminder.

@nickva
Copy link
Contributor

nickva commented Jul 16, 2018

EUnit tests pass with and without the --erlang-md5 configure option.

+1

I'd suggest mentioning the issue number (#1171) in the commit message. Also rebase against latest master.

Then create a docs pull request to make note about the new option. Maybe under Runtime Errors section in https://github.com/apache/couchdb-documentation/blob/master/src/install/troubleshooting.rst. Make sure to mention there could be a performance penalty.

@rokek rokek force-pushed the md5-config branch 2 times, most recently from 781ca24 to abb8217 Compare July 16, 2018 19:34
@rokek rokek changed the title Make MD5 hash implementation configurable - issue #1171 Make MD5 hash implementation configurable Jul 16, 2018
@nickva
Copy link
Contributor

nickva commented Jul 16, 2018

@rokek no, don't need a new PR. Just switch to master, do a git pull to update it. Then switch back to the PR branch. Run git rebase master. Then update commit to mention issue number in it (git commit --amend). And at last git push --force to update the branch on Github.

Documentation PR is separate.

@rokek
Copy link
Contributor Author

rokek commented Jul 16, 2018

@nickva @rnewson Completed, thank you for the guidance, and let me know if there is more to do.

@nickva
Copy link
Contributor

nickva commented Jul 16, 2018

@rokek LGTM. Thanks for your contribution, it is much appreciated!

I'll merge it and will review the docs PR

@nickva nickva merged commit 3acf15f into apache:master Jul 16, 2018
@rokek
Copy link
Contributor Author

rokek commented Jul 16, 2018

@nickva Thank you for the assistance!

@wohali
Copy link
Member

wohali commented Jul 17, 2018

@nickva @rokek sadly this broke the build, see #1450 for details.

willholley added a commit that referenced this pull request Dec 2, 2019
#1445 introduced a shim to
enable CouchDB to be compiled to use the Erlang MD5 function.
This allows CouchDB to run in FIPS environments where the crypto
module is restricted such that `crypto:hash(md5,..)` is blocked
(fails with `notsup` error).

This commit replaces usage of `crypto:hash(md5, ..)` introduced since
the original PR with the shim function.
willholley added a commit that referenced this pull request Dec 2, 2019
#1445 introduced a shim to
enable CouchDB to be compiled to use the Erlang MD5 function.
This allows CouchDB to run in FIPS environments where the crypto
module is restricted such that `crypto:hash(md5,..)` is blocked
(fails with `notsup` error).

This commit replaces usage of `crypto:hash(md5, ..)` introduced since
the original PR with the shim function.
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 this pull request may close these issues.

4 participants