Hi Hans,
Hans Leidekker hans@codeweavers.com wrote:
This series is based on a patch by Rob Shearman.
Since during some time I was involved in the project that aims adding Kerberos support to Wine (it's still not fully finished yet, that's why we haven't published it yet) I have some comments on your approach.
Our main target for testing was KerberosAuthenticationTester.exe from http://blog.michelbarneveld.nl/michel/archive/2009/12/05/kerberos-authentica... (the author doesn't provide the sources, but it's pretty trivial to get full C# source for the whole application, so understanding and debugging is not an issue). Another good test is klist.exe from (older) PSDK.
Our team also decided to use Rob's patch as a base for SSP, and in addition we've also implemented Kerberos ticket management API.
Main difference between your and our approaches is the architecture. We decided to add support for Kerberos using an approximation to what Windows has in place: implement Authentication Package (AP) manager in secur32 (LSA APIs) which dinamically loads APs listed in the registry. All the real code (both SSP and AP) is implemented in kerberos.dll, which dynamically loads libkrb5.so and gssapi_krb5.so.
Did you consider moving the Kerberos support into separate kerberos AP/SSP dll?
On Mon, 2017-10-16 at 17:05 +0800, Dmitry Timoshkov wrote:
Since during some time I was involved in the project that aims adding Kerberos support to Wine (it's still not fully finished yet, that's why we haven't published it yet) I have some comments on your approach.
Our main target for testing was KerberosAuthenticationTester.exe from http://blog.michelbarneveld.nl/michel/archive/2009/12/05/kerberos-authentica... (the author doesn't provide the sources, but it's pretty trivial to get full C# source for the whole application, so understanding and debugging is not an issue). Another good test is klist.exe from (older) PSDK.
Thanks, I'll give this a try.
Our team also decided to use Rob's patch as a base for SSP, and in addition we've also implemented Kerberos ticket management API.
Main difference between your and our approaches is the architecture. We decided to add support for Kerberos using an approximation to what Windows has in place: implement Authentication Package (AP) manager in secur32 (LSA APIs) which dinamically loads APs listed in the registry. All the real code (both SSP and AP) is implemented in kerberos.dll, which dynamically loads libkrb5.so and gssapi_krb5.so.
I assume you still need to load the native libraries dynamically because one is used to implement the AP and the other for the SSP, and either could be absent?
Did you consider moving the Kerberos support into separate kerberos AP/SSP dll?
I didn't, I followed the NTLM pattern in secur32.
Hans Leidekker hans@codeweavers.com wrote:
Our team also decided to use Rob's patch as a base for SSP, and in addition we've also implemented Kerberos ticket management API.
Main difference between your and our approaches is the architecture. We decided to add support for Kerberos using an approximation to what Windows has in place: implement Authentication Package (AP) manager in secur32 (LSA APIs) which dinamically loads APs listed in the registry. All the real code (both SSP and AP) is implemented in kerberos.dll, which dynamically loads libkrb5.so and gssapi_krb5.so.
I assume you still need to load the native libraries dynamically because one is used to implement the AP and the other for the SSP, and either could be absent?
That's correct. We also were considering using libkrb5.so directly in the SSP to avoid another dependency.
Did you consider moving the Kerberos support into separate kerberos AP/SSP dll?
I didn't, I followed the NTLM pattern in secur32.
NTLM support also needs to be moved to msv1_0.dll, but we decided to postpone that step.
I'm attaching the version of our patches that I have around for the reference. Feel free to use them as a base for your patches, or I could just send them to wine-patches (with proper sign-offs).
On Mon, 2017-10-16 at 17:55 +0800, Dmitry Timoshkov wrote:
I'm attaching the version of our patches that I have around for the reference. Feel free to use them as a base for your patches, or I could just send them to wine-patches (with proper sign-offs).
Thanks. From patch 7:
+ LOAD_FUNCPTR(gss_import_name); + LOAD_FUNCPTR(gss_acquire_cred); + LOAD_FUNCPTR(gss_release_name); + LOAD_FUNCPTR(gss_init_sec_context); + LOAD_FUNCPTR(gss_accept_sec_context); + LOAD_FUNCPTR(gss_delete_sec_context); + LOAD_FUNCPTR(gss_get_mic); + LOAD_FUNCPTR(gss_verify_mic); + LOAD_FUNCPTR(gss_release_cred); + LOAD_FUNCPTR(gss_wrap); + LOAD_FUNCPTR(gss_unwrap);
This is what Rob did originally and it would probably work with a Unix Kerberos server, but I found that we need the newer iov functions to make it work with an Active Directory server. From patch 8:
@@ -61,22 +69,50 @@ static SECURITY_STATUS SEC_ENTRY nego_AcquireCredentialsHandleW( PLUID pLogonID, PVOID pAuthData, SEC_GET_KEY_FN pGetKeyFn, PVOID pGetKeyArgument, PCredHandle phCredential, PTimeStamp ptsExpiry ) { - static SEC_WCHAR ntlmW[] = {'N','T','L','M',0}; SECURITY_STATUS ret; TRACE("%s, %s, 0x%08x, %p, %p, %p, %p, %p, %p\n", debugstr_w(pszPrincipal), debugstr_w(pszPackage), fCredentialUse, pLogonID, pAuthData, pGetKeyFn, pGetKeyArgument, phCredential, ptsExpiry); - FIXME("forwarding to NTLM\n"); - ret = ntlm_AcquireCredentialsHandleW( pszPrincipal, ntlmW, fCredentialUse, + /* Assume this */ + ret = SEC_E_INTERNAL_ERROR; + + /* First we need to try kerberos */ + + if (kerberos_provider) + { + ret = kerberos_provider->fnTableW. + AcquireCredentialsHandleW(pszPrincipal, kerberos_name_W, fCredentialUse, pLogonID, pAuthData, pGetKeyFn, pGetKeyArgument, - phCredential, ptsExpiry ); + phCredential, ptsExpiry); + } + if (ret == SEC_E_OK) { + /* FIXME: create KerberosCredentials */ NtlmCredentials *cred = (NtlmCredentials *)phCredential->dwLower; cred->no_cached_credentials = (pAuthData == NULL); + return ret; + } + + FIXME("Failed to AcquireCredentialHandle via Kerberos.\n"); + + /* Maybe ntlm? */ + if (ntlm_provider)
It's not part of this patch series but I have worked on the Negotiate part. I found that native is able to pick the right provider at the last possible moment, when the first authentication token arrives. So it can't work like this. We probably need to acquire credential handles for both providers and store them in the Negotiate handle until we can decide.
Hans Leidekker hans@codeweavers.com wrote:
...
This is what Rob did originally and it would probably work with a Unix Kerberos server, but I found that we need the newer iov functions to make it work with an Active Directory server. From patch 8:
...
It's not part of this patch series but I have worked on the Negotiate part. I found that native is able to pick the right provider at the last possible moment, when the first authentication token arrives. So it can't work like this. We probably need to acquire credential handles for both providers and store them in the Negotiate handle until we can decide.
Thanks for the review of the patches, I've passed this along.
Hans, are you planning to somehow reuse our patches or still prefer your own way of adding Kerberos support?
Hi Dmitry,
Hans Leidekker hans@codeweavers.com wrote:
...
This is what Rob did originally and it would probably work with a Unix Kerberos server, but I found that we need the newer iov functions to make it work with an Active Directory server. From patch 8:
...
It's not part of this patch series but I have worked on the Negotiate part. I found that native is able to pick the right provider at the last possible moment, when the first authentication token arrives. So it can't work like this. We probably need to acquire credential handles for both providers and store them in the Negotiate handle until we can decide.
Thanks for the review of the patches, I've passed this along.
Hans, are you planning to somehow reuse our patches or still prefer your own way of adding Kerberos support?
I don't see an immediate need to split the Kerberos code into a new dll. Do you have an application that depends on this?
Hans Leidekker hans@codeweavers.com wrote:
Hans, are you planning to somehow reuse our patches or still prefer your own way of adding Kerberos support?
I don't see an immediate need to split the Kerberos code into a new dll. Do you have an application that depends on this?
Yes, I have an application that indirectly depends on this: it's a security provider that installs its own DLL (GOST), adds it onto the list of existing SSPs in the registry, and expects that secur32 would load it on the applications' requests. Also separating Kerberos implementation moves external dependencies out of secur32.dll.
On Tue, 2017-10-17 at 16:08 +0800, Dmitry Timoshkov wrote:
Hans Leidekker hans@codeweavers.com wrote:
Hans, are you planning to somehow reuse our patches or still prefer your own way of adding Kerberos support?
I don't see an immediate need to split the Kerberos code into a new dll. Do you have an application that depends on this?
Yes, I have an application that indirectly depends on this: it's a security provider that installs its own DLL (GOST), adds it onto the list of existing SSPs in the registry, and expects that secur32 would load it on the applications' requests.
I see. In that case, would you be willing to upstream your SSP loader code?
Also separating Kerberos implementation moves external dependencies out of secur32.dll.
Well, if we load them dynamically there wouldn't be any difference in practice.
Hans Leidekker hans@codeweavers.com wrote:
On Tue, 2017-10-17 at 16:08 +0800, Dmitry Timoshkov wrote:
Hans Leidekker hans@codeweavers.com wrote:
Hans, are you planning to somehow reuse our patches or still prefer your own way of adding Kerberos support?
I don't see an immediate need to split the Kerberos code into a new dll. Do you have an application that depends on this?
Yes, I have an application that indirectly depends on this: it's a security provider that installs its own DLL (GOST), adds it onto the list of existing SSPs in the registry, and expects that secur32 would load it on the applications' requests.
I see. In that case, would you be willing to upstream your SSP loader code?
The thing is that the secur32.dll already has the SPP loading code, however it lacks the LSA Authentication Package (AP) manager, and the AP is another part of the security provider DLL.
Also separating Kerberos implementation moves external dependencies out of secur32.dll.
Well, if we load them dynamically there wouldn't be any difference in practice.
Sure.
On Tue, 2017-10-17 at 16:54 +0800, Dmitry Timoshkov wrote:
Yes, I have an application that indirectly depends on this: it's a security
provider that installs its own DLL (GOST), adds it onto the list of existing SSPs in the registry, and expects that secur32 would load it on the applications' requests.
I see. In that case, would you be willing to upstream your SSP loader code?
The thing is that the secur32.dll already has the SPP loading code, however it lacks the LSA Authentication Package (AP) manager, and the AP is another part of the security provider DLL.
Right, I guess I should have said SSP/AP loader code. I think this should be a separate patch, and it could be done either before my patches or after. Doing it beforehand would avoid some code churn of course.
Since you have a real application to test with it would be of great help if you could upstream your code. I wouldn't mind rebasing my patches on top of yours.
Hans Leidekker hans@codeweavers.com wrote:
On Tue, 2017-10-17 at 16:54 +0800, Dmitry Timoshkov wrote:
Yes, I have an application that indirectly depends on this: it's a security
provider that installs its own DLL (GOST), adds it onto the list of existing SSPs in the registry, and expects that secur32 would load it on the applications' requests.
I see. In that case, would you be willing to upstream your SSP loader code?
The thing is that the secur32.dll already has the SPP loading code, however it lacks the LSA Authentication Package (AP) manager, and the AP is another part of the security provider DLL.
Right, I guess I should have said SSP/AP loader code. I think this should be a separate patch, and it could be done either before my patches or after. Doing it beforehand would avoid some code churn of course.
Since you have a real application to test with it would be of great help if you could upstream your code. I wouldn't mind rebasing my patches on top of yours.
I just sent first two patches of the series I sent to you earlier.
Just in case here is a reference to the Windows security architecture description I find pretty useful for my personal education: https://technet.microsoft.com/en-us/library/cc780455(v=ws.10).aspx
On Mon, 2017-10-16 at 17:55 +0800, Dmitry Timoshkov wrote: diff --git a/wine/dlls/kerberos/kerberos.spec b/wine/dlls/kerberos/kerberos.spec
diff --git a/wine/dlls/kerberos/kerberos.spec b/wine/dlls/kerberos/kerberos.spec index d277cee..e92516f 100644 --- a/wine/dlls/kerberos/kerberos.spec +++ b/wine/dlls/kerberos/kerberos.spec @@ -1 +1,3 @@ @ stdcall SpLsaModeInitialize(long ptr ptr ptr) +@ stdcall InitSecurityInterfaceA() +@ stdcall InitSecurityInterfaceW()
The Windows 10 version doesn't export these functions. It does export a SECPKG_USER_FUNCTION_TABLE via SpUserModeInitialize but that doesn't give us everything we need.
diff --git a/wine/loader/wine.inf.in b/wine/loader/wine.inf.in index 787ad7f..0a9f8c3 100644 --- a/wine/loader/wine.inf.in +++ b/wine/loader/wine.inf.in @@ -683,6 +683,8 @@ HKLM,Software\Policies,,16 HKLM,Software\Registered Applications,,16 HKLM,System\CurrentControlSet\Control\hivelist,,16 HKLM,System\CurrentControlSet\Control\Lsa\Kerberos,,16 +HKLM,System\CurrentControlSet\Control\Lsa\Kerberos,,16 +HKLM,System\CurrentControlSet\Control\SecurityProviders,"SecurityProviders",,"kerberos"
I don't see "kerberos" listed under SecurityProviders. There's just one entry called "credssp" on Windows 7 and 10.
So I'm inclined to keep the Kerberos SSP in secur32 for now. Would you agree?
Hans Leidekker hans@codeweavers.com wrote:
On Mon, 2017-10-16 at 17:55 +0800, Dmitry Timoshkov wrote: diff --git a/wine/dlls/kerberos/kerberos.spec b/wine/dlls/kerberos/kerberos.spec
diff --git a/wine/dlls/kerberos/kerberos.spec b/wine/dlls/kerberos/kerberos.spec index d277cee..e92516f 100644 --- a/wine/dlls/kerberos/kerberos.spec +++ b/wine/dlls/kerberos/kerberos.spec @@ -1 +1,3 @@ @ stdcall SpLsaModeInitialize(long ptr ptr ptr) +@ stdcall InitSecurityInterfaceA() +@ stdcall InitSecurityInterfaceW()
The Windows 10 version doesn't export these functions. It does export a SECPKG_USER_FUNCTION_TABLE via SpUserModeInitialize but that doesn't give us everything we need.
dlls/schannel stub in Wine has the same problem.
I guess that George Popoff (the author of this code) added these exports to make Wine SSP loader work. I have to admit that I haven't verified this part of the code myself against kerberos.dll in Windows, and assumed that Wine SSP loader code does the correct thing. Please give me a bit of time to investigate this matter once again.
diff --git a/wine/loader/wine.inf.in b/wine/loader/wine.inf.in index 787ad7f..0a9f8c3 100644 --- a/wine/loader/wine.inf.in +++ b/wine/loader/wine.inf.in @@ -683,6 +683,8 @@ HKLM,Software\Policies,,16 HKLM,Software\Registered Applications,,16 HKLM,System\CurrentControlSet\Control\hivelist,,16 HKLM,System\CurrentControlSet\Control\Lsa\Kerberos,,16 +HKLM,System\CurrentControlSet\Control\Lsa\Kerberos,,16 +HKLM,System\CurrentControlSet\Control\SecurityProviders,"SecurityProviders",,"kerberos"
I don't see "kerberos" listed under SecurityProviders. There's just one entry called "credssp" on Windows 7 and 10.
There is no msv1_0 (NTLM provider) there either, probably Windows loads them from the Control\Lsa key.
So I'm inclined to keep the Kerberos SSP in secur32 for now. Would you agree?
No, I don't agree. Windows definitely has all the providers in separate dlls - msv1_0 (NTLM), kerberos, schannel, etc. We need to find out how to properly implement kerberos support as a kerberos.dll. This will also help to move NTLM code to msv1_0.dll.
Hans Leidekker hans@codeweavers.com wrote:
On Mon, 2017-10-16 at 17:55 +0800, Dmitry Timoshkov wrote: diff --git a/wine/dlls/kerberos/kerberos.spec b/wine/dlls/kerberos/kerberos.spec
diff --git a/wine/dlls/kerberos/kerberos.spec b/wine/dlls/kerberos/kerberos.spec index d277cee..e92516f 100644 --- a/wine/dlls/kerberos/kerberos.spec +++ b/wine/dlls/kerberos/kerberos.spec @@ -1 +1,3 @@ @ stdcall SpLsaModeInitialize(long ptr ptr ptr) +@ stdcall InitSecurityInterfaceA() +@ stdcall InitSecurityInterfaceW()
The Windows 10 version doesn't export these functions. It does export a SECPKG_USER_FUNCTION_TABLE via SpUserModeInitialize but that doesn't give us everything we need.
It looks like MSDN suggests that both SECPKG_FUNCTION_TABLE (returned by SpLsaModeInitialize) and SECPKG_USER_FUNCTION_TABLE (returned by SpUserModeInitialize) should be used for an SSPI: https://msdn.microsoft.com/en-us/library/windows/desktop/aa380175(v=vs.85).a... https://msdn.microsoft.com/en-us/library/windows/desktop/aa380185(v=vs.85).a...
Since this requires quite a bit of changing to secur32 SSP management code I'd suggest to simply add two above exports for now.
On Fri, 2017-10-20 at 10:51 +0800, Dmitry Timoshkov wrote:
Hans Leidekker hans@codeweavers.com wrote:
On Mon, 2017-10-16 at 17:55 +0800, Dmitry Timoshkov wrote: diff --git a/wine/dlls/kerberos/kerberos.spec b/wine/dlls/kerberos/kerberos.spec
diff --git a/wine/dlls/kerberos/kerberos.spec b/wine/dlls/kerberos/kerberos.spec index d277cee..e92516f 100644 --- a/wine/dlls/kerberos/kerberos.spec +++ b/wine/dlls/kerberos/kerberos.spec @@ -1 +1,3 @@ @ stdcall SpLsaModeInitialize(long ptr ptr ptr) +@ stdcall InitSecurityInterfaceA() +@ stdcall InitSecurityInterfaceW()
The Windows 10 version doesn't export these functions. It does export a SECPKG_USER_FUNCTION_TABLE via SpUserModeInitialize but that doesn't give us everything we need.
It looks like MSDN suggests that both SECPKG_FUNCTION_TABLE (returned by SpLsaModeInitialize) and SECPKG_USER_FUNCTION_TABLE (returned by SpUserModeInitialize) should be used for an SSPI: https://msdn.microsoft.com/en-us/library/windows/desktop/aa380175(v=vs.85).a... https://msdn.microsoft.com/en-us/library/windows/desktop/aa380185(v=vs.85).a...
Since this requires quite a bit of changing to secur32 SSP management code I'd suggest to simply add two above exports for now.
Neither option reflects what Windows does. The dll listed under SecurityProviders (credssp) doesn't export SpUserModeInitialize or SpLsaModeInitialize. It does export InitSecurityInterfaceW, but it's not called kerberos.
It seems to me as if the MSDN pages you are referring to describe the old situation, and MS has since replaced the list of providers with credssp, which tunnels SPNEGO over a TLS channel.
Hans Leidekker hans@codeweavers.com wrote:
On Fri, 2017-10-20 at 10:51 +0800, Dmitry Timoshkov wrote:
Hans Leidekker hans@codeweavers.com wrote:
On Mon, 2017-10-16 at 17:55 +0800, Dmitry Timoshkov wrote: diff --git a/wine/dlls/kerberos/kerberos.spec b/wine/dlls/kerberos/kerberos.spec
diff --git a/wine/dlls/kerberos/kerberos.spec b/wine/dlls/kerberos/kerberos.spec index d277cee..e92516f 100644 --- a/wine/dlls/kerberos/kerberos.spec +++ b/wine/dlls/kerberos/kerberos.spec @@ -1 +1,3 @@ @ stdcall SpLsaModeInitialize(long ptr ptr ptr) +@ stdcall InitSecurityInterfaceA() +@ stdcall InitSecurityInterfaceW()
The Windows 10 version doesn't export these functions. It does export a SECPKG_USER_FUNCTION_TABLE via SpUserModeInitialize but that doesn't give us everything we need.
It looks like MSDN suggests that both SECPKG_FUNCTION_TABLE (returned by SpLsaModeInitialize) and SECPKG_USER_FUNCTION_TABLE (returned by SpUserModeInitialize) should be used for an SSPI: https://msdn.microsoft.com/en-us/library/windows/desktop/aa380175(v=vs.85).a... https://msdn.microsoft.com/en-us/library/windows/desktop/aa380185(v=vs.85).a...
Since this requires quite a bit of changing to secur32 SSP management code I'd suggest to simply add two above exports for now.
Neither option reflects what Windows does. The dll listed under SecurityProviders (credssp) doesn't export SpUserModeInitialize or SpLsaModeInitialize. It does export InitSecurityInterfaceW, but it's not called kerberos.
It seems to me as if the MSDN pages you are referring to describe the old situation, and MS has since replaced the list of providers with credssp, which tunnels SPNEGO over a TLS channel.
Yes, it looks like newer Windows versions have changed the behaviour, so I'd still suggest to create an SSP dll that is compatible with Wine's secur32 SSP loader expectations.
On Fri, 2017-10-20 at 17:00 +0800, Dmitry Timoshkov wrote:
It seems to me as if the MSDN pages you are referring to describe the
old situation, and MS has since replaced the list of providers with credssp, which tunnels SPNEGO over a TLS channel.
Yes, it looks like newer Windows versions have changed the behaviour, so I'd still suggest to create an SSP dll that is compatible with Wine's secur32 SSP loader expectations.
I loaded credssp directly and called its EnumerateSecurityPackages export. It only exports a CREDSSP provider.
If I call the EnumerateSecurityPackages export from secur32 I get 9 providers, one of which is CREDSSP. If I remove that entry from the SecurityProviders key I get 8 providers: CREDSSP is no longer there.
So while this key is consulted, it's not used to load NTLM, Kerberos or any of the other providers.