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 Kerberos ApReq Authenticator Checksum Flags #207

Closed

Conversation

TheBestTvarynka
Copy link
Collaborator

@TheBestTvarynka TheBestTvarynka commented Feb 9, 2024

Hi,
In this pull request, I fixed the issue mentioned here: #206 (comment)

During the issue investigation, I noticed that authentication works with all flags except the GSS_C_DELEG_FLAG one. It didn't seem related to the smart card authentication stuff. I tested the authentication with password-based logon and got the same error. So it's not related to the login type.

Then I reread the RFC that contains this flag explanation. RFC 4121: Section 4.1.1: Authenticator Checksum:

The length of the checksum field MUST be at least 24 octets when GSS_C_DELEG_FLAG is not set,
and at least 28 octets plus Dlgth octets when GSS_C_DELEG_FLAG is set.

Our authenticator checksum always has the 24 octets len (

pub const AUTHENTICATOR_DEFAULT_CHECKSUM: [u8; 24] = [
and
inner: AUTHENTICATOR_DEFAULT_CHECKSUM.to_vec(),
) and we don't support any checksum extensions yet. But the RFC requires at least 28 octets when the GSS_C_DELEG_FLAG is set. On the other hand, the same RFC says:

When delegation is used, a ticket-granting ticket will be transferred in a KRB_CRED message.

In our current Kerberos implementation, we don't support the KRB_CRED message. So, my solution is to disable the GSS_C_DELEG_FLAG flag. I added a comment in the code with an explanation about this decision.

Docs & references:

@TheBestTvarynka TheBestTvarynka self-assigned this Feb 9, 2024
@CBenoit
Copy link
Member

CBenoit commented Feb 9, 2024

Glad to see you found the issue! 🎉

For this patch, do you think you could target master instead? Looks like this is a bug we always had, it would be nice to fix it immediately (and we can rebase the smartcard dev branch on top of master afterwards)

@TheBestTvarynka
Copy link
Collaborator Author

No problem. I'll open a new pull request for it because this branch is based on the winscard-rs-impl

@TheBestTvarynka TheBestTvarynka marked this pull request as ready for review February 9, 2024 12:26
@TheBestTvarynka
Copy link
Collaborator Author

do you think you could target master instead?

done: #208

@CBenoit CBenoit deleted the fix/kerberos-authenticator-flags branch March 7, 2024 09:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants