-
Notifications
You must be signed in to change notification settings - Fork 344
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
Ask interactively for user name and password if Kerberos credentials missing #7287
base: master
Are you sure you want to change the base?
Conversation
I find it confusing how the API object is passed around. How about you include the api in the transport instance and get the callback from
and pass the api object along later:
then you can replace the complicated checks with The I think the How do you feel about a different set of parameters for the callback to make it future-proof?
import threading
import typing
from ipalib import plugable
from ipalib.rpc import MultiProtocolTransport
def acquire_cred_cb(
*,
api: plugable.API,
transport: MultiProtocolTransport,
context: threading.local,
principal: typing.Optional[str] = None,
ccache: typing.Optional[str] = None,
service=typing.Optional[str] = None,
**kwargs
) -> None:
"""Acquire credential callback
:param api: ipalib API object
:param transport: RPC transport instance
:param context: threading local context with additional data
:param principal: Kerberos principal name (str, None)
:param ccache: credential cache with ccache type (str, None)
:param service: service service prefix (e.g. 'HTTP' or None)
"""
pass |
Please note that I explicitly point in the commit message that current implementation will not work for passwordless methods and that is intended -- I am working on an addition to support those but they'll come in a separate PR. This one lays a foundation to enable callbacks that abstract out credentials acquisition at the time when this change happens. |
d452f2d
to
5400678
Compare
One place does not pass Lines 736 to 737 in 64861a0
|
Addressed this via a separate commit. |
Filed a ticket to this specific change: https://pagure.io/freeipa/issue/9561 |
This is a pretty big change from previous behavior. I wonder if in order to retain previous behavior without having to always pass in -n a new config option be added. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see another potential problem with api.env.interactive
. It's always set to True
unless ipa
CLI is called with ipa --no-prompt
. This may cause issues with scenarios like systemd services or Ansible playbooks that run ipa
non-interactively. I haven't tested it, but I think that the new code is going to block on stdin read indefinitely.
Suggestions:
For password prompt, only use the interactive flag if stdin is a tty:
interactive = (self.api.env.interactive and sys.stdin.isatty()) or self.api.env.prompt_all
then pass interactive
to the acquire_cred
callback. This would let each callback implementation detect if interaction with a user is possible. The reference implementation of acquire_cred_interactively
does nothing, but a callback with a password from a config file would work.
ipalib/cli.py
Outdated
@@ -1454,10 +1454,48 @@ def format_description(self, description): | |||
) | |||
|
|||
|
|||
def acquire_cred_interactively(*args, **kw): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The function should never receive positional arguments and always the keyword arguments api, principal, and ccache, I recommend this syntax:
def acquire_cred_interactively(*args, **kw): | |
def acquire_cred_interactively(*, api, principal, ccache, **kwargs): |
the *
without argument name means that all following args must be passed as keywords.
>>> def acquire_cred_interactively(*, api, principal, ccache, **kwargs): pass
...
>>> acquire_cred_interactively(1, 2, 3)
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
TypeError: acquire_cred_interactively() takes 0 positional arguments but 3 were given
>>> acquire_cred_interactively(api=1, principal=2, ccache=3)
>>>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, makes sense. It is a nice feature.
I updated the code and also decided to pass in an exception class to raise in case the callback decides it cannot acquire credentials. This helps against loops within self.get_auth_info()
: self._handle_exception()
-> self._try_acquire_cred()
-> raise errors.NonFatalError()
-> self.get_auth_info()
.
I am turning this back to draft mode. I have made some progress with interactive prompter to get rid of calling out to |
./makeapi:443: SyntaxWarning: invalid escape sequence '\(' m = re.match('^[a-zA-Z0-9]+\(\'([a-z][_a-z0-9?\*\+]*)\'.*', line) Changing just one line will cause surrounding lines to be affected by the Pylint checks. Changing those lines will expose more code to the checks. In the end, I had to reformat the whole script. Signed-off-by: Alexander Bokovoy <[email protected]>
Signed-off-by: Alexander Bokovoy <[email protected]>
This allows applications to specify their own acquire_cred callback. The callback should be put into the environment under 'acquire_cred' name. The callback would need to adhere to the following signature: def acquire_cred_cb( *, api: plugable.API, transport: MultiProtocolTransport, context: threading.local, principal: typing.Optional[str] = None, ccache: typing.Optional[str] = None, service: typing.Optional[str] = None, interactive: bool, exc: Exception, **kwargs ) -> None: """Acquire credential callback :param api: ipalib API object :param transport: RPC transport instance :param context: threading local context with additional data :param principal: Kerberos principal name (str, None) :param ccache: credential cache with ccache type (str, None) :param service: GSSAPI service name (e.g. '[email protected]' or None) :param interactive: true if API is used interactively over TTY :param exc: exception to raise in case callback couldn't acquire creds """ pass and ideally should acquire a Kerberos credential into the ccache. Both principal and the ccache name might be None, signifying that a default name and a default credentials cache would need to be used. Example usage: ----------------------------------------------- import os from ipalib import api from ipalib.install.kinit import kinit_password username = 'admin' password = '<some-value>' def acquire_cred(*, api, principal, ccache, exc, **kw): ccache = ccache if ccache else os.environ.get('KRB5CCNAME', 'KCM:') principal = principal if principal else username try: kinit_password(principal, password, ccache, canonicalize=True, enterprise=True) except: raise exc() api.bootstrap(acquire_cred=acquire_cred) api.finalize() api.Backend.rpcclient.connect() print(api.Command.ping()) ---------------------------------------------- Fixes: https://pagure.io/freeipa/issue/9561 Signed-off-by: Alexander Bokovoy <[email protected]>
Ask interactively for credentials in case they are missing in the default credentials cache in the environment: $ kdestroy -A $ ipa ping Username[testuser]: admin Password: <some-password> -------------------------------------------- IPA server version 4.11.1. API version 2.253 -------------------------------------------- Only ask for the credentials if we are using a TTY device. This means a pipe-provided standard input would not trigger ask for credentials: $ kdestroy -A $ echo -n | ipa ping ipa: ERROR: did not receive Kerberos credentials Initial implementation only supports password-based credential. Fixes: https://pagure.io/freeipa/issue/9561 Signed-off-by: Alexander Bokovoy <[email protected]>
When we encounter GSSAPI exception, it means we are either in a process of negotiating authentication with a remote service or we just started acquiring a ticket. In either case, we should know GSSAPI service name: either it was passed to us by get_auth_info() directly or can infer it from the opened connection. Fixes: https://pagure.io/freeipa/issue/9561 Signed-off-by: Alexander Bokovoy <[email protected]>
In order to add bindings to more MIT Kerberos functions, move existing ones to a separate file. Session code uses few of the Kerberos functions to operate on the credential caches. Signed-off-by: Alexander Bokovoy <[email protected]>
Initializing credentials with passwordless pre-authentication methods requires two major elements: - use of FAST channel for exposing passwordless methods - use of pre-authentication method prompts Communicating with kinit command line utility to provide prompts is awkward. Instead, expose MIT Kerberos API for prompting. This commit adds initial kinit_with_cb() implementation that allows to invoke an externally provided callback as a prompter. A default callback implementation will be provided later. Signed-off-by: Alexander Bokovoy <[email protected]>
Add initial implementation of the interactive prompter to be used with ipapython.kerberos.kinit_with_cb(). The prompter can work with all preauthentication methods FreeIPA supports that require prompts: - password-based ones (timestamp and SPAKE) - OTP/RADIUS - passkey - external IdP Signed-off-by: Alexander Bokovoy <[email protected]>
9815c9c
to
d2b2751
Compare
cli: add acquire_cred callback support to the Kerberos transport
This allows applications to specify their own acquire_cred callback. The callback should be put into the environment under '
acquire_cred
' name as a tuple of a(callable, pass-in object)
.The callable should accept
(pass-in object, principal, ccache, service)
and ideally should acquire a Kerberos credential into theccache
. Bothprincipal
and theccache
name might be None, signifying that a default name and a default credentials cache would need to be used.Example usage:
cli: Ask for user name and password if Kerberos credentials missing
Ask interactively for credentials in case they are missing in the default credentials cache in the environment:
Initial implementation only supports password-based credential.