Merge pull request #4063 from akallabeth/auth_fixes

Fixed leaks, certificate comparison and channel context cleanup
This commit is contained in:
David Fort 2017-08-30 10:19:12 +02:00 committed by GitHub
commit 5115ecd948
12 changed files with 52 additions and 31 deletions

View File

@ -1182,6 +1182,7 @@ static UINT cliprdr_virtual_channel_event_disconnected(cliprdrPlugin* cliprdr)
static UINT cliprdr_virtual_channel_event_terminated(cliprdrPlugin* cliprdr)
{
cliprdr->InitHandle = 0;
free(cliprdr->context);
free(cliprdr);
return CHANNEL_RC_OK;
}

View File

@ -1436,6 +1436,7 @@ static UINT drdynvc_virtual_channel_event_terminated(drdynvcPlugin* drdynvc)
return CHANNEL_RC_BAD_CHANNEL_HANDLE;
drdynvc->InitHandle = 0;
free(drdynvc->context);
free(drdynvc);
return CHANNEL_RC_OK;
}

View File

@ -1148,6 +1148,7 @@ static UINT encomsp_virtual_channel_event_disconnected(encomspPlugin* encomsp)
static UINT encomsp_virtual_channel_event_terminated(encomspPlugin* encomsp)
{
encomsp->InitHandle = 0;
free(encomsp->context);
free(encomsp);
return CHANNEL_RC_OK;
}

View File

@ -738,6 +738,7 @@ static UINT rail_virtual_channel_event_disconnected(railPlugin* rail)
static void rail_virtual_channel_event_terminated(railPlugin* rail)
{
rail->InitHandle = 0;
free(rail->context);
free(rail);
}

View File

@ -942,6 +942,7 @@ static UINT remdesk_virtual_channel_event_disconnected(remdeskPlugin* remdesk)
static void remdesk_virtual_channel_event_terminated(remdeskPlugin* remdesk)
{
remdesk->InitHandle = 0;
free(remdesk->context);
free(remdesk);
}

View File

