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

Add warnings if s4u2proxy options are inconsistent #232

Merged
merged 1 commit into from
Sep 4, 2020

Conversation

simo5
Copy link
Contributor

@simo5 simo5 commented Sep 4, 2020

In most cases people configuring GssapiUseS4U2Proxy should really
set all three cred store options for keytab, client_keytab and ccache,
to isolate httpd from default system ccaches and keytabs.

Not doing so unintentionally, easily lead to very hard to debug issues
when trying to use the proxying feature.

NOt enforcing as a hard misconfiguration both for compatibility reasons
and also because there are corner cases where the configuration is
intentional.

Fixes #230

@simo5 simo5 requested a review from frozencemetery September 4, 2020 00:11
/* we check only once */
if (cfg->verified) return;

/* Check if cred store donfig is consistent with use_s4u2proxy.
Copy link
Member

Choose a reason for hiding this comment

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

Typo - "config"

In most cases, people configuring GssapiUseS4U2Proxy should really
set all three cred store options for keytab, client_keytab, and ccache
to isolate httpd from default system ccaches and keytabs.

Not doing so unintentionally easily leads to very hard to debug issues
when trying to use the proxying feature.

Not enforcing as a hard misconfiguration both for compatibility reasons
and also because there are corner cases where the configuration is
intentional.

Signed-off-by: Simo Sorce <simo@redhat.com>
[rharwood@redhat.com: typo fix and commit message cleanup]
Reviewed-by: Robbie Harwood <rharwood@redhat.com>
@frozencemetery frozencemetery merged commit b4b43c2 into gssapi:master Sep 4, 2020
@AdamWill
Copy link

Seems like this is broken, causing a crash in httpd during FreeIPA server deployment. https://bugzilla.redhat.com/show_bug.cgi?id=2121952 . I'm guessing it's not safe to assume cfg->cred_store->count will always be defined, or something like that.

@AdamWill
Copy link

One thing I notice, shouldn't this whole thing be inside a #ifdef HAVE_CRED_STORE ? Other things that use the credential store are. I don't think that's causing the Fedora bug, because HAVE_CRED_STORE should be defined there (our krb5 is of course new enough). But it seems like a bug still. I can't see what is causing the Fedora bug, so for a quick downstream fix I'm doing a build with the check commented out.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Warnings when the config can lead to known errors with delegation
3 participants