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

Remove getenv(OPENSSL_FIPS) in openssl command #11995

Closed

Conversation

@bernd-edlinger
Copy link
Member

@bernd-edlinger bernd-edlinger commented May 30, 2020

This is left over from the past.

Checklist
  • documentation is added or updated
  • tests are added or updated
This is left over from the past.
@bernd-edlinger
Copy link
Member Author

@bernd-edlinger bernd-edlinger commented May 30, 2020

My concern about this code, is that it might be re-used for the 3.0 FIPS provider,
and then in consequence be set in environment variables, which will then
1.1.1 fail unexpectedly.

Copy link
Contributor

@paulidale paulidale left a comment

The configuration file specifies FIPS mode now. It's reasonable for this to go.

At the least the message should be changed to say that the environment variable is not supported, use the configuration file instead.

@paulidale
Copy link
Contributor

@paulidale paulidale commented May 31, 2020

I'm unsure about 1.1.1 for this.

@bernd-edlinger
Copy link
Member Author

@bernd-edlinger bernd-edlinger commented May 31, 2020

Why, this is no documented behavior, and it plays a prank on the user.

@levitte
Copy link
Member

@levitte levitte commented May 31, 2020

This environment variable was most relevant with the FIPS OpenSSL Module 2.0 (compatible with OpenSSL 1.0.2). The reason we have the error is for users that have upgrade to the 1.1.x series must not be lead to believe that OPENSSL_FIPS still has the meaning they might think.

So I see nothing against removing this in 3.0, we have moved far enough from 1.0.2 to simply drop the check. For 1.1.1, I'm just as dubious as @paulidale.

@bernd-edlinger
Copy link
Member Author

@bernd-edlinger bernd-edlinger commented May 31, 2020

Okay, then I'll add this to my 1.1.1 feature branch #11900

@openssl-machine
Copy link

@openssl-machine openssl-machine commented Jun 1, 2020

This pull request is ready to merge

openssl-machine pushed a commit that referenced this pull request Jun 1, 2020
This is left over from the past.

Reviewed-by: Paul Dale <paul.dale@oracle.com>
(Merged from #11995)
@bernd-edlinger
Copy link
Member Author

@bernd-edlinger bernd-edlinger commented Jun 1, 2020

Merged to master as 32df134

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

Successfully merging this pull request may close these issues.

None yet

4 participants