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

Ask interactively for user name and password if Kerberos credentials missing #7287

Draft
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

abbra
Copy link
Contributor

@abbra abbra commented Mar 25, 2024

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 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, service):
    if ccache is None:
        ccache = os.environ.get('KRB5CCNAME', 'KCM:')
    if principal is None:
        principal = username

    kinit_password(principal, password, ccache)

api.bootstrap(acquire_cred=(acquire_cred, api))
api.finalize()
api.Backend.rpcclient.connect()
print(api.Command.ping())

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:

	$ ipa ping
	Username: testuser
	Password: <some-password>
	--------------------------------------------
	IPA server version 4.11.1. API version 2.253
	--------------------------------------------

Initial implementation only supports password-based credential.

@abbra abbra added needs review Pull Request is waiting for a review ipa-4-11 Mark for backport to ipa 4.11 labels Mar 25, 2024
@tiran
Copy link
Member

tiran commented Mar 25, 2024

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 self.api instead?

class MultiProtocolTransport(Transport):
    """Transport that handles both XML-RPC and JSON"""
    def __init__(self, *args, **kwargs):
        Transport.__init__(self)
        self.protocol = kwargs.get('protocol', None)
        self.api = kwargs.get('api', api)

and pass the api object along later:

                proxy_kw['transport'] = transport_class(
                    protocol=self.protocol, service='HTTP', ccache=ccache,
                    api=self.api)

then you can replace the complicated checks with if self.api.env.acquire_cred is not None.

The single_request method does not pass service to _handle_exception().

I think the acquire_cred_interactively function won't work for users with 2FA, hardened, or OIDC login. It's not using a FAST tunnel.

How do you feel about a different set of parameters for the callback to make it future-proof?

  • require passing of keyword arguments instead of positional arguments
  • pass context and transport in case we need it in the future. We could drop principal, ccache, and service arguments. The information is available on context and transport.
  • require **kwargs so we can extend the API later without breaking code
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

ipalib/cli.py Outdated Show resolved Hide resolved
@abbra
Copy link
Contributor Author

abbra commented Mar 25, 2024

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.

@abbra abbra force-pushed the on-demand-kinit branch 2 times, most recently from d452f2d to 5400678 Compare March 25, 2024 09:49
@tiran
Copy link
Member

tiran commented Mar 25, 2024

One place does not pass service to _handle_exception:

freeipa/ipalib/rpc.py

Lines 736 to 737 in 64861a0

except gssapi.exceptions.GSSError as e:
self._handle_exception(e)

@abbra
Copy link
Contributor Author

abbra commented Mar 25, 2024

One place does not pass service to _handle_exception:

freeipa/ipalib/rpc.py

Lines 736 to 737 in 64861a0

except gssapi.exceptions.GSSError as e:
self._handle_exception(e)

Addressed this via a separate commit.

@abbra
Copy link
Contributor Author

abbra commented Mar 25, 2024

Filed a ticket to this specific change: https://pagure.io/freeipa/issue/9561

@abbra abbra added the re-run Trigger a new run of PR-CI label Mar 25, 2024
@freeipa-pr-ci freeipa-pr-ci removed the re-run Trigger a new run of PR-CI label Mar 25, 2024
@abbra abbra added the re-run Trigger a new run of PR-CI label Mar 25, 2024
@freeipa-pr-ci freeipa-pr-ci removed the re-run Trigger a new run of PR-CI label Mar 25, 2024
@rcritten
Copy link
Contributor

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.

Copy link
Member

@tiran tiran left a 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):
Copy link
Member

@tiran tiran Mar 27, 2024

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:

Suggested change
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)
>>>

Copy link
Contributor Author

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().

ipalib/rpc.py Outdated Show resolved Hide resolved
@abbra
Copy link
Contributor Author

abbra commented Apr 16, 2024

I am turning this back to draft mode. I have made some progress with interactive prompter to get rid of calling out to kinit, but haven't yet integrated it into the helpers for automatic use.

@f-trivino f-trivino removed the needs review Pull Request is waiting for a review label Apr 24, 2024
./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]>
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]>
@abbra abbra added the re-run Trigger a new run of PR-CI label Oct 28, 2024
@freeipa-pr-ci freeipa-pr-ci removed the re-run Trigger a new run of PR-CI label Oct 28, 2024
@abbra abbra added ipa-4-12 Mark for backport to ipa 4.12 re-run Trigger a new run of PR-CI and removed ipa-4-11 Mark for backport to ipa 4.11 labels Oct 29, 2024
@freeipa-pr-ci freeipa-pr-ci removed the re-run Trigger a new run of PR-CI label Oct 29, 2024
@abbra abbra added the re-run Trigger a new run of PR-CI label Oct 29, 2024
@freeipa-pr-ci freeipa-pr-ci removed the re-run Trigger a new run of PR-CI label Oct 29, 2024
@abbra abbra added the re-run Trigger a new run of PR-CI label Oct 29, 2024
@freeipa-pr-ci freeipa-pr-ci removed re-run Trigger a new run of PR-CI labels Oct 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ipa-4-12 Mark for backport to ipa 4.12
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants