secur32: Fix some wrong assumptions in the NTLM test case, make it pass in XP SP2 and Wine.

dlls/secur32/ntlm.c

@@ -546,20 +546,23 @@ static SECURITY_STATUS SEC_ENTRY ntlm_InitializeSecurityContextW(
                 }
 
                 /* put the decoded client blob into the out buffer */
-                if(pOutput == NULL){
+
-                    TRACE("pOutput is NULL\n");
+                if (!pOutput || !pOutput->cBuffers || pOutput->pBuffers[0].cbBuffer < bin_len)
+                {
+                    TRACE("out buffer is NULL or has not enough space\n");
                     HeapFree(GetProcessHeap(), 0, buffer);
                     HeapFree(GetProcessHeap(), 0, bin);
-                    return SEC_E_INSUFFICIENT_MEMORY;
+                    return SEC_E_BUFFER_TOO_SMALL;
                 }
 
-                if(pOutput->cBuffers < 1)
+                if (!pOutput->pBuffers[0].pvBuffer)
                 {
-                    TRACE("pOutput->cBuffers is %ld\n", pOutput->cBuffers);
+                    TRACE("out buffer is NULL\n");
                     HeapFree(GetProcessHeap(), 0, buffer);
                     HeapFree(GetProcessHeap(), 0, bin);
-                    return SEC_E_INSUFFICIENT_MEMORY;
+                    return SEC_E_INTERNAL_ERROR;
                 }
+
                 pOutput->pBuffers[0].cbBuffer = bin_len;
                 pOutput->pBuffers[0].BufferType = SECBUFFER_DATA;
                 memcpy(pOutput->pBuffers[0].pvBuffer, bin, bin_len);
@@ -570,18 +573,18 @@ static SECURITY_STATUS SEC_ENTRY ntlm_InitializeSecurityContextW(
             {
                 /* handle second call here */
                 /* encode server data to base64 */
-                if(pInput == NULL)
+                if (!pInput || !pInput->cBuffers)
                 {
                     HeapFree(GetProcessHeap(), 0, buffer);
                     HeapFree(GetProcessHeap(), 0, bin);
                     return SEC_E_INCOMPLETE_MESSAGE;
                 }
 
-                if(pInput->cBuffers < 1)
+                if (!pInput->pBuffers[0].pvBuffer)
                 {
                     HeapFree(GetProcessHeap(), 0, buffer);
                     HeapFree(GetProcessHeap(), 0, bin);
-                    return SEC_E_INCOMPLETE_MESSAGE;
+                    return SEC_E_INTERNAL_ERROR;
                 }
 
                 if(pInput->pBuffers[0].cbBuffer > max_len)
@@ -638,18 +641,22 @@ static SECURITY_STATUS SEC_ENTRY ntlm_InitializeSecurityContextW(
                     return ret;
                 }
 
-                if(pOutput == NULL)
+                /* put the decoded client blob into the out buffer */
+
+                if (!pOutput || !pOutput->cBuffers || pOutput->pBuffers[0].cbBuffer < bin_len)
                 {
+                    TRACE("out buffer is NULL or has not enough space\n");
                     HeapFree(GetProcessHeap(), 0, buffer);
                     HeapFree(GetProcessHeap(), 0, bin);
-                    return SEC_E_INSUFFICIENT_MEMORY;
+                    return SEC_E_BUFFER_TOO_SMALL;
                 }
 
-                if(pOutput->cBuffers < 1)
+                if (!pOutput->pBuffers[0].pvBuffer)
                 {
+                    TRACE("out buffer is NULL\n");
                     HeapFree(GetProcessHeap(), 0, buffer);
                     HeapFree(GetProcessHeap(), 0, bin);
-                    return SEC_E_INSUFFICIENT_MEMORY;
+                    return SEC_E_INTERNAL_ERROR;
                 }
 
                 pOutput->pBuffers[0].cbBuffer = bin_len;
@@ -659,7 +666,6 @@ static SECURITY_STATUS SEC_ENTRY ntlm_InitializeSecurityContextW(
                 ret = SEC_E_OK;
                 phNewContext->dwUpper = ctxt_attr;
                 phNewContext->dwLower = ret;
-
             }
             HeapFree(GetProcessHeap(), 0, buffer);
             HeapFree(GetProcessHeap(), 0, bin);

dlls/secur32/secur32.c

@@ -680,19 +680,9 @@ static void SECUR32_freeProviders(void)
  */
 SECURITY_STATUS WINAPI FreeContextBuffer(PVOID pv)
 {
-    SECURITY_STATUS ret;
+    if (pv) SECUR32_FREE(pv);
 
-    /* as it turns out, SECURITY_STATUSes are actually HRESULTS */
+    return SEC_E_OK;
-    if (pv)
-    {
-        if (SECUR32_FREE(pv) == NULL)
-            ret = SEC_E_OK;
-        else
-            ret = HRESULT_FROM_WIN32(GetLastError());
-    }
-    else
-        ret = HRESULT_FROM_WIN32(ERROR_INVALID_PARAMETER);
-    return ret;
 }
 
 /***********************************************************************

dlls/secur32/secur32_priv.h

@@ -25,15 +25,10 @@
 #include "wine/list.h"
 
 /* Memory allocation functions for memory accessible by callers of secur32.
- * There is no REALLOC, because LocalReAlloc can only work if used in
+ * The details are implementation specific.
- * conjunction with LMEM_MOVEABLE and LocalLock, but callers aren't using
- * LocalLock.  I don't use the Heap functions because there seems to be an
- * implicit assumption that LocalAlloc and Free will be used--MS' secur32
- * imports them (but not the heap functions), the sample SSP uses them, and
- * there isn't an exported secur32 function to allocate memory.
  */
-#define SECUR32_ALLOC(bytes) LocalAlloc(0, (bytes))
+#define SECUR32_ALLOC(bytes) HeapAlloc(GetProcessHeap(), 0, (bytes))
-#define SECUR32_FREE(p)      LocalFree(p)
+#define SECUR32_FREE(p)      HeapFree(GetProcessHeap(), 0, (p))
 
 typedef struct _SecureProvider
 {

dlls/secur32/tests/main.c

@@ -2,6 +2,7 @@
  * Miscellaneous secur32 tests
  *
  * Copyright 2005 Kai Blin
+ * Copyright 2006 Dmitry Timoshkov
  *
  * This library is free software; you can redistribute it and/or
  * modify it under the terms of the GNU Lesser General Public
@@ -20,13 +21,15 @@
 
 #define SECURITY_WIN32
 
-#include <stdio.h>
 #include <stdarg.h>
-#include <windows.h>
+#include <stdio.h>
-#include "wine/test.h"
+#include <assert.h>
+#include <windef.h>
 #include <winbase.h>
 #include <sspi.h>
 
+#include "wine/test.h"
+
 #define BUFF_SIZE 2048
 #define MAX_MESSAGE 12000
 
@@ -72,6 +75,8 @@ void InitFunctionPtrs(void)
 
 static const char* getSecError(SECURITY_STATUS status)
 {
+    static char buf[20];
+
 #define _SEC_ERR(x) case (x): return #x;
     switch(status)
     {
@@ -93,9 +98,10 @@ static const char* getSecError(SECURITY_STATUS status)
         _SEC_ERR(SEC_E_ILLEGAL_MESSAGE);
         _SEC_ERR(SEC_E_LOGON_DENIED);
         _SEC_ERR(SEC_E_NO_CREDENTIALS);
+        _SEC_ERR(SEC_E_OUT_OF_SEQUENCE);
         default:
-            trace("Error = %ld\n", status);
+            sprintf(buf, "%08lx\n", status);
-            return "Unknown error";
+            return buf;
     }
 #undef _SEC_ERR
 }
@@ -106,7 +112,7 @@ static const char* getSecError(SECURITY_STATUS status)
 static SECURITY_STATUS setupPackageA(SEC_CHAR *p_package_name, 
         PSecPkgInfo *p_pkg_info)
 {
-    SECURITY_STATUS ret = SEC_E_SECPKG_NOT_FOUND;
+    SECURITY_STATUS ret;
     
     ret = pQuerySecurityPackageInfoA( p_package_name, p_pkg_info);
     return ret;
@@ -199,7 +205,7 @@ SECURITY_STATUS setupBuffers(PSecBufferDesc *new_in_buf,
     PSecBufferDesc in_buf = HeapAlloc(GetProcessHeap(), 0, 
             sizeof(SecBufferDesc));
     PSecBufferDesc out_buf = HeapAlloc(GetProcessHeap(), 0,
-            sizeof(PSecBufferDesc));
+            sizeof(SecBufferDesc));
 
     if(in_buf != NULL)
     {
@@ -254,7 +260,7 @@ SECURITY_STATUS setupBuffers(PSecBufferDesc *new_in_buf,
         out_buf->cBuffers = 1;
         out_buf->pBuffers = sec_buffer;
 
-        sec_buffer->cbBuffer = 0;
+        sec_buffer->cbBuffer = size;
         sec_buffer->BufferType = SECBUFFER_TOKEN;
         sec_buffer->pvBuffer = buffer;
         *new_out_buf = out_buf;
@@ -272,7 +278,7 @@ SECURITY_STATUS setupBuffers(PSecBufferDesc *new_in_buf,
 
 void cleanupBuffers(PSecBufferDesc in_buf, PSecBufferDesc out_buf)
 {
-    int i;
+    ULONG i;
 
     if(in_buf != NULL)
     {
@@ -305,8 +311,63 @@ SECURITY_STATUS runClient(PCredHandle cred, PCtxtHandle ctxt,
     ULONG ctxt_attr;
     TimeStamp ttl;
 
+    assert(in_buf->cBuffers >= 1);
+    assert(in_buf->pBuffers[0].pvBuffer != NULL);
+    assert(in_buf->pBuffers[0].cbBuffer != 0);
+
+    assert(out_buf->cBuffers >= 1);
+    assert(out_buf->pBuffers[0].pvBuffer != NULL);
+    assert(out_buf->pBuffers[0].cbBuffer != 0);
+
     trace("Running the client the %s time.\n", first?"first":"second");
 
+    /* We can either use ISC_REQ_ALLOCATE_MEMORY flag to ask the provider
+     * always allocate output buffers for us, or initialize cbBuffer
+     * before each call because the API changes it to represent actual
+     * amount of data in the buffer.
+     */
+
+    /* test a failing call only the first time, otherwise we get
+     * SEC_E_OUT_OF_SEQUENCE
+     */
+    if (first)
+    {
+        void *old_buf;
+
+        /* pass NULL as an output buffer */
+        ret = pInitializeSecurityContextA(cred, NULL, NULL, req_attr, 
+            0, SECURITY_NATIVE_DREP, NULL, 0, ctxt, NULL,
+            &ctxt_attr, &ttl);
+
+        ok(ret == SEC_E_BUFFER_TOO_SMALL, "expected SEC_E_BUFFER_TOO_SMALL, got %s\n", getSecError(ret));
+
+        /* pass NULL as an output buffer */
+        old_buf = out_buf->pBuffers[0].pvBuffer;
+        out_buf->pBuffers[0].pvBuffer = NULL;
+
+        ret = pInitializeSecurityContextA(cred, NULL, NULL, req_attr, 
+            0, SECURITY_NATIVE_DREP, NULL, 0, ctxt, out_buf,
+            &ctxt_attr, &ttl);
+
+        ok(ret == SEC_E_INTERNAL_ERROR, "expected SEC_E_INTERNAL_ERROR, got %s\n", getSecError(ret));
+
+        out_buf->pBuffers[0].pvBuffer = old_buf;
+
+        /* pass an output buffer of 0 size */
+        out_buf->pBuffers[0].cbBuffer = 0;
+
+        ret = pInitializeSecurityContextA(cred, NULL, NULL, req_attr, 
+            0, SECURITY_NATIVE_DREP, NULL, 0, ctxt, out_buf,
+            &ctxt_attr, &ttl);
+
+        ok(ret == SEC_E_BUFFER_TOO_SMALL, "expected SEC_E_BUFFER_TOO_SMALL, got %s\n", getSecError(ret));
+
+        ok(out_buf->pBuffers[0].cbBuffer == 0,
+           "InitializeSecurityContext set buffer size to %lu\n", out_buf->pBuffers[0].cbBuffer);
+    }
+
+    out_buf->pBuffers[0].cbBuffer = MAX_MESSAGE;
+
     ret = pInitializeSecurityContextA(cred, first?NULL:ctxt, NULL, req_attr, 
             0, SECURITY_NATIVE_DREP, first?NULL:in_buf, 0, ctxt, out_buf,
             &ctxt_attr, &ttl);
@@ -320,6 +381,9 @@ SECURITY_STATUS runClient(PCredHandle cred, PCtxtHandle ctxt,
             ret = SEC_E_OK;
     }       
 
+    ok(out_buf->pBuffers[0].cbBuffer < MAX_MESSAGE,
+       "InitializeSecurityContext set buffer size to %lu\n", out_buf->pBuffers[0].cbBuffer);
+
     return ret;
 }
 
@@ -346,7 +410,6 @@ SECURITY_STATUS runServer(PCredHandle cred, PCtxtHandle ctxt,
             ret = SEC_E_OK;
     }
 
-
     return ret;
 }
 
@@ -450,35 +513,29 @@ static void testEnumerateSecurityPackages(void)
 static void testQuerySecurityPackageInfo(void)
 {
     SECURITY_STATUS     sec_status;
-    SEC_CHAR            sec_pkg_name[256];
+    PSecPkgInfo         pkg_info;
-    PSecPkgInfo         pkg_info = NULL;
-    ULONG               max_token = 0;
-    USHORT              version = 0;
     
     trace("Running testQuerySecurityPackageInfo\n");
     
     /* Test with an existing package. Test should pass */
 
-    lstrcpy(sec_pkg_name, "NTLM");
+    pkg_info = (void *)0xdeadbeef;
-    
+    sec_status = setupPackageA("NTLM", &pkg_info);
-    sec_status = setupPackageA(sec_pkg_name, &pkg_info);
 
     ok((sec_status == SEC_E_OK) || (sec_status == SEC_E_SECPKG_NOT_FOUND), 
        "Return value of QuerySecurityPackageInfo() shouldn't be %s\n",
        getSecError(sec_status) );
         
-    if(pkg_info != NULL){
+    if (sec_status == SEC_E_OK)
-        max_token = pkg_info->cbMaxToken;
+    {
-        version   = pkg_info->wVersion;
+        ok(pkg_info != (void *)0xdeadbeef, "wrong pkg_info address %p\n", pkg_info);
-        ok(version == 1, "wVersion always should be 1, but is %d\n", version);
+        ok(pkg_info->wVersion == 1, "wVersion always should be 1, but is %d\n", pkg_info->wVersion);
-        todo_wine{
+        /* there is no point in testing pkg_info->cbMaxToken since it varies
-            ok(max_token == 12000, "cbMaxToken for NTLM is %ld, not 12000.\n",
+         * between implementations.
-                max_token);
+         */
-        }
     }
 
-    
+    sec_status = pFreeContextBuffer(pkg_info);
-    sec_status = pFreeContextBuffer(&pkg_info);
 
     ok( sec_status == SEC_E_OK,
         "Return value of FreeContextBuffer() shouldn't be %s\n",
@@ -486,23 +543,22 @@ static void testQuerySecurityPackageInfo(void)
 
     /* Test with a nonexistent package, test should fail */
 
-    lstrcpy(sec_pkg_name, "Winetest");
+    pkg_info = (void *)0xdeadbeef;
-
+    sec_status = pQuerySecurityPackageInfoA("Winetest", &pkg_info);
-    sec_status = pQuerySecurityPackageInfoA( sec_pkg_name, &pkg_info);
 
     ok( sec_status != SEC_E_OK,
         "Return value of QuerySecurityPackageInfo() should not be %s for a nonexistent package\n", getSecError(SEC_E_OK));
 
-    sec_status = pFreeContextBuffer(&pkg_info);
+    ok(pkg_info == (void *)0xdeadbeef, "wrong pkg_info address %p\n", pkg_info);
+
+    sec_status = pFreeContextBuffer(pkg_info);
     
     ok( sec_status == SEC_E_OK,
         "Return value of FreeContextBuffer() shouldn't be %s\n",
         getSecError(sec_status) );
-
-
 }
 
-void testAuth(const char* sec_pkg, const char* domain)
+void testAuth(char* sec_pkg_name, const char* domain)
 {
     SECURITY_STATUS     sec_status;
     PSecPkgInfo         pkg_info = NULL;
@@ -510,17 +566,15 @@ void testAuth(const char* sec_pkg, const char* domain)
     CredHandle          client_cred;
     CtxtHandle          server_ctxt;
     CtxtHandle          client_ctxt;
-    SEC_CHAR            sec_pkg_name[256];
 
     PSecBufferDesc client_in = NULL, client_out = NULL;
     PSecBufferDesc server_in = NULL, server_out = NULL;
 
     BOOL continue_client = FALSE, continue_server = FALSE;
 
-    lstrcpy(sec_pkg_name, sec_pkg);
     if(setupPackageA(sec_pkg_name, &pkg_info) == SEC_E_OK)
     {
-        pFreeContextBuffer(&pkg_info);
+        pFreeContextBuffer(pkg_info);
         sec_status = setupClient(&client_cred, "testuser", "testpass", domain,
                 sec_pkg_name);
 
@@ -614,7 +668,6 @@ void testAuth(const char* sec_pkg, const char* domain)
     {
         trace("Package not installed, skipping test.\n");
     }
-    pFreeContextBuffer(&pkg_info);
 }