From: Armin Novak Date: Mon, 20 Nov 2017 09:11:35 +0000 (+0100) Subject: Fix #4247: warnings introduced with #3904 X-Git-Tag: 2.0.0-rc1~17^2~1 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=4fe12b0ea3cd5df7196f43bb1fab753f56ad478b;p=platform%2Fupstream%2Ffreerdp.git Fix #4247: warnings introduced with #3904 --- diff --git a/libfreerdp/core/certificate.c b/libfreerdp/core/certificate.c index 482f6d4..70b1084 100644 --- a/libfreerdp/core/certificate.c +++ b/libfreerdp/core/certificate.c @@ -172,7 +172,6 @@ BOOL certificate_read_x509_certificate(rdpCertBlob* cert, rdpCertInfo* info) return FALSE; memset(info, 0, sizeof(rdpCertInfo)); - s = Stream_New(cert->data, cert->length); if (!s) @@ -323,7 +322,6 @@ error1: rdpX509CertChain* certificate_new_x509_certificate_chain(UINT32 count) { rdpX509CertChain* x509_cert_chain; - x509_cert_chain = (rdpX509CertChain*) malloc(sizeof(rdpX509CertChain)); if (!x509_cert_chain) @@ -396,25 +394,28 @@ static BOOL certificate_process_server_public_key(rdpCertificate* certificate, w Stream_Read(s, certificate->cert_info.Modulus, certificate->cert_info.ModulusLength); Stream_Seek(s, 8); /* 8 bytes of zero padding */ - return TRUE; } static BOOL certificate_process_server_public_signature(rdpCertificate* certificate, - const BYTE* sigdata, int sigdatalen, wStream* s, UINT32 siglen) + const BYTE* sigdata, int sigdatalen, wStream* s, UINT32 siglen) { - int i, sum; + size_t i, sum; BYTE sig[TSSK_KEY_LENGTH]; BYTE encsig[TSSK_KEY_LENGTH + 8]; +#if defined(VALIDATE_MD5) BYTE md5hash[WINPR_MD5_DIGEST_LENGTH]; +#endif + /* Do not bother with validation of server proprietary certificate. The use of MD5 here is not allowed under FIPS. + * Since the validation is not protecting against anything since the private/public keys are well known and documented in + * MS-RDPBCGR section 5.3.3.1, we are not gaining any security by using MD5 for signature comparison. Rather then use MD5 + * here we just dont do the validation to avoid its use. Historically, freerdp has been ignoring a failed validation anyways. */ +#if defined(VALIDATE_MD5) - /* Do not bother with validation of server proprietary certificate. The use of MD5 here is not allowed under FIPS. */ - /* Since the validation is not protecting against anything since the private/public keys are well known and documented in */ - /* MS-RDPBCGR section 5.3.3.1, we are not gaining any security by using MD5 for signature comparison. Rather then use MD5 */ - /* here we just dont do the validation to avoid its use. Historically, freerdp has been ignoring a failed validation anyways. */ - /*if (!winpr_Digest(WINPR_MD_MD5, sigdata, sigdatalen, md5hash, sizeof(md5hash))) - return FALSE;*/ + if (!winpr_Digest(WINPR_MD_MD5, sigdata, sigdatalen, md5hash, sizeof(md5hash))) + return FALSE; +#endif Stream_Read(s, encsig, siglen); /* Last 8 bytes shall be all zero. */ @@ -425,21 +426,29 @@ static BOOL certificate_process_server_public_signature(rdpCertificate* certific if (sum != 0) { WLog_ERR(TAG, "invalid signature"); - //return FALSE; + return FALSE; } siglen -= 8; - // TODO: check the result of decrypt - crypto_rsa_public_decrypt(encsig, siglen, TSSK_KEY_LENGTH, tssk_modulus, tssk_exponent, sig); + if (crypto_rsa_public_decrypt(encsig, siglen, TSSK_KEY_LENGTH, tssk_modulus, tssk_exponent, + sig) <= 0) + { + WLog_ERR(TAG, "invalid RSA decrypt"); + return FALSE; + } /* Verify signature. */ /* Do not bother with validation of server proprietary certificate as described above. */ - /*if (memcmp(md5hash, sig, sizeof(md5hash)) != 0) +#if defined(VALIDATE_MD5) + + if (memcmp(md5hash, sig, sizeof(md5hash)) != 0) { WLog_ERR(TAG, "invalid signature"); - //return FALSE; - }*/ + return FALSE; + } + +#endif /* * Verify rest of decrypted data: @@ -454,7 +463,7 @@ static BOOL certificate_process_server_public_signature(rdpCertificate* certific if (sig[16] != 0x00 || sum != 0xFF * (62 - 17) || sig[62] != 0x01) { WLog_ERR(TAG, "invalid signature"); - //return FALSE; + return FALSE; } return TRUE; @@ -561,7 +570,6 @@ BOOL certificate_read_server_x509_certificate_chain(rdpCertificate* certificate, BOOL ret; UINT32 certLength; UINT32 numCertBlobs; - DEBUG_CERTIFICATE("Server X.509 Certificate Chain"); if (Stream_GetRemainingLength(s) < 4) @@ -595,13 +603,11 @@ BOOL certificate_read_server_x509_certificate_chain(rdpCertificate* certificate, if ((numCertBlobs - i) == 2) { rdpCertInfo cert_info = { 0 }; - DEBUG_CERTIFICATE("License Server Certificate"); ret = certificate_read_x509_certificate(&certificate->x509_cert_chain->array[i], &cert_info); DEBUG_LICENSE("modulus length:%"PRIu32"", cert_info.ModulusLength); - free(cert_info.Modulus); if (!ret) @@ -668,7 +674,6 @@ BOOL certificate_read_server_certificate(rdpCertificate* certificate, BYTE* serv } Stream_Free(s, FALSE); - return ret; } @@ -680,12 +685,13 @@ rdpRsaKey* key_new_from_content(const char *keycontent, const char *keyfile) const BIGNUM *rsa_e = NULL; const BIGNUM *rsa_n = NULL; const BIGNUM *rsa_d = NULL; - key = (rdpRsaKey*) calloc(1, sizeof(rdpRsaKey)); + if (!key) return NULL; bio = BIO_new_mem_buf((void *)keycontent, strlen(keycontent)); + if (!bio) goto out_free; @@ -742,7 +748,6 @@ rdpRsaKey* key_new_from_content(const char *keycontent, const char *keyfile) crypto_reverse(key->exponent, sizeof(key->exponent)); RSA_free(rsa); return key; - out_free_modulus: free(key->Modulus); out_free_rsa: @@ -759,8 +764,8 @@ rdpRsaKey* key_new(const char* keyfile) INT64 length; char* buffer = NULL; rdpRsaKey* key = NULL; - fp = fopen(keyfile, "rb"); + if (!fp) { WLog_ERR(TAG, "unable to open RSA key file %s: %s.", keyfile, strerror(errno)); @@ -769,27 +774,31 @@ rdpRsaKey* key_new(const char* keyfile) if (_fseeki64(fp, 0, SEEK_END) < 0) goto out_free; + if ((length = _ftelli64(fp)) < 0) goto out_free; + if (_fseeki64(fp, 0, SEEK_SET) < 0) goto out_free; buffer = (char *)malloc(length + 1); + if (!buffer) goto out_free; if (fread((void*) buffer, length, 1, fp) != 1) goto out_free; + fclose(fp); buffer[length] = '\0'; - key = key_new_from_content(buffer, keyfile); free(buffer); return key; - out_free: + if (fp) fclose(fp); + free(buffer); return NULL; } @@ -817,8 +826,10 @@ rdpCertificate* certificate_clone(rdpCertificate* certificate) if (certificate->cert_info.ModulusLength) { _certificate->cert_info.Modulus = (BYTE*) malloc(certificate->cert_info.ModulusLength); + if (!_certificate->cert_info.Modulus) goto out_fail; + CopyMemory(_certificate->cert_info.Modulus, certificate->cert_info.Modulus, certificate->cert_info.ModulusLength); _certificate->cert_info.ModulusLength = certificate->cert_info.ModulusLength; } @@ -826,13 +837,16 @@ rdpCertificate* certificate_clone(rdpCertificate* certificate) if (certificate->x509_cert_chain) { _certificate->x509_cert_chain = (rdpX509CertChain*) malloc(sizeof(rdpX509CertChain)); + if (!_certificate->x509_cert_chain) goto out_fail; + CopyMemory(_certificate->x509_cert_chain, certificate->x509_cert_chain, sizeof(rdpX509CertChain)); if (certificate->x509_cert_chain->count) { _certificate->x509_cert_chain->array = (rdpCertBlob*) calloc(certificate->x509_cert_chain->count, sizeof(rdpCertBlob)); + if (!_certificate->x509_cert_chain->array) goto out_fail; @@ -843,6 +857,7 @@ rdpCertificate* certificate_clone(rdpCertificate* certificate) if (certificate->x509_cert_chain->array[index].length) { _certificate->x509_cert_chain->array[index].data = (BYTE*) malloc(certificate->x509_cert_chain->array[index].length); + if (!_certificate->x509_cert_chain->array[index].data) { for (--index; index >= 0; --index) @@ -850,8 +865,10 @@ rdpCertificate* certificate_clone(rdpCertificate* certificate) if (certificate->x509_cert_chain->array[index].length) free(_certificate->x509_cert_chain->array[index].data); } + goto out_fail; } + CopyMemory(_certificate->x509_cert_chain->array[index].data, certificate->x509_cert_chain->array[index].data, _certificate->x509_cert_chain->array[index].length); } @@ -860,13 +877,14 @@ rdpCertificate* certificate_clone(rdpCertificate* certificate) } return _certificate; - out_fail: + if (_certificate->x509_cert_chain) { free(_certificate->x509_cert_chain->array); free(_certificate->x509_cert_chain); } + free(_certificate->cert_info.Modulus); free(_certificate); return NULL; @@ -894,8 +912,6 @@ void certificate_free(rdpCertificate* certificate) return; certificate_free_x509_certificate_chain(certificate->x509_cert_chain); - free(certificate->cert_info.Modulus); - free(certificate); } diff --git a/libfreerdp/core/freerdp.c b/libfreerdp/core/freerdp.c index 4468dc2..483c3bc 100644 --- a/libfreerdp/core/freerdp.c +++ b/libfreerdp/core/freerdp.c @@ -615,7 +615,6 @@ BOOL freerdp_context_new(freerdp* instance) rdpRdp* rdp; rdpContext* context; BOOL ret = TRUE; - DWORD flags = WINPR_SSL_INIT_DEFAULT; instance->context = (rdpContext*) calloc(1, instance->ContextSize); if (!instance->context) diff --git a/libfreerdp/core/security.c b/libfreerdp/core/security.c index e51898c..be03311 100644 --- a/libfreerdp/core/security.c +++ b/libfreerdp/core/security.c @@ -137,31 +137,41 @@ static BOOL security_salted_hash(const BYTE* salt, const BYTE* input, int length /* SHA1_Digest = SHA1(Input + Salt + Salt1 + Salt2) */ if (!(sha1 = winpr_Digest_New())) goto out; + if (!winpr_Digest_Init(sha1, WINPR_MD_SHA1)) goto out; + if (!winpr_Digest_Update(sha1, input, length)) /* Input */ goto out; + if (!winpr_Digest_Update(sha1, salt, 48)) /* Salt (48 bytes) */ goto out; + if (!winpr_Digest_Update(sha1, salt1, 32)) /* Salt1 (32 bytes) */ goto out; + if (!winpr_Digest_Update(sha1, salt2, 32)) /* Salt2 (32 bytes) */ goto out; + if (!winpr_Digest_Final(sha1, sha1_digest, sizeof(sha1_digest))) goto out; /* SaltedHash(Salt, Input, Salt1, Salt2) = MD5(S + SHA1_Digest) */ if (!(md5 = winpr_Digest_New())) goto out; + /* Allow FIPS override for use of MD5 here, this is used for creating hashes of the premaster_secret and master_secret */ /* used for RDP licensing as described in MS-RDPELE. This is for RDP licensing packets */ /* which will already be encrypted under FIPS, so the use of MD5 here is not for sensitive data protection. */ if (!winpr_Digest_Init_Allow_FIPS(md5, WINPR_MD_MD5)) goto out; + if (!winpr_Digest_Update(md5, salt, 48)) /* Salt (48 bytes) */ goto out; + if (!winpr_Digest_Update(md5, sha1_digest, sizeof(sha1_digest))) /* SHA1_Digest */ goto out; + if (!winpr_Digest_Final(md5, output, WINPR_MD5_DIGEST_LENGTH)) goto out; @@ -218,14 +228,19 @@ BOOL security_md5_16_32_32(const BYTE* in0, const BYTE* in1, const BYTE* in2, BY if (!(md5 = winpr_Digest_New())) return FALSE; + if (!winpr_Digest_Init(md5, WINPR_MD_MD5)) goto out; + if (!winpr_Digest_Update(md5, in0, 16)) goto out; + if (!winpr_Digest_Update(md5, in1, 32)) goto out; + if (!winpr_Digest_Update(md5, in2, 32)) goto out; + if (!winpr_Digest_Final(md5, output, WINPR_MD5_DIGEST_LENGTH)) goto out; @@ -237,36 +252,37 @@ out: BOOL security_md5_16_32_32_Allow_FIPS(const BYTE* in0, const BYTE* in1, const BYTE* in2, BYTE* output) { - WINPR_DIGEST_CTX* md5 = NULL; - BOOL result = FALSE; - - if (!(md5 = winpr_Digest_New())) - return FALSE; - if (!winpr_Digest_Init_Allow_FIPS(md5, WINPR_MD_MD5)) - goto out; - if (!winpr_Digest_Update(md5, in0, 16)) - goto out; - if (!winpr_Digest_Update(md5, in1, 32)) - goto out; - if (!winpr_Digest_Update(md5, in2, 32)) - goto out; - if (!winpr_Digest_Final(md5, output, WINPR_MD5_DIGEST_LENGTH)) - goto out; - - result = TRUE; + WINPR_DIGEST_CTX* md5 = NULL; + BOOL result = FALSE; + + if (!(md5 = winpr_Digest_New())) + return FALSE; + if (!winpr_Digest_Init_Allow_FIPS(md5, WINPR_MD_MD5)) + goto out; + if (!winpr_Digest_Update(md5, in0, 16)) + goto out; + if (!winpr_Digest_Update(md5, in1, 32)) + goto out; + if (!winpr_Digest_Update(md5, in2, 32)) + goto out; + if (!winpr_Digest_Final(md5, output, WINPR_MD5_DIGEST_LENGTH)) + goto out; + + result = TRUE; out: - winpr_Digest_Free(md5); - return result; + winpr_Digest_Free(md5); + return result; } BOOL security_licensing_encryption_key(const BYTE* session_key_blob, const BYTE* client_random, - const BYTE* server_random, BYTE* output) + const BYTE* server_random, BYTE* output) { - /* LicensingEncryptionKey = MD5(Second128Bits(SessionKeyBlob) + ClientRandom + ServerRandom)) */ - /* Allow FIPS use of MD5 here, this is just used for creating the licensing encryption key as described in MS-RDPELE. */ - /* This is for RDP licensing packets which will already be encrypted under FIPS, so the use of MD5 here is not for - /* sensitive data protection. */ - return security_md5_16_32_32_Allow_FIPS(&session_key_blob[16], client_random, server_random, output); + /* LicensingEncryptionKey = MD5(Second128Bits(SessionKeyBlob) + ClientRandom + ServerRandom)) + * Allow FIPS use of MD5 here, this is just used for creating the licensing encryption key as described in MS-RDPELE. + * This is for RDP licensing packets which will already be encrypted under FIPS, so the use of MD5 here is not for + * sensitive data protection. */ + return security_md5_16_32_32_Allow_FIPS(&session_key_blob[16], client_random, server_random, + output); } void security_UINT32_le(BYTE* output, UINT32 value) @@ -285,41 +301,50 @@ BOOL security_mac_data(const BYTE* mac_salt_key, const BYTE* data, UINT32 length BYTE length_le[4]; BYTE sha1_digest[WINPR_SHA1_DIGEST_LENGTH]; BOOL result = FALSE; - /* MacData = MD5(MacSaltKey + pad2 + SHA1(MacSaltKey + pad1 + length + data)) */ - security_UINT32_le(length_le, length); /* length must be little-endian */ /* SHA1_Digest = SHA1(MacSaltKey + pad1 + length + data) */ if (!(sha1 = winpr_Digest_New())) goto out; + if (!winpr_Digest_Init(sha1, WINPR_MD_SHA1)) goto out; + if (!winpr_Digest_Update(sha1, mac_salt_key, 16)) /* MacSaltKey */ goto out; + if (!winpr_Digest_Update(sha1, pad1, sizeof(pad1))) /* pad1 */ goto out; + if (!winpr_Digest_Update(sha1, length_le, sizeof(length_le))) /* length */ goto out; + if (!winpr_Digest_Update(sha1, data, length)) /* data */ goto out; + if (!winpr_Digest_Final(sha1, sha1_digest, sizeof(sha1_digest))) goto out; /* MacData = MD5(MacSaltKey + pad2 + SHA1_Digest) */ if (!(md5 = winpr_Digest_New())) goto out; + /* Allow FIPS override for use of MD5 here, this is only used for creating the MACData field of the */ /* Client Platform Challenge Response packet (from MS-RDPELE section 2.2.2.5). This is for RDP licensing packets */ /* which will already be encrypted under FIPS, so the use of MD5 here is not for sensitive data protection. */ if (!winpr_Digest_Init_Allow_FIPS(md5, WINPR_MD_MD5)) goto out; + if (!winpr_Digest_Update(md5, mac_salt_key, 16)) /* MacSaltKey */ goto out; + if (!winpr_Digest_Update(md5, pad2, sizeof(pad2))) /* pad2 */ goto out; + if (!winpr_Digest_Update(md5, sha1_digest, sizeof(sha1_digest))) /* SHA1_Digest */ goto out; + if (!winpr_Digest_Final(md5, output, WINPR_MD5_DIGEST_LENGTH)) goto out; @@ -338,36 +363,46 @@ BOOL security_mac_signature(rdpRdp *rdp, const BYTE* data, UINT32 length, BYTE* BYTE md5_digest[WINPR_MD5_DIGEST_LENGTH]; BYTE sha1_digest[WINPR_SHA1_DIGEST_LENGTH]; BOOL result = FALSE; - security_UINT32_le(length_le, length); /* length must be little-endian */ /* SHA1_Digest = SHA1(MACKeyN + pad1 + length + data) */ if (!(sha1 = winpr_Digest_New())) goto out; + if (!winpr_Digest_Init(sha1, WINPR_MD_SHA1)) goto out; + if (!winpr_Digest_Update(sha1, rdp->sign_key, rdp->rc4_key_len)) /* MacKeyN */ goto out; + if (!winpr_Digest_Update(sha1, pad1, sizeof(pad1))) /* pad1 */ goto out; + if (!winpr_Digest_Update(sha1, length_le, sizeof(length_le))) /* length */ goto out; + if (!winpr_Digest_Update(sha1, data, length)) /* data */ goto out; + if (!winpr_Digest_Final(sha1, sha1_digest, sizeof(sha1_digest))) goto out; /* MACSignature = First64Bits(MD5(MACKeyN + pad2 + SHA1_Digest)) */ if (!(md5 = winpr_Digest_New())) goto out; + if (!winpr_Digest_Init(md5, WINPR_MD_MD5)) goto out; + if (!winpr_Digest_Update(md5, rdp->sign_key, rdp->rc4_key_len)) /* MacKeyN */ goto out; + if (!winpr_Digest_Update(md5, pad2, sizeof(pad2))) /* pad2 */ goto out; + if (!winpr_Digest_Update(md5, sha1_digest, sizeof(sha1_digest))) /* SHA1_Digest */ goto out; + if (!winpr_Digest_Final(md5, md5_digest, sizeof(md5_digest))) goto out; @@ -389,7 +424,6 @@ BOOL security_salted_mac_signature(rdpRdp *rdp, const BYTE* data, UINT32 length, BYTE md5_digest[WINPR_MD5_DIGEST_LENGTH]; BYTE sha1_digest[WINPR_SHA1_DIGEST_LENGTH]; BOOL result = FALSE; - security_UINT32_le(length_le, length); /* length must be little-endian */ if (encryption) @@ -408,32 +442,44 @@ BOOL security_salted_mac_signature(rdpRdp *rdp, const BYTE* data, UINT32 length, /* SHA1_Digest = SHA1(MACKeyN + pad1 + length + data) */ if (!(sha1 = winpr_Digest_New())) goto out; + if (!winpr_Digest_Init(sha1, WINPR_MD_SHA1)) goto out; + if (!winpr_Digest_Update(sha1, rdp->sign_key, rdp->rc4_key_len)) /* MacKeyN */ goto out; + if (!winpr_Digest_Update(sha1, pad1, sizeof(pad1))) /* pad1 */ goto out; + if (!winpr_Digest_Update(sha1, length_le, sizeof(length_le))) /* length */ goto out; + if (!winpr_Digest_Update(sha1, data, length)) /* data */ goto out; + if (!winpr_Digest_Update(sha1, use_count_le, sizeof(use_count_le))) /* encryptionCount */ goto out; + if (!winpr_Digest_Final(sha1, sha1_digest, sizeof(sha1_digest))) goto out; /* MACSignature = First64Bits(MD5(MACKeyN + pad2 + SHA1_Digest)) */ if (!(md5 = winpr_Digest_New())) goto out; + if (!winpr_Digest_Init(md5, WINPR_MD_MD5)) goto out; + if (!winpr_Digest_Update(md5, rdp->sign_key, rdp->rc4_key_len)) /* MacKeyN */ goto out; + if (!winpr_Digest_Update(md5, pad2, sizeof(pad2))) /* pad2 */ goto out; + if (!winpr_Digest_Update(md5, sha1_digest, sizeof(sha1_digest))) /* SHA1_Digest */ goto out; + if (!winpr_Digest_Final(md5, md5_digest, sizeof(md5_digest))) goto out; @@ -477,6 +523,7 @@ static void fips_expand_key_bits(BYTE* in, BYTE* out) { p = b / 8; r = b % 8; + if (r == 0) { out[i] = buf[p] & 0xfe; @@ -505,7 +552,6 @@ BOOL security_establish_keys(const BYTE* client_random, rdpRdp* rdp) BYTE salt[] = { 0xD1, 0x26, 0x9E }; /* 40 bits: 3 bytes, 56 bits: 1 byte */ rdpSettings* settings; BOOL status; - settings = rdp->settings; server_random = settings->ServerRandom; @@ -526,6 +572,7 @@ BOOL security_establish_keys(const BYTE* client_random, rdpRdp* rdp) winpr_Digest_Free(sha1); return FALSE; } + client_encrypt_key_t[20] = client_encrypt_key_t[0]; if (!winpr_Digest_Init(sha1, WINPR_MD_SHA1) || @@ -536,6 +583,7 @@ BOOL security_establish_keys(const BYTE* client_random, rdpRdp* rdp) winpr_Digest_Free(sha1); return FALSE; } + client_decrypt_key_t[20] = client_decrypt_key_t[0]; if (!winpr_Digest_Init(sha1, WINPR_MD_SHA1) || @@ -589,7 +637,6 @@ BOOL security_establish_keys(const BYTE* client_random, rdpRdp* rdp) if (!status) return FALSE; - if (settings->EncryptionMethods == ENCRYPTION_METHOD_40BIT) { memcpy(rdp->sign_key, salt, 3); @@ -615,7 +662,6 @@ BOOL security_establish_keys(const BYTE* client_random, rdpRdp* rdp) rdp->decrypt_checksum_use_count = 0; rdp->encrypt_use_count =0; rdp->encrypt_checksum_use_count =0; - return TRUE; } @@ -627,36 +673,47 @@ BOOL security_key_update(BYTE* key, BYTE* update_key, int key_len, rdpRdp* rdp) WINPR_RC4_CTX* rc4 = NULL; BYTE salt[] = { 0xD1, 0x26, 0x9E }; /* 40 bits: 3 bytes, 56 bits: 1 byte */ BOOL result = FALSE; - WLog_DBG(TAG, "updating RDP key"); + if (!(sha1 = winpr_Digest_New())) goto out; + if (!winpr_Digest_Init(sha1, WINPR_MD_SHA1)) goto out; + if (!winpr_Digest_Update(sha1, update_key, key_len)) goto out; + if (!winpr_Digest_Update(sha1, pad1, sizeof(pad1))) goto out; + if (!winpr_Digest_Update(sha1, key, key_len)) goto out; + if (!winpr_Digest_Final(sha1, sha1h, sizeof(sha1h))) goto out; if (!(md5 = winpr_Digest_New())) goto out; + if (!winpr_Digest_Init(md5, WINPR_MD_MD5)) goto out; + if (!winpr_Digest_Update(md5, update_key, key_len)) goto out; + if (!winpr_Digest_Update(md5, pad2, sizeof(pad2))) goto out; + if (!winpr_Digest_Update(md5, sha1h, sizeof(sha1h))) goto out; + if (!winpr_Digest_Final(md5, key, WINPR_MD5_DIGEST_LENGTH)) goto out; if (!(rc4 = winpr_RC4_New(key, key_len))) goto out; + if (!winpr_RC4_Update(rc4, key_len, key, key)) goto out; @@ -670,7 +727,6 @@ out: winpr_Digest_Free(sha1); winpr_Digest_Free(md5); winpr_RC4_Free(rc4); - return result; } @@ -683,6 +739,7 @@ BOOL security_encrypt(BYTE* data, size_t length, rdpRdp* rdp) winpr_RC4_Free(rdp->rc4_encrypt_key); rdp->rc4_encrypt_key = winpr_RC4_New(rdp->encrypt_key, rdp->rc4_key_len); + if (!rdp->rc4_encrypt_key) return FALSE; @@ -691,6 +748,7 @@ BOOL security_encrypt(BYTE* data, size_t length, rdpRdp* rdp) if (!winpr_RC4_Update(rdp->rc4_encrypt_key, length, data, data)) return FALSE; + rdp->encrypt_use_count++; rdp->encrypt_checksum_use_count++; return TRUE; @@ -705,16 +763,20 @@ BOOL security_decrypt(BYTE* data, size_t length, rdpRdp* rdp) { if (!security_key_update(rdp->decrypt_key, rdp->decrypt_update_key, rdp->rc4_key_len, rdp)) return FALSE; + winpr_RC4_Free(rdp->rc4_decrypt_key); rdp->rc4_decrypt_key = winpr_RC4_New(rdp->decrypt_key, rdp->rc4_key_len); + if (!rdp->rc4_decrypt_key) return FALSE; rdp->decrypt_use_count = 0; } + if (!winpr_RC4_Update(rdp->rc4_decrypt_key, length, data, data)) return FALSE; + rdp->decrypt_use_count += 1; rdp->decrypt_checksum_use_count++; return TRUE; @@ -726,17 +788,20 @@ BOOL security_hmac_signature(const BYTE* data, size_t length, BYTE* output, rdpR BYTE use_count_le[4]; WINPR_HMAC_CTX* hmac; BOOL result = FALSE; - security_UINT32_le(use_count_le, rdp->encrypt_use_count); if (!(hmac = winpr_HMAC_New())) return FALSE; + if (!winpr_HMAC_Init(hmac, WINPR_MD_SHA1, rdp->fips_sign_key, WINPR_SHA1_DIGEST_LENGTH)) goto out; + if (!winpr_HMAC_Update(hmac, data, length)) goto out; + if (!winpr_HMAC_Update(hmac, use_count_le, 4)) goto out; + if (!winpr_HMAC_Final(hmac, buf, WINPR_SHA1_DIGEST_LENGTH)) goto out; @@ -753,6 +818,7 @@ BOOL security_fips_encrypt(BYTE* data, size_t length, rdpRdp* rdp) if (!winpr_Cipher_Update(rdp->fips_encrypt, data, length, data, &olen)) return FALSE; + rdp->encrypt_use_count++; return TRUE; } @@ -763,6 +829,7 @@ BOOL security_fips_decrypt(BYTE* data, size_t length, rdpRdp* rdp) if (!winpr_Cipher_Update(rdp->fips_decrypt, data, length, data, &olen)) return FALSE; + return TRUE; } @@ -772,17 +839,20 @@ BOOL security_fips_check_signature(const BYTE* data, size_t length, const BYTE* BYTE use_count_le[4]; WINPR_HMAC_CTX* hmac; BOOL result = FALSE; - security_UINT32_le(use_count_le, rdp->decrypt_use_count); if (!(hmac = winpr_HMAC_New())) return FALSE; + if (!winpr_HMAC_Init(hmac, WINPR_MD_SHA1, rdp->fips_sign_key, WINPR_SHA1_DIGEST_LENGTH)) goto out; + if (!winpr_HMAC_Update(hmac, data, length)) goto out; + if (!winpr_HMAC_Update(hmac, use_count_le, 4)) goto out; + if (!winpr_HMAC_Final(hmac, buf, WINPR_SHA1_DIGEST_LENGTH)) goto out; @@ -790,6 +860,7 @@ BOOL security_fips_check_signature(const BYTE* data, size_t length, const BYTE* if (!memcmp(sig, buf, 8)) result = TRUE; + out: winpr_HMAC_Free(hmac); return result; diff --git a/winpr/libwinpr/crypto/cipher.c b/winpr/libwinpr/crypto/cipher.c index 78a6501..f1e4290 100644 --- a/winpr/libwinpr/crypto/cipher.c +++ b/winpr/libwinpr/crypto/cipher.c @@ -47,14 +47,15 @@ WINPR_RC4_CTX* winpr_RC4_New_Internal(const BYTE* key, size_t keylen, BOOL overr { WINPR_RC4_CTX* ctx = NULL; #if defined(WITH_OPENSSL) - EVP_CIPHER* evp = NULL; + const EVP_CIPHER* evp = NULL; #endif if (!key || (keylen == 0)) return NULL; #if defined(WITH_OPENSSL) - if (!(ctx = EVP_CIPHER_CTX_new())) + + if (!(ctx = (WINPR_RC4_CTX*) EVP_CIPHER_CTX_new())) return NULL; evp = EVP_rc4(); @@ -62,21 +63,22 @@ WINPR_RC4_CTX* winpr_RC4_New_Internal(const BYTE* key, size_t keylen, BOOL overr if (!evp) return NULL; - EVP_CIPHER_CTX_init((EVP_CIPHER_CTX *) ctx); - EVP_EncryptInit_ex((EVP_CIPHER_CTX *) ctx, evp, NULL, NULL, NULL); - + EVP_CIPHER_CTX_init((EVP_CIPHER_CTX*) ctx); + EVP_EncryptInit_ex((EVP_CIPHER_CTX*) ctx, evp, NULL, NULL, NULL); /* EVP_CIPH_FLAG_NON_FIPS_ALLOW does not exist before openssl 1.0.1 */ #if !(OPENSSL_VERSION_NUMBER < 0x10001000L) - if (override_fips == TRUE) - EVP_CIPHER_CTX_set_flags((EVP_CIPHER_CTX *) ctx, EVP_CIPH_FLAG_NON_FIPS_ALLOW); -#endif - EVP_CIPHER_CTX_set_key_length((EVP_CIPHER_CTX *) ctx, keylen); - EVP_EncryptInit_ex((EVP_CIPHER_CTX *) ctx, NULL, NULL, key, NULL); + if (override_fips == TRUE) + EVP_CIPHER_CTX_set_flags((EVP_CIPHER_CTX*) ctx, EVP_CIPH_FLAG_NON_FIPS_ALLOW); +#endif + EVP_CIPHER_CTX_set_key_length((EVP_CIPHER_CTX*) ctx, keylen); + EVP_EncryptInit_ex((EVP_CIPHER_CTX*) ctx, NULL, NULL, key, NULL); #elif defined(WITH_MBEDTLS) && defined(MBEDTLS_ARC4_C) + if (!(ctx = (WINPR_RC4_CTX*) calloc(1, sizeof(mbedtls_arc4_context)))) return NULL; + mbedtls_arc4_init((mbedtls_arc4_context*) ctx); mbedtls_arc4_setup((mbedtls_arc4_context*) ctx, key, (unsigned int) keylen); #endif @@ -97,12 +99,13 @@ BOOL winpr_RC4_Update(WINPR_RC4_CTX* ctx, size_t length, const BYTE* input, BYTE { #if defined(WITH_OPENSSL) int outputLength; - EVP_CipherUpdate((EVP_CIPHER_CTX *) ctx, output, &outputLength, input, length); + EVP_CipherUpdate((EVP_CIPHER_CTX*) ctx, output, &outputLength, input, length); return TRUE; - #elif defined(WITH_MBEDTLS) && defined(MBEDTLS_ARC4_C) + if (mbedtls_arc4_crypt((mbedtls_arc4_context*) ctx, length, input, output) == 0) return TRUE; + #endif return FALSE; } @@ -113,7 +116,7 @@ void winpr_RC4_Free(WINPR_RC4_CTX* ctx) return; #if defined(WITH_OPENSSL) - EVP_CIPHER_CTX_free((EVP_CIPHER_CTX *) ctx); + EVP_CIPHER_CTX_free((EVP_CIPHER_CTX*) ctx); #elif defined(WITH_MBEDTLS) && defined(MBEDTLS_ARC4_C) mbedtls_arc4_free((mbedtls_arc4_context*) ctx); free(ctx); @@ -546,7 +549,6 @@ mbedtls_cipher_type_t winpr_mbedtls_get_cipher_type(int cipher) WINPR_CIPHER_CTX* winpr_Cipher_New(int cipher, int op, const BYTE* key, const BYTE* iv) { WINPR_CIPHER_CTX* ctx = NULL; - #if defined(WITH_OPENSSL) int operation; const EVP_CIPHER* evp; @@ -567,9 +569,7 @@ WINPR_CIPHER_CTX* winpr_Cipher_New(int cipher, int op, const BYTE* key, const BY } EVP_CIPHER_CTX_set_padding(octx, 0); - ctx = (WINPR_CIPHER_CTX*) octx; - #elif defined(WITH_MBEDTLS) int key_bitlen; mbedtls_operation_t operation; @@ -588,7 +588,7 @@ WINPR_CIPHER_CTX* winpr_Cipher_New(int cipher, int op, const BYTE* key, const BY if (mbedtls_cipher_setup(mctx, cipher_info) != 0) { - free (mctx); + free(mctx); return NULL; } @@ -597,27 +597,27 @@ WINPR_CIPHER_CTX* winpr_Cipher_New(int cipher, int op, const BYTE* key, const BY if (mbedtls_cipher_setkey(mctx, key, key_bitlen, operation) != 0) { mbedtls_cipher_free(mctx); - free (mctx); + free(mctx); return NULL; } if (mbedtls_cipher_set_padding_mode(mctx, MBEDTLS_PADDING_NONE) != 0) { mbedtls_cipher_free(mctx); - free (mctx); + free(mctx); return NULL; } ctx = (WINPR_CIPHER_CTX*) mctx; #endif - return ctx; } -BOOL winpr_Cipher_Update(WINPR_CIPHER_CTX* ctx, const BYTE* input, size_t ilen, BYTE* output, size_t* olen) +BOOL winpr_Cipher_Update(WINPR_CIPHER_CTX* ctx, const BYTE* input, size_t ilen, BYTE* output, + size_t* olen) { #if defined(WITH_OPENSSL) - int outl = (int) *olen; + int outl = (int) * olen; if (EVP_CipherUpdate((EVP_CIPHER_CTX*) ctx, output, &outl, input, ilen) == 1) { @@ -626,17 +626,18 @@ BOOL winpr_Cipher_Update(WINPR_CIPHER_CTX* ctx, const BYTE* input, size_t ilen, } #elif defined(WITH_MBEDTLS) + if (mbedtls_cipher_update((mbedtls_cipher_context_t*) ctx, input, ilen, output, olen) == 0) return TRUE; -#endif +#endif return FALSE; } BOOL winpr_Cipher_Final(WINPR_CIPHER_CTX* ctx, BYTE* output, size_t* olen) { #if defined(WITH_OPENSSL) - int outl = (int) *olen; + int outl = (int) * olen; if (EVP_CipherFinal_ex((EVP_CIPHER_CTX*) ctx, output, &outl) == 1) { @@ -645,10 +646,11 @@ BOOL winpr_Cipher_Final(WINPR_CIPHER_CTX* ctx, BYTE* output, size_t* olen) } #elif defined(WITH_MBEDTLS) + if (mbedtls_cipher_finish((mbedtls_cipher_context_t*) ctx, output, olen) == 0) return TRUE; -#endif +#endif return FALSE; } @@ -670,13 +672,13 @@ void winpr_Cipher_Free(WINPR_CIPHER_CTX* ctx) * Key Generation */ -int winpr_Cipher_BytesToKey(int cipher, int md, const BYTE* salt, const BYTE* data, int datal, int count, BYTE* key, BYTE* iv) +int winpr_Cipher_BytesToKey(int cipher, int md, const BYTE* salt, const BYTE* data, int datal, + int count, BYTE* key, BYTE* iv) { /** * Key and IV generation compatible with OpenSSL EVP_BytesToKey(): * https://www.openssl.org/docs/manmaster/crypto/EVP_BytesToKey.html */ - #if defined(WITH_OPENSSL) const EVP_MD* evp_md; const EVP_CIPHER* evp_cipher; @@ -692,13 +694,10 @@ int winpr_Cipher_BytesToKey(int cipher, int md, const BYTE* salt, const BYTE* da const mbedtls_md_info_t* md_info; mbedtls_cipher_type_t cipher_type; const mbedtls_cipher_info_t* cipher_info; - mbedtls_md_type_t md_type = winpr_mbedtls_get_md_type(md); md_info = mbedtls_md_info_from_type(md_type); - cipher_type = winpr_mbedtls_get_cipher_type(cipher); cipher_info = mbedtls_cipher_info_from_type(cipher_type); - nkey = cipher_info->key_bitlen / 8; niv = cipher_info->iv_size; @@ -742,8 +741,10 @@ int winpr_Cipher_BytesToKey(int cipher, int md, const BYTE* salt, const BYTE* da { if (mbedtls_md_starts(&ctx) != 0) goto err; + if (mbedtls_md_update(&ctx, md_buf, mds) != 0) goto err; + if (mbedtls_md_finish(&ctx, md_buf) != 0) goto err; } @@ -756,10 +757,13 @@ int winpr_Cipher_BytesToKey(int cipher, int md, const BYTE* salt, const BYTE* da { if (nkey == 0) break; + if (i == mds) break; + if (key) *(key++) = md_buf[i]; + nkey--; i++; } @@ -771,10 +775,13 @@ int winpr_Cipher_BytesToKey(int cipher, int md, const BYTE* salt, const BYTE* da { if (niv == 0) break; + if (i == mds) break; + if (iv) *(iv++) = md_buf[i]; + niv--; i++; } @@ -790,6 +797,5 @@ err: SecureZeroMemory(md_buf, 64); return rv; #endif - return 0; }