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

Hostbased initiator name support #1154

Merged
merged 2 commits into from Jan 28, 2021
Merged

Conversation

greghudson
Copy link
Member

Having tried a couple of approaches to this problem, I'm putting this up to keep a record of my design notes.

Hostbased initiator names are not currently supported very well; even without dns_canonicalize_hostname=fallback, they fail if there is no [domain_realm] entry supplying a realm name. But apparently FreeIPA uses them, and there is no conceptual reason why they shouldn't work on a best-effort basis.

gic_keytab does not support wildcarding like krb5_rd_req() does. At this time I think that's inherent to the nature of the initiator: we need to pick a concrete principal to authenticate as, and there might be multiple matches for a wildcard in the keytab or cache collection. By contrast, the acceptor has the ticket encryption key to use as an authority. For this reason, I think it makes sense that a GSS host-based name like "host" can only match the local host, and if the realm can't be determined with [domain_realm] then we can only match the default realm.

The current implementation approach is to rewrite cred->name->princ after we match a ccache in the collection or acquire creds from the client keytab. This is much simpler than adding a new field to the cred (we create credentials and use cred->name->princ in many places), but unfortunately it creates a corner case for GSS_C_BOTH credentials, because cred->name->princ->data[1] is now post-canonicalization but kg_rd_req() thinks it is pre-canonicalization.

The mere possibility of this situation is a result of the krb5_sname_to_principal() fallback design. When we see a host-based krb5_principal, we currently have to deduce from the call graph whether it's pre-canonical or post-canonical. Heimdal uses a fake name type (KRB5_NT_SRV_HST_NEEDS_CANON) to make this explicit, but that leaves open the possibility of the fake name type leaking out onto the wire.

I currently think the safe solution is to create a new (internal) "principal description" type which can contain a service, hostname, iteration state, and (once known) canonical principal name. (For non-hostbased names it would just contain a fixed canonical principal name.) I will investigate that approach.

The PR currently reuses reuses krb5_kt_have_match() to match against the client keytab when checking whether we can acquire initial creds. This is a bit overly permissive because that function uses wildcarding rules and obeys ignore_acceptor_hostname. Being a little too permissive isn't necessarily a big problem, though.

@frozencemetery
Copy link
Contributor

frozencemetery commented Jan 19, 2021

Hostbased initiator names are not currently supported very well; even without dns_canonicalize_hostname=fallback, they fail if there is no [domain_realm] entry supplying a realm name. But apparently FreeIPA uses them, and there is no conceptual reason why they shouldn't work on a best-effort basis.

It's not clear to me what a better option would be. Hardcoding the realm is odd because it should be possible to be realm-agnostic in the code (even if at runtime it's determined). And while GSS_KRB5_NT_PRINCIPAL_NAME gets around the issue, it's Kerberos-specific.

@greghudson
Copy link
Member Author

It's not clear to me what a better option would be.

I couldn't find the FreeIPA code in question, so I don't understand the use case. But I think most initiator code passes GSS_C_NO_NAME for desired_name and lets the mech decide.

@frozencemetery
Copy link
Contributor

Huh, I didn't realize that worked. I suppose that speaks to your point about how uncommon this is.

I have sent a fix to freeipa for the code in question here: freeipa/freeipa#5452

@greghudson
Copy link
Member Author

I think there's been a miscommunication.

I think GSS initiator code frequently passes GSS_C_NO_CREDENTIAL for claimant_cred_handle to gss_init_sec_context(). Or, if it needs a credential for whatever reason, it passes GSS_C_NO_NAME for desired_name to gss_acquire_cred().

The linked PR seems to instead create a name, but passes GSS_C_NO_OID for input_name_type to gss_import_name(). That does work for importing a principal name (or other mech default name type) but isn't related to what I said.

@frozencemetery
Copy link
Contributor

Ah, okay. Well, that explains why it was strange at least :)

@greghudson greghudson force-pushed the hostbased-init branch 4 times, most recently from e45283e to 96ba42f Compare January 22, 2021 22:51
@greghudson greghudson changed the title WIP: hostbased initiator name support Hostbased initiator name support Jan 22, 2021
@greghudson
Copy link
Member Author

I ironed out the acceptor name issue by generating the acceptor matching princ early and storing it. I fixed a couple of other bugs and wrote a proper commit message. I think this can be merged pending review.

Copy link
Contributor

@frozencemetery frozencemetery left a comment

Choose a reason for hiding this comment

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

Looks (and tested) good.

To facilitate fallback tests, add a canonicalize_hostname() function
to k5test.py which works similarly to krb5_expand_hostname().  Use it
in t_gssapi.py for the recently-added acceptor name fallback test.
When checking if we can get initial credentials in the GSS krb5 mech,
use krb5_kt_have_match() to support fallback iteration.  When scanning
the ccache or getting initial credentials, rewrite cred->name->princ
to the canonical client name.  When a name check is necessary (such as
when the caller specifies both a name and ccache), use a new internal
API k5_sname_compare() to support fallback iteration.  Add fallback
iteration to krb5_cc_cache_match() to allow host-based names to be
canonicalized against the cache collection.

Create and store the matching principal for acceptor names in
acquire_accept_cred() so that it isn't affected by changes in
cred->name->princ during acquire_init_cred().

ticket: 8978 (new)
@greghudson greghudson merged commit c374ab4 into krb5:master Jan 28, 2021
@greghudson greghudson deleted the hostbased-init branch January 28, 2021 23:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants