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

support service principal with slash #105

Merged
merged 1 commit into from
Apr 21, 2018
Merged

Conversation

ah-
Copy link
Contributor

@ah- ah- commented Apr 20, 2018

Hi,

we're using gokrb in vault-plugin-auth-kerberos to authenticate users, and noticed that we were getting unexpected "Matching key not found in keytab" errors when service principals contained slashes/multiple components, e.g. "HTTP/full-hostname.keytab@EXAMPLE.COM".

There is more detail in wintoncode/vault-plugin-auth-kerberos#1

It turns out that this is because the the sa is represented differently, in the keytab it's a list of components (["HTTP", "full-hostname.keytab@EXAMPLE.COM"]), in ValidateAPREQ it's a single string "HTTP/full-hostname.keytab@EXAMPLE.COM".
DecryptEncPart in https://github.com/jcmturner/gokrb5/blob/master/messages/Ticket.go#L191 currently simply converts the single string into a list containing that single string: ["HTTP/full-hostname.keytab@EXAMPLE.COM"].

Therefore when GetEncryptionKey is called, the lengths of both lists don't match and it can't find the key in the keytab.

The reason that the version in the keytab is split up is the keytab format and parsing here: https://github.com/jcmturner/gokrb5/blob/master/keytab/keytab.go#L246

The proposed solution is to simply split up the string into it's components.

All the work for this was done by @kristian-lesko in https://github.com/wintoncode/vault-plugin-auth-kerberos/pull/2/files, thanks a lot Kristian!

@ah-
Copy link
Contributor Author

ah- commented Apr 20, 2018

The CI failure seems to be a formatting issue affecting only go master: https://travis-ci.org/jcmturner/gokrb5/jobs/369170616

$ test -z $(gofmt -s -l $GO_FILES)
The command "test -z $(gofmt -s -l $GO_FILES)" exited with 1.

@jcmturner jcmturner changed the title Support service principal with slash support service principal with slash Apr 21, 2018
@jcmturner
Copy link
Owner

relates to #108

@jcmturner
Copy link
Owner

Thanks very much for your contribution!

I have fixed the gofmt testing. Looks like later versions of Go have a fix for some formatting.

The commit looks good. I'm going to review the code for other areas that this should be done. Once I've done this I'll create a release version.

I hope you will contribute further in the future. If so, can I ask that you raise an issue for any contributions (I created on for this) as it helps with the history and for people to find issues they may be facing. For other details on contribution guidelines please see: https://github.com/jcmturner/gokrb5/blob/master/CONTRIBUTING.md

Thanks again.

@jcmturner jcmturner merged commit 5db19ae into jcmturner:master Apr 21, 2018
@ah-
Copy link
Contributor Author

ah- commented Apr 21, 2018

Fantastic, thanks for merging so quickly! gokrb5 has been really great, without it we probably wouldn't have built our vault plugin at all.

I'll create an issue first next time, agree that it helps people find these changes more easily.

@jcmturner
Copy link
Owner

It's good to hear gokrb5 is useful to people.
Release v4.1.2 now has the fix for this.

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.

2 participants