@ -587,7 +587,7 @@ BOOL certificate_read_server_x509_certificate_chain(rdpCertificate* certificate,
if ((numCertBlobs - i) == 2)
{
rdpCertInfo cert_info;
rdpCertInfo cert_info = { 0 };
DEBUG_CERTIFICATE("License Server Certificate");

View File

@ -663,12 +663,6 @@ void freerdp_channels_close(rdpChannels* channels, freerdp* instance)
{
pChannelOpenData = &channels->openDataList[index];
if (pChannelOpenData->pInterface)
{
free(pChannelOpenData->pInterface);
pChannelOpenData->pInterface = NULL;
}
freerdp_channel_remove_open_handle_data(&g_ChannelHandles, pChannelOpenData->OpenHandle);
if (channels->openHandles)

View File

@ -143,7 +143,7 @@ void nla_identity_free(SEC_WINNT_AUTH_IDENTITY* identity)
* @param credssp
*/
int nla_client_init(rdpNla* nla)
static int nla_client_init(rdpNla* nla)
{
char* spn;
int length;
@ -388,7 +388,7 @@ int nla_client_begin(rdpNla* nla)
return 1;
}
int nla_client_recv(rdpNla* nla)
static int nla_client_recv(rdpNla* nla)
{
int status = -1;
@ -533,7 +533,7 @@ int nla_client_recv(rdpNla* nla)
return status;
}
int nla_client_authenticate(rdpNla* nla)
static int nla_client_authenticate(rdpNla* nla)
{
wStream* s;
int status;
@ -581,7 +581,7 @@ int nla_client_authenticate(rdpNla* nla)
* @param credssp
*/
int nla_server_init(rdpNla* nla)
static int nla_server_init(rdpNla* nla)
{
rdpTls* tls = nla->transport->tls;
@ -668,7 +668,7 @@ int nla_server_init(rdpNla* nla)
* @return 1 if authentication is successful
*/
int nla_server_authenticate(rdpNla* nla)
static int nla_server_authenticate(rdpNla* nla)
{
if (nla_server_init(nla) < 1)
return -1;
@ -913,7 +913,7 @@ int nla_authenticate(rdpNla* nla)
return nla_client_authenticate(nla);
}
void ap_integer_increment_le(BYTE* number, int size)
static void ap_integer_increment_le(BYTE* number, int size)
{
int index;
@ -932,7 +932,7 @@ void ap_integer_increment_le(BYTE* number, int size)
}
}
void ap_integer_decrement_le(BYTE* number, int size)
static void ap_integer_decrement_le(BYTE* number, int size)
{
int index;
@ -1006,9 +1006,9 @@ SECURITY_STATUS nla_decrypt_public_key_echo(rdpNla* nla)
ULONG pfQOP = 0;
BYTE* public_key1;
BYTE* public_key2;
int public_key_length;
int public_key_length = 0;
int signature_length;
SecBuffer Buffers[2];
SecBuffer Buffers[2] = { 0 };
SecBufferDesc Message;
SECURITY_STATUS status;
signature_length = nla->pubKeyAuth.cbBuffer - nla->PublicKey.cbBuffer;
@ -1042,6 +1042,7 @@ SECURITY_STATUS nla_decrypt_public_key_echo(rdpNla* nla)
{
WLog_ERR(TAG, "DecryptMessage failure %s [%08"PRIX32"]",
GetSecurityStatusString(status), status);
free(buffer);
return status;
}
@ -1061,6 +1062,7 @@ SECURITY_STATUS nla_decrypt_public_key_echo(rdpNla* nla)
winpr_HexDump(TAG, WLOG_ERROR, public_key1, public_key_length);
WLog_ERR(TAG, "Actual (length = %d):", public_key_length);
winpr_HexDump(TAG, WLOG_ERROR, public_key2, public_key_length);
free(buffer);
return SEC_E_MESSAGE_ALTERED; /* DO NOT SEND CREDENTIALS! */
}
@ -1176,7 +1178,7 @@ BOOL nla_read_ts_password_creds(rdpNla* nla, wStream* s)
return TRUE;
}
int nla_write_ts_password_creds(rdpNla* nla, wStream* s)
static int nla_write_ts_password_creds(rdpNla* nla, wStream* s)
{
int size = 0;
int innerSize = nla_sizeof_ts_password_creds(nla);
@ -1202,7 +1204,7 @@ int nla_write_ts_password_creds(rdpNla* nla, wStream* s)
return size;
}
int nla_sizeof_ts_credentials(rdpNla* nla)
static int nla_sizeof_ts_credentials(rdpNla* nla)
{
int size = 0;
size += ber_sizeof_integer(1);
@ -1215,7 +1217,7 @@ static BOOL nla_read_ts_credentials(rdpNla* nla, PSecBuffer ts_credentials)
{
wStream* s;
int length;
int ts_password_creds_length;
int ts_password_creds_length = 0;
BOOL ret;
s = Stream_New(ts_credentials->pvBuffer, ts_credentials->cbBuffer);
@ -1238,7 +1240,7 @@ static BOOL nla_read_ts_credentials(rdpNla* nla, PSecBuffer ts_credentials)
return ret;
}
int nla_write_ts_credentials(rdpNla* nla, wStream* s)
static int nla_write_ts_credentials(rdpNla* nla, wStream* s)
{
int size = 0;
int passwordSize;
@ -1261,7 +1263,7 @@ int nla_write_ts_credentials(rdpNla* nla, wStream* s)
* @param credssp
*/
BOOL nla_encode_ts_credentials(rdpNla* nla)
static BOOL nla_encode_ts_credentials(rdpNla* nla)
{
wStream* s;
int length;
@ -1313,7 +1315,7 @@ BOOL nla_encode_ts_credentials(rdpNla* nla)
return TRUE;
}
SECURITY_STATUS nla_encrypt_ts_credentials(rdpNla* nla)
static SECURITY_STATUS nla_encrypt_ts_credentials(rdpNla* nla)
{
SecBuffer Buffers[2];
SecBufferDesc Message;
@ -1357,7 +1359,7 @@ SECURITY_STATUS nla_encrypt_ts_credentials(rdpNla* nla)
return SEC_E_OK;
}
SECURITY_STATUS nla_decrypt_ts_credentials(rdpNla* nla)
static SECURITY_STATUS nla_decrypt_ts_credentials(rdpNla* nla)
{
int length;
BYTE* buffer;
@ -1394,6 +1396,7 @@ SECURITY_STATUS nla_decrypt_ts_credentials(rdpNla* nla)
{
WLog_ERR(TAG, "DecryptMessage failure %s [0x%08"PRIX32"]",
GetSecurityStatusString(status), status);
free(buffer);
return status;
}
@ -1407,14 +1410,14 @@ SECURITY_STATUS nla_decrypt_ts_credentials(rdpNla* nla)
return SEC_E_OK;
}
int nla_sizeof_nego_token(int length)
static int nla_sizeof_nego_token(int length)
{
length = ber_sizeof_octet_string(length);
length += ber_sizeof_contextual_tag(length);
return length;
}
int nla_sizeof_nego_tokens(int length)
static int nla_sizeof_nego_tokens(int length)
{
length = nla_sizeof_nego_token(length);
length += ber_sizeof_sequence_tag(length);
@ -1423,21 +1426,21 @@ int nla_sizeof_nego_tokens(int length)
return length;
}
int nla_sizeof_pub_key_auth(int length)
static int nla_sizeof_pub_key_auth(int length)
{
length = ber_sizeof_octet_string(length);
length += ber_sizeof_contextual_tag(length);
return length;
}
int nla_sizeof_auth_info(int length)
static int nla_sizeof_auth_info(int length)
{
length = ber_sizeof_octet_string(length);
length += ber_sizeof_contextual_tag(length);
return length;
}
int nla_sizeof_ts_request(int length)
static int nla_sizeof_ts_request(int length)
{
length += ber_sizeof_integer(2);
length += ber_sizeof_contextual_tag(3);
@ -1538,7 +1541,7 @@ BOOL nla_send(rdpNla* nla)
return TRUE;
}
int nla_decode_ts_request(rdpNla* nla, wStream* s)
static int nla_decode_ts_request(rdpNla* nla, wStream* s)
{
int length;
@ -1850,6 +1853,8 @@ void nla_free(rdpNla* nla)
if (nla->table)
{
SECURITY_STATUS status;
status = nla->table->FreeCredentialsHandle(&nla->credentials);
status = nla->table->DeleteSecurityContext(&nla->context);
if (status != SEC_E_OK)

View File

@ -184,7 +184,9 @@ static int certificate_data_match_legacy(rdpCertificateStore* certificate_store,
hostname, pline);
else if (strcmp(hostname, certificate_data->hostname) == 0)
{
match = strcmp(pline, certificate_data->fingerprint);
const int diff = strcmp(pline, certificate_data->fingerprint);
match = (diff == 0) ? 0 : -1;
break;
}
}

View File

@ -537,11 +537,17 @@ SECURITY_STATUS SEC_ENTRY ntlm_InitializeSecurityContextW(PCredHandle phCredenti
if (context->Workstation.Length < 1)
{
if (ntlm_SetContextWorkstation(context, NULL) < 0)
{
ntlm_ContextFree(context);
return SEC_E_INTERNAL_ERROR;
}
}
if (ntlm_SetContextServicePrincipalNameW(context, pszTargetName) < 0)
{
ntlm_ContextFree(context);
return SEC_E_INTERNAL_ERROR;
}
sspi_SecureHandleSetLowerPointer(phNewContext, context);
sspi_SecureHandleSetUpperPointer(phNewContext, (void*) NTLM_PACKAGE_NAME);

View File

@ -381,7 +381,7 @@ SECURITY_STATUS SEC_ENTRY negotiate_FreeCredentialsHandle(PCredHandle phCredenti
return SEC_E_INVALID_HANDLE;
sspi_CredentialsFree(credentials);
sspi_SecureHandleInvalidate(phCredential);
return SEC_E_OK;
}

View File

@ -304,6 +304,15 @@ void* sspi_SecureHandleGetLowerPointer(SecHandle* handle)
return pointer;
}
void sspi_SecureHandleInvalidate(SecHandle* handle)
{
if (!handle)
return;
handle->dwLower = 0;
handle->dwUpper = 0;
}
void sspi_SecureHandleSetLowerPointer(SecHandle* handle, void* pointer)
{
if (!handle)