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.

Sign up for GitHub

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

Jump to bottom

Fix hostbased name #66

Merged
merged 3 commits into from
Feb 24, 2022
Merged

Fix hostbased name #66

merged 3 commits into from
Feb 24, 2022

Conversation

simo5
Copy link
Collaborator

@simo5 simo5 commented Feb 24, 2022

Fixes #63

@simo5
Copy link
Collaborator Author

simo5 commented Feb 24, 2022

@filipnavara hopefully this addresses the SPN issue.
However it would be really nice to check that there are no regressions against a modern windows server given that I am fiddling with fields of the Traget Info that I haven't really touched before.

filipnavara
filipnavara approved these changes Feb 24, 2022
View reviewed changes
Copy link

@filipnavara filipnavara left a comment

Choose a reason for hiding this comment

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

Tested against Windows Server 2022 by HTTP Negotiate. Also tested against my locally running unit test with custom NTLM server and the SPN was echoed correctly through the authentication flow.

@simo5
Copy link
Collaborator Author

simo5 commented Feb 24, 2022

Nice,
I'm going to add just a couple of tests and then push it to master.

@simo5
Copy link
Collaborator Author

simo5 commented Feb 24, 2022

Glad I added tests, found incorrect behavior with names like "", "@", "@foo.bar", ...
@filipnavara do you have any tests with names like the above? ^^

@simo5
Store the full SPN within a server gssntlm_name
fce89ad 
The SPN is used to fill the Traget Name Attribute in the Target Info
array. This means we need to preserver the SPN as passed to us (via
conversion from a GSS Name).

This patch adds an spn field to the server union part of gssntlm_name
structure.

Signed-off-by: Simo Sorce <simo@redhat.com>
@simo5
Use the SPN for Target Info
2c882fc 
When we encode/decode/process target_info use the new stored SPN.
Also mark the SPN as unverified, because we never know if the calling
code speaks authoritatively, and may be passing an incorrect name.

Signed-off-by: Simo Sorce <simo@redhat.com>
@simo5
Fix serialization to export also the server spn
2035e33 
This ups both context and credentials export versions as the size and
content of the serilized structires change.

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

@filipnavara do you have any tests with names like the above? ^^

Not at the moment but I will try it and cross check with other implementations!

@simo5
Copy link
Collaborator Author

simo5 commented Feb 24, 2022

Alright, in the meanwhile I'll push this code as is, can always correct later if anything comes up about current behavior vs other implementations.

@simo5
Copy link
Collaborator Author

simo5 commented Feb 24, 2022

Please note that the implementation will not currently set Traget Name in Target Info if a gss name is obtained that only includes a name and not a full spn ...

@simo5 simo5 merged commit 8911176 into gssapi:main Feb 24, 2022
@jborean93 jborean93 mentioned this pull request Mar 15, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@filipnavara filipnavara filipnavara approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Service name from SPN is incorrectly dropped
2 participants
@simo5 @filipnavara