search

gssapi / gss-ntlmssp

A complete implementation of the MS-NLMP documents as a GSSAPI mechanism
ISC License
30 stars 26 forks source link

Support get_sids request via name attributes #36

Closed kvv81 closed 4 years ago

kvv81 commented 4 years ago

To allow server to get details about authenticated user new get_sid request by "urn:gssntlmssp:sids" name attribute is implemented (see RFC6680) to report the list of user SIDs. The list of SIDs is stored in gssntlm source_name (part of gss context, can be get by gss_inquire_context) in textual representation - comma-separated list of SIDs. gss_get_name_attribute() and gssntlm_inquire_name() should be used to inquire name attributes.

See previous discussion here: https://github.com/gssapi/gss-ntlmssp/pull/31

Signed-off-by: Volodymyr Khomenko volodymyr@vastdata.com

kvv81 commented 4 years ago

I have tested new end-to-end flow from our code via gss_accept_sec_context + gss_inquire_context->gss_get_name_attribute. Also basic gss_accept_sec_context() + name copy part test was done via gss-server/gss-client utils that available in MIT library.

gssntlm_inquire_name was not tested yet...

Yes, I agree - more testing is needed. I see you already have 'tests' subdirectory with ntlmssptest.c - probably we can make test_gssapi_rfc6680 function covering this.

simo5 commented 4 years ago

Yes, I agree - more testing is needed. I see you already have 'tests' subdirectory with ntlmssptest.c - probably we can make test_gssapi_rfc6680 function covering this.

Yes exactly what I would expect, should be simple enough (again not expecting end-to-end test with wibindd in the mix, but could be a welcome optional test :-)

kvv81 commented 4 years ago

Yes, unit-test is a last piece of puzzle for this PR I have planned to do.

I'm not sure how all previous comments will behave once I remove all previous commits and add a 'squashed' commit, usually 'squashing' into single commit is done directly during the merge. I can make a new PR with rebased code and squashed commit only to resolve this, is it good enough for you?

simo5 commented 4 years ago

Please do NOT create a new PR, just squash the commits so I can do a final check before pushing. We do not do merges only fast forwards.

kvv81 commented 4 years ago

Ok, rebased + squashed everything to single commit in the same PR. Please re-review everything once again.

simo5 commented 4 years ago

I just realized that we missed to handle the attribute in export_name()/import_name() in gss_serialize.c, but it's ok, I'll handle that in a followup PR.