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

Add CompleteAuthToken from SSPI #381

Closed
michael-o opened this issue Nov 14, 2014 · 17 comments
Closed

Add CompleteAuthToken from SSPI #381

michael-o opened this issue Nov 14, 2014 · 17 comments

Comments

@michael-o
Copy link

Currently, SSPI mappings completely miss CompleteAuthToken with the appropriate security statuses SEC_I_COMPLETE_NEEDED and SEC_I_COMPLETE_AND_CONTINUE which require the aforementioned function. Moreover, the return codes from this function must be available as well.

@michael-o michael-o changed the title Add CompleteAuthToken Add CompleteAuthToken from SSPI Nov 14, 2014
@dblock
Copy link
Member

dblock commented Nov 14, 2014

Please contribute!

@michael-o
Copy link
Author

OK, will have a look how things work in JNA to make the appropriate changes.

@dblock
Copy link
Member

dblock commented Aug 28, 2015

Does this also say that we're not handling SEC_I_COMPLETE_NEEDED in https://github.com/dblock/waffle? I've never seen this error in any production environment.

@michael-o
Copy link
Author

Yes, it does. I cannot tell when this is really necessary but if it is documented, it should be applied. The secrets of this are only known to Microsoft.

@dblock
Copy link
Member

dblock commented Aug 28, 2015

@michael-o Actually the doc is very clear on this, https://msdn.microsoft.com/en-us/library/windows/desktop/aa374764(v=vs.85).aspx says:

screenshot 2015-08-28 17 53 02

So it's not necessary for Negotiate (neither NTLM nor Kerberos).

@michael-o
Copy link
Author

That contradicts the return values for Kerberos: https://msdn.microsoft.com/en-us/library/windows/desktop/aa375507%28v=vs.85%29.aspx

One should ask Microsoft...

The safest best would be to comply to the return values and add some comment to the code. This is clearly a dispute.

@dblock
Copy link
Member

dblock commented Aug 29, 2015

I think the safest would be to handle it. Either way JNA needs this function mapping which is this issue, and please feel free to contribute.

@michael-o
Copy link
Author

This is something I have on my radar but won't happen before next month.

@matthiasblaesing
Copy link
Member

From my POV implementing CompleteAuthToken is of limited use. From my reading it is only used in digest authentication. I'm willing to look into this, but I failed to find documentation for a simple digest handshake. If someone can point me to sample code for digest auth, I'm willing to look into this.

@michael-o
Copy link
Author

@matthiasblaesing MSDN is contradicting itself. One place talks about Digest only, other places take about the general use.

@matthiasblaesing
Copy link
Member

@michael-o I won't dispute that here. I could bind the function just with the MSDN/header, but I want to have a unittest for the function. The current unittests simulate the authentication headshake in process, I failed to implement that for digest and given that digest is the only known module, that definitely requires this, I need sample code, that demonstrates the correct use.

@michael-o
Copy link
Author

@matthiasblaesing
Copy link
Member

Sorry - that code only exercises the NTLM and Negotiate packages. This won't help, as Negotiate is already used in the unittest code and does not lead to the requirement for CompleteAuthToken. As the MSDN only requires it for Digest, I'd like to code sample for that -- preferably a minimal "in-process" handshake.

@michael-o
Copy link
Author

Oh sorry, I thought you just wanted a real use case. But as you can see, the call is added for the sake of the interface. Being in a corporate environment, I have never seen a use of Digest. SPNEGO or NTLM only. Even though, one cannot be sure that it won't be used for NTLM, we cannot look into Microsoft's code.

@matthiasblaesing
Copy link
Member

I'm about to bind the method without testing, but working with SecBufferDesc I learned the hard way, that bindings without full testing are prone to failures. You'll end up with an untested method, that will most probably never be called, so you don't even know whether it would work.

@michael-o
Copy link
Author

Correct. I'd just add it. If someone does have some valid Digest use case, that'd be fine.

@matthiasblaesing
Copy link
Member

Ok, merged as announced.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants