NTLM buffer length insufficient

2013/12/16:
> I don't see any problem with the patch. Perhaps we could use a
> parameter to let InitializeSecurityContext allocate memory for you but
> I have to test it.
>
> Frediano

Thanks for the suggestion. I have previously missed that flag. Please see the patch below.

While I have done some basic testing (the DBD-Sybase test suite passes, the same way it did after I increased NTLMBUF_LEN), I would appreciate a thorough review and possibly some more testing. I can then make a merge request.

Thanks,
Sam

--- src\src\tds\sspi#3.c  2014-02-06 16:24:53.000000000 +-0100
+++ src\src\tds\sspi.c    2014-02-06 18:50:25.000000000 +-0100
@@ -120,19 +120,17 @@
tds_sspi_free(TDSSOCKET * tds, struct tds_authentication * tds_auth)
{
               TDSSSPIAUTH *auth = (TDSSSPIAUTH *) tds_auth;
                sec_fn->DeleteSecurityContext(&auth->cred_ctx);
               sec_fn->FreeCredentialsHandle(&auth->cred);
-              free(auth->tds_auth.packet);
+             sec_fn->FreeContextBuffer(auth->tds_auth.packet);
               free(auth->sname);
               free(auth);
               return TDS_SUCCEED;
}
-
-enum { NTLMBUF_LEN = 4096 };
 static int
tds_sspi_handle_next(TDSSOCKET * tds, struct tds_authentication * tds_auth, size_t len)
{
               SecBuffer in_buf, out_buf;
               SecBufferDesc in_desc, out_desc;
@@ -133,19 +131,19 @@
 static int
tds_sspi_handle_next(TDSSOCKET * tds, struct tds_authentication * tds_auth, size_t len)
{
               SecBuffer in_buf, out_buf;
               SecBufferDesc in_desc, out_desc;
               ULONG attrs;
               TimeStamp ts;
               TDS_UCHAR *auth_buf;
                TDSSSPIAUTH *auth = (TDSSSPIAUTH *) tds_auth;
-              if (len < 32 || len > NTLMBUF_LEN)
+             if (len < 32 || len > auth->tds_auth.packet_len)
                               return TDS_FAIL;
                auth_buf = (TDS_UCHAR *) malloc(len);
               if (!auth_buf)
                               return TDS_FAIL;
               tds_get_n(tds, auth_buf, (int)len);
@@ -147,19 +145,19 @@
                               return TDS_FAIL;
                auth_buf = (TDS_UCHAR *) malloc(len);
               if (!auth_buf)
                               return TDS_FAIL;
               tds_get_n(tds, auth_buf, (int)len);
               in_buf.BufferType = SECBUFFER_TOKEN;
               in_buf.pvBuffer   = auth_buf;
               in_buf.cbBuffer   = (ULONG)len;
                out_buf.BufferType = SECBUFFER_TOKEN;
               out_buf.pvBuffer   = auth->tds_auth.packet;
-              out_buf.cbBuffer   = NTLMBUF_LEN;
+             out_buf.cbBuffer   = auth->tds_auth.packet_len;
                status = sec_fn->InitializeSecurityContext(&auth->cred, &auth->cred_ctx, auth->sname,
                               ISC_REQ_CONFIDENTIALITY | ISC_REQ_REPLAY_DETECT | ISC_REQ_CONNECTION,
                               0, SECURITY_NETWORK_DREP, &in_desc,
                               0, &auth->cred_ctx, &out_desc,
                               &attrs, &ts);
@@ -166,69 +164,63 @@
                status = sec_fn->InitializeSecurityContext(&auth->cred, &auth->cred_ctx, auth->sname,
                               ISC_REQ_CONFIDENTIALITY | ISC_REQ_REPLAY_DETECT | ISC_REQ_CONNECTION,
                               0, SECURITY_NETWORK_DREP, &in_desc,
                               0, &auth->cred_ctx, &out_desc,
                               &attrs, &ts);
                               NULL, identity.Domain ? &identity : NULL,
                               NULL, NULL, &auth->cred, &ts) != SEC_E_OK) {
                               free(auth);
                               return NULL;
               }
-              /* allocate buffer */
-              auth->tds_auth.packet = (TDS_UCHAR *) malloc(NTLMBUF_LEN);
-              if (!auth->tds_auth.packet) {
-                              sec_fn->FreeCredentialsHandle(&auth->cred);
-                              free(auth);
-                              return NULL;
-              }
               desc.ulVersion = SECBUFFER_VERSION;
               desc.cBuffers  = 1;
               desc.pBuffers  = &buf;
-              buf.cbBuffer   = NTLMBUF_LEN;
+             buf.cbBuffer   = 0;
               buf.BufferType = SECBUFFER_TOKEN;
-              buf.pvBuffer   = auth->tds_auth.packet;
+             buf.pvBuffer   = NULL;
                /* build SPN */
               server_name = tds_dstr_cstr(&connection->server_host_name);
               if (strchr(server_name, '.') == NULL) {
                               struct hostent *host = gethostbyname(server_name);
                               if (host && strchr(host->h_name, '.') != NULL)
                                               server_name = host->h_name;
               }
               if (strchr(server_name, '.') != NULL) {
                               if (asprintf(&auth->sname, "MSSQLSvc/%s:%d", server_name, connection->port) < 0) {
-                                              free(auth->tds_auth.packet);
                                               sec_fn->FreeCredentialsHandle(&auth->cred);
                                               free(auth);
                                               return NULL;
                               }
                               tdsdump_log(TDS_DBG_NETWORK, "kerberos name %s\n", auth->sname);
               }
                status = sec_fn->InitializeSecurityContext(&auth->cred, NULL, auth->sname,
-                              ISC_REQ_CONFIDENTIALITY | ISC_REQ_REPLAY_DETECT | ISC_REQ_CONNECTION,
+                             ISC_REQ_CONFIDENTIALITY | ISC_REQ_REPLAY_DETECT | ISC_REQ_CONNECTION | ISC_REQ_ALLOCATE_MEMORY,
                               0, SECURITY_NETWORK_DREP,
                               NULL, 0,
                               &auth->cred_ctx, &desc,
                               &attrs, &ts);
                if (status == SEC_I_COMPLETE_AND_CONTINUE || status == SEC_I_CONTINUE_NEEDED) {
                               sec_fn->CompleteAuthToken(&auth->cred_ctx, &desc);
               } else if(status != SEC_E_OK) {
                               free(auth->sname);
-                              free(auth->tds_auth.packet);
+                             sec_fn->FreeContextBuffer(buf.pvBuffer);
                               sec_fn->FreeCredentialsHandle(&auth->cred);
                               free(auth);
                               return NULL;
               }
                auth->tds_auth.packet_len = buf.cbBuffer;
+             auth->tds_auth.packet = buf.pvBuffer;
+
               return &auth->tds_auth;
}
 #endif /* HAVE_SSPI */
 /** @} */



_______________________________________________

This message is for information purposes only, it is not a recommendation, advice, offer or solicitation to buy or sell a product or service nor an official confirmation of any transaction. It is directed at persons who are professionals and is not intended for retail customer use. Intended for recipient only. This message is subject to the terms at: www.barclays.com/emaildisclaimer.

For important disclosures, please see: www.barclays.com/salesandtradingdisclaimer regarding market commentary from Barclays Sales and/or Trading, who are active market participants; and in respect of Barclays Research, including disclosures relating to specific issuers, please see http://publicresearch.barclays.com.

_______________________________________________