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 pkinit-anonymous command #774

Closed
wants to merge 1 commit into from

Conversation

stlaz
Copy link
Contributor

@stlaz stlaz commented May 10, 2017

Ever since from v4.5, FreeIPA expects at least some kind of
anonymous PKINIT to work. Deprecate the command which is
capable of turning this feature off.

https://pagure.io/freeipa/issue/6936

@stlaz stlaz force-pushed the pkinit_deprecate branch 2 times, most recently from 09bc1fe to 02e9b01 Compare May 10, 2017 14:49
@martbab martbab self-assigned this May 12, 2017
Copy link
Contributor

@martbab martbab left a comment

Choose a reason for hiding this comment

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

I have a few comments.

For more information on anonymous pkinit see:

http://k5wiki.kerberos.org/wiki/Projects/Anonymous_pkinit
This module is deprecated since FreeIPA 4.5.1
Copy link
Contributor

Choose a reason for hiding this comment

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

I would write that the whole module is deprecated (as there will probably be the pkinit-status command in this module, see #790. You can rather write that the pkinit-anonymous commands are deprecated and I will then re-write the module docstring in my PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok.


takes_args = (
Str('action', valid_arg),
Str('action?'),
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is action turned into a optional argument? is this required?

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 don't want users to be bothered by error messages that action is required or whatever is printed there should action not be provided, given that the output stays the same no matter what.

Copy link
Contributor

Choose a reason for hiding this comment

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

True if it is deprecated and no-op, then we should not bother users about required options. Do you agree @HonzaCholasta ?

Copy link
Contributor

Choose a reason for hiding this comment

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

We should remove the command completely, as it never really worked.

@abbra
Copy link
Contributor

abbra commented May 23, 2017

Just remove the command completely. FreeIPA prior to 4.5 never supported PKINIT operations and never allowed using anonymous PKINIT. Disabling/enabling it was left for admins that knew what they wanted. However, with FreeIPA 4.5 we require anonymous PKINIT to be enabled all time -- be it with a local self-signed cert or with some other certificate issued by a proper CA. An anonymous principal can only be used to create a FAST channel, nothing else.

Ever since from v4.5, FreeIPA expects at least some kind of
anonymous PKINIT to work. The pkinit-anonymous command was supposed
to enable/disable anonymous pkinit by locking/unlocking the
anonymous principal. We can't allow this for FreeIPA to work
so we are removing the command as it was never supported anyway.

https://pagure.io/freeipa/issue/6936
@stlaz
Copy link
Contributor Author

stlaz commented May 23, 2017

Thanks, pruned the file a bit.

@stlaz stlaz changed the title Deprecate pkinit-anonymous command Remove pkinit-anonymous command May 23, 2017
@martbab martbab added ack Pull Request approved, can be merged pushed Pull Request has already been pushed labels May 23, 2017
@martbab
Copy link
Contributor

martbab commented May 23, 2017

master:

  • 24099d0 Remove pkinit-anonymous command

@martbab martbab closed this May 23, 2017
@martbab
Copy link
Contributor

martbab commented May 23, 2017

ipa-4-5 needs a separate PR due to merge conflicts.

@stlaz stlaz deleted the pkinit_deprecate branch July 7, 2017 12:17
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
4 participants