Fix #4247: warnings introduced with #3904

diff --git a/libfreerdp/core/certificate.c b/libfreerdp/core/certificate.c
index 482f6d4..70b1084 100644 (file)
--- 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 (file)
--- 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 (file)
--- 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 (file)
--- 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;
 }