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

Add FIPS-token password of HTTPD NSS database #450

Closed
wants to merge 1 commit into from

Conversation

stlaz
Copy link
Contributor

@stlaz stlaz commented Feb 8, 2017

This change is required for httpd to function properly in FIPS

https://fedorahosted.org/freeipa/ticket/5695

@@ -504,7 +504,10 @@ def create_password_conf(self):
f = open(self.pwd_conf, "w")
f.write("internal:")
pwdfile = open(self.passwd_fname)
f.write(pwdfile.read())
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you are touching this, you can change thsi code to use with statement

f.write(password)
# make sure other processes can access the file contents ASAP
f.flush()
if os.path.exists(self.pwd_conf):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do you need to check presence of file that is actually created a few lines above?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, I was doing something else and left some mess there.

@rcritten
Copy link
Contributor

I guess this is one approach to fix the problem. Would it be cleaner to pass in, or detect, FIPS mode, and only write out the token that will actually be used?

@stlaz
Copy link
Contributor Author

stlaz commented Feb 10, 2017

That was my original approach to it but we had offline talk with @HonzaCholasta and got to the point that it might be better to do it this way.
From my point of view it's more fool-proof for the people who would install FreeIPA in non-FIPS mode but then thought it'd be cool to turn FIPS on. Anyone reading this in the future - that is NOT SUPPORTED. There would probably be more different issues, let this not be one.

f.write("internal:")
f.write(password)
f.write("\nNSS FIPS 140-2 Certificate DB:")
f.write(password)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No \n here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did not run into any issues with it not being there. It was not there when there was only the internal token so I assume there's no need for it here as well.

@HonzaCholasta
Copy link
Contributor

LGTM. I guess we don't have to bother with upgrade, given that you can turn on FIPS post-install, right?

@stlaz
Copy link
Contributor Author

stlaz commented Feb 15, 2017

You shouldn't turn FIPS on post-install (is what I think you mean), correct.

This change is required for httpd to function properly in FIPS

https://fedorahosted.org/freeipa/ticket/5695
@tkrizek tkrizek added the ack Pull Request approved, can be merged label Feb 15, 2017
@MartinBasti
Copy link
Contributor

@MartinBasti MartinBasti added the pushed Pull Request has already been pushed label Feb 15, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ack Pull Request approved, can be merged pushed Pull Request has already been pushed
Projects
None yet
5 participants