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

Fix s4u2self with adtrust #709

Closed
wants to merge 1 commit into from
Closed

Fix s4u2self with adtrust #709

wants to merge 1 commit into from

Conversation

simo5
Copy link
Contributor

@simo5 simo5 commented Apr 11, 2017

When ADtrust is installed we add a PAC to all tickets, during protocol
transition we need to generate a new PAC for the requested user ticket,
not check the existing PAC on the requestor ticket.

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

When ADtrust is installed we add a PAC to all tickets, during protocol
transition we need to generate a new PAC for the requested user ticket,
not check the existing PAC on the requestor ticket.

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

Signed-off-by: Simo Sorce <simo@redhat.com>
@flo-renaud
Copy link
Contributor

Hi @simo5,

I tested webUI authentication with a IPA user and it is working with this patch.

/* we need to create a PAC if we are requested one and this is an AS REQ,
* or we are doing protocol transition (s4u2self) */
if ((is_as_req && (flags & KRB5_KDB_FLAG_INCLUDE_PAC)) ||
(flags & KRB5_KDB_FLAG_PROTOCOL_TRANSITION)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think if should be (flags & KRB5_KDB_FLAGS_S4U) because both protocol transition and constraint delegation are in KRB5_KDB_FLAGS_S4U and fetch_kdb_authdata() does check against this constant when looking if it is included.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No this would force a rebuild of the PAC in s4u2proxy cases.
In that case we check and forward port the PAC or create it. And this case is already handled in pre-existing code.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ok, the other case (constraint delegation) of KRB5_KDB_FLAGS_S4U flag is handled already.

Copy link
Contributor

@abbra abbra left a comment

Choose a reason for hiding this comment

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

Looks good to me if you address KRB5_KDB_FLAGS_S4U flag question.

Copy link
Contributor

@abbra abbra left a comment

Choose a reason for hiding this comment

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

LGTM.

/* we need to create a PAC if we are requested one and this is an AS REQ,
* or we are doing protocol transition (s4u2self) */
if ((is_as_req && (flags & KRB5_KDB_FLAG_INCLUDE_PAC)) ||
(flags & KRB5_KDB_FLAG_PROTOCOL_TRANSITION)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Ok, the other case (constraint delegation) of KRB5_KDB_FLAGS_S4U flag is handled already.

@abbra abbra added the ack Pull Request approved, can be merged label Apr 11, 2017
@abbra abbra self-assigned this Apr 11, 2017
@pvomacka
Copy link

ipa-4-5:

  • b511407 Fix s4u2self with adtrust
    master:

  • e88d5e8 Fix s4u2self with adtrust

@pvomacka pvomacka added the pushed Pull Request has already been pushed label Apr 12, 2017
@pvomacka pvomacka closed this Apr 12, 2017
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