FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

crypto: enable FIPS only when configured with it by indutny · Pull Request #3153 · nodejs/node · GitHub

/ node Public

crypto: enable FIPS only when configured with it - #3153

Closed
indutny wants to merge 1 commit into
nodejs:masterfrom
indutny:fix/gh-3077
ConversationCommits (1)ChecksFiles changed
Closed

crypto: enable FIPS only when configured with it#3153
indutny wants to merge 1 commit into
nodejs:masterfrom
indutny:fix/gh-3077

Conversation

indutny commented Oct 1, 2015

Copy link
Copy Markdown
Member

Do not rely on OPENSSL_FIPS in node_crypto.cc when building with
shared FIPS-enabled OpenSSL library. Enable FIPS in core only when
configured with --openssl-fips.

Fix: #3077

cc @nodejs/crypto

crypto: enable FIPS only when configured with it
69510a7 
Do not rely on `OPENSSL_FIPS` in `node_crypto.cc` when building with
shared FIPS-enabled OpenSSL library. Enable FIPS in core only when
configured with `--openssl-fips`.

Fix: nodejs#3077
indutny mentioned this pull request Oct 1, 2015
Closed

bnoordhuis commented Oct 1, 2015

Copy link
Copy Markdown
Member

LGTM

indutny commented Oct 1, 2015

Copy link
Copy Markdown
Member Author

Thanks, @bnoordhuis. I'm going to skip CI run, because we are not testing FIPS mode anyway.

indutny commented Oct 1, 2015

Copy link
Copy Markdown
Member Author

Landed in 9bd26e7

indutny closed this Oct 1, 2015
indutny deleted the fix/gh-3077 branch October 1, 2015 21:41
indutny added a commit that referenced this pull request Oct 1, 2015
crypto: enable FIPS only when configured with it
9bd26e7 
Do not rely on `OPENSSL_FIPS` in `node_crypto.cc` when building with
shared FIPS-enabled OpenSSL library. Enable FIPS in core only when
configured with `--openssl-fips`.

Fix: #3077
PR-URL: #3153
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>

rvagg commented Oct 2, 2015

Copy link
Copy Markdown
Member

I'm going to skip CI run, because we are not testing FIPS mode anyway.

@indutny don't discount weird side-effects that you can't predict, like Windows oddities with .gyp files or compiler differences. CI runs are cheap and for this kind of thing you could just look at a few test runs and verify that builds have happened successfully elsewhere to have confidence so you don't even need to wait for the whole thing if you're impatient.

indutny commented Oct 2, 2015

Copy link
Copy Markdown
Member Author

@rvagg ok, I will do it next time if you insist. I just always try to spare the CI, and save some time for other people :)

indutny added a commit that referenced this pull request Oct 2, 2015
crypto: enable FIPS only when configured with it
f055a66 
Do not rely on `OPENSSL_FIPS` in `node_crypto.cc` when building with
shared FIPS-enabled OpenSSL library. Enable FIPS in core only when
configured with `--openssl-fips`.

Fix: #3077
PR-URL: #3153
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
rvagg mentioned this pull request Oct 3, 2015
Closed
aliscco mentioned this pull request Nov 29, 2023
Open
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

Back | FazBrowse Home | New Git URL