steffengy / schannel-rs

Schannel API-bindings for rust (provides an interface for native SSL/TLS using windows APIs)
MIT License
49 stars 51 forks source link

Pass ISC_REQ_USE_SUPPLIED_CREDS to InitializeSecurityContext #24

Closed jethrogb closed 7 years ago

jethrogb commented 7 years ago

Fixes #23. As mentioned in there, not sure if this is the behavior you want.

sfackler commented 7 years ago

From the documentation, it sounds like this flag may not solve the problem described in #23?

When this flag is specified, Schannel will return SEC_I_INCOMPLETE_CREDENTIALS to the client when the server requests authentication and the client has not previously supplied a certificate.

https://msdn.microsoft.com/en-us/library/windows/desktop/aa378819(v=vs.85).aspx

steffengy commented 7 years ago

@jethrogb's testing and common usage such as https://github.com/curl/curl/blob/07fd7871b38cc8472c3806e254ba4062e3adeae0/lib/vtls/schannel.c#L594

seems to indicate that the documentation is off (once again).

sfackler commented 7 years ago

We would need a similar check for that error code, right: https://github.com/curl/curl/blob/07fd7871b38cc8472c3806e254ba4062e3adeae0/lib/vtls/schannel.c#L592

steffengy commented 7 years ago

In short, If we want the same behavior, yes.

Behavior 1 CURL

==> Potentially unwanted that certificates are sent automatically?

Other behavior (as of this PR)

The more sensible alternative would be to rely on ISC_REQ_USE_SUPPLIED_CRED for all cases. This would apparently only send the certificate associated with the acquired credential handle.

We already pass certificates to AcquireCredentialHandle so this behavior should also work out of the box (not sure if client auth is/was used before).

Not really having any hard opinion on this, actually pretty unsure what the best tradeoff is.

sfackler commented 7 years ago

Ah, right, misread that check. LGTM