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

Jump to bottom

Fix anonymous principal handling in replica install #666

Closed
wants to merge 2 commits into from
Closed

Fix anonymous principal handling in replica install #666

wants to merge 2 commits into from

Conversation

martbab
Copy link
Contributor

@martbab martbab commented Mar 28, 2017 •
edited by MartinBasti

This PR should unblock replica install against <4.5 masters if --no-pkinit
option is given. Be aware of the non-working WebUI after install, this will be
fixed once local PKINIT will be implemented.

Requires #631

@stlaz stlaz self-assigned this Mar 29, 2017
stlaz
stlaz reviewed Mar 29, 2017
View reviewed changes
ipaserver/install/server/upgrade.py Outdated
krb.setup_pkinit()
if not os.path.exists(paths.KDC_CERT):
root_logger.info("Requesting PKINIT certificate")
krb.setup_pkinit()
Copy link
Contributor

Choose a reason for hiding this comment

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

You should probably stop the execution here in case PKINIT is already set up.

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 could perhaps add the outcome of the offline discussion here, please :)

@stlaz
Copy link
Contributor

stlaz commented Mar 29, 2017

I actually did the review of #631 alongside this.
I do not think the order of adding the anonymous principal and setting up PKINIT matters that much. From what I saw in Kerberos guides, it's usually actually done after PKINIT setup since until then, the anonymous principal is pretty much unusable.
The problem was rather the testing of anonymous pkinit before the anonymous principal was added, that is just plainly weird and I'm glad that that's now fixed.
ACK since this fixes the issues mentioned in comments.

@stlaz stlaz added the ack Pull Request approved, can be merged label Mar 29, 2017
@MartinBasti
Copy link
Contributor

needs rebase

@MartinBasti MartinBasti reopened this Mar 30, 2017
Martin Babinsky added 2 commits March 30, 2017 15:02
Always check and create anonymous principal during KDC install
ec8727e 
The anonymous principal will now be checked for presence and created on
both server and replica install. This fixes errors caused during replica
installation against older master that do not have anonymous principal
present.

https://pagure.io/freeipa/issue/6799
Remove duplicate functionality in upgrade
26bb2d6 
Since krbinstance code can now handle all operations of the
`enabled_anonymous_principal` function from upgrade we can remove
extraneous function altogether.

https://pagure.io/freeipa/issue/6799
@martbab martbab force-pushed the pkinit-replica-add-anon-princ branch from e82000f to 26bb2d6 Compare March 30, 2017 13:03
@MartinBasti
Copy link
Contributor

master:

  • 191668e Always check and create anonymous principal during KDC install
  • 2eabb0d Remove duplicate functionality in upgrade

ipa-4-5:

  • ce94f7f Always check and create anonymous principal during KDC install
  • 0fcd565 Remove duplicate functionality in upgrade

@MartinBasti MartinBasti added the pushed Pull Request has already been pushed label Mar 30, 2017
@martbab martbab deleted the pkinit-replica-add-anon-princ branch April 3, 2017 10:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@stlaz stlaz stlaz left review comments

Assignees

@stlaz stlaz

Labels
ack Pull Request approved, can be merged pushed Pull Request has already been pushed
Projects
None yet
Milestone
No milestone
3 participants
@martbab @stlaz @MartinBasti