From 68187b9f023964f996fd9b4dd9919fac02a7781e Mon Sep 17 00:00:00 2001 From: "John W. Bruce" Date: Mon, 27 Sep 2021 18:17:04 -0700 Subject: [PATCH] Fix -Wshorten-64-to-32 errors in BoringSSL interactions (This is a merge from the Widevine Repo of http://go/wvgerrit/134310.) This patch fixes code that would trigger -Wshorten-64-to-32 by implicitly narrowing a variable from 64 to 32 bits. Most of the time, it does this by making the implicit conversion explicit. The cause of most of these is that OpenSSL uses "int" for the length of things rather than size_t. (While BoringSSL sometimes uses int and sometimes uses size_t.) One exception is LogBoringSSLError(). We have a couple copies of this function around, and they varied slightly. This patch brings them all in-line, which conveniently also removes any code in them that would deal with integer variables. GetRandBytes() now takes a size_t and downcasts to BoringSSL's native int internally, so that callers can pass in a size_t value as they would expect. There's also an interesting case in oec_session_util.cpp. Because BoringSSL and OpenSSL disagree about the width of an error code, we have to use the "auto" type for a temporary variable that holds an error, in order to retain compatibility with both. Bug: 194971260 Test: x86-64 Test: x86-64-openssl Change-Id: I88bc62b4cda396f8a1eabd1a3cb7d1b03f47a33f --- .../cdm/core/src/privacy_crypto_boringssl.cpp | 17 +++++----- libwvdrmengine/cdm/core/test/http_socket.cpp | 13 ++++---- libwvdrmengine/cdm/core/test/test_base.cpp | 2 +- .../oemcrypto/test/oec_key_deriver.cpp | 13 +++++--- .../oemcrypto/test/oec_session_util.cpp | 32 +++++++++++-------- .../oemcrypto/test/oec_session_util.h | 6 ++-- 6 files changed, 47 insertions(+), 36 deletions(-) diff --git a/libwvdrmengine/cdm/core/src/privacy_crypto_boringssl.cpp b/libwvdrmengine/cdm/core/src/privacy_crypto_boringssl.cpp index 13156209..522a8837 100644 --- a/libwvdrmengine/cdm/core/src/privacy_crypto_boringssl.cpp +++ b/libwvdrmengine/cdm/core/src/privacy_crypto_boringssl.cpp @@ -29,7 +29,7 @@ const int kRsaPkcs1OaepPaddingLength = 41; RSA* GetKey(const std::string& serialized_key) { BIO* bio = BIO_new_mem_buf(const_cast(serialized_key.data()), - serialized_key.size()); + static_cast(serialized_key.size())); if (bio == nullptr) { LOGE("BIO_new_mem_buf failed: returned null"); return nullptr; @@ -123,11 +123,11 @@ bool AesCbcKey::Encrypt(const std::string& in, std::string* out, } out->resize(in.size() + AES_BLOCK_SIZE); - int out_length = out->size(); + int out_length = static_cast(out->size()); if (EVP_EncryptUpdate( evp_cipher_ctx, reinterpret_cast(&(*out)[0]), &out_length, reinterpret_cast(const_cast(in.data())), - in.size()) == 0) { + static_cast(in.size())) == 0) { LOGE("AES CBC encryption failure: %s", ERR_error_string(ERR_get_error(), nullptr)); EVP_CIPHER_CTX_free(evp_cipher_ctx); @@ -195,7 +195,7 @@ bool RsaPublicKey::Encrypt(const std::string& clear_message, encrypted_message->assign(rsa_size, 0); if (RSA_public_encrypt( - clear_message.size(), + static_cast(clear_message.size()), const_cast( reinterpret_cast(clear_message.data())), reinterpret_cast(&(*encrypted_message)[0]), key, @@ -212,9 +212,9 @@ bool RsaPublicKey::Encrypt(const std::string& clear_message, // LogBoringSSLError is a callback from BoringSSL which is called with each // error in the thread's error queue. -static int LogBoringSSLError(const char* msg, size_t /* len */, - void* /* ctx */) { - LOGE(" %s", msg); +static int LogBoringSSLError(const char* msg, size_t /* length */, + void* /* user_data */) { + LOGE(" BoringSSL Error: %s", msg); return 1; } @@ -345,7 +345,8 @@ bool ExtractExtensionValueFromCertificate(const std::string& cert, return false; } - X509* const intermediate_cert = sk_X509_value(certs, cert_index); + X509* const intermediate_cert = + sk_X509_value(certs, static_cast(cert_index)); if (intermediate_cert == nullptr) { LOGE("Unable to get intermediate cert"); return false; diff --git a/libwvdrmengine/cdm/core/test/http_socket.cpp b/libwvdrmengine/cdm/core/test/http_socket.cpp index 0bce6275..886ec687 100644 --- a/libwvdrmengine/cdm/core/test/http_socket.cpp +++ b/libwvdrmengine/cdm/core/test/http_socket.cpp @@ -66,10 +66,10 @@ SSL_CTX* InitSslContext() { return ctx; } -static int LogBoringSslError(const char* message, size_t length, +static int LogBoringSslError(const char* message, size_t /* length */, void* /* user_data */) { LOGE(" BoringSSL Error: %s", message); - return length; + return 1; } bool IsRetryableSslError(int ssl_error) { @@ -433,7 +433,7 @@ int HttpSocket::Read(char* data, int len, int timeout_in_ms) { if (secure_connect_) { read = SSL_read(ssl_, data, to_read); } else { - read = recv(socket_fd_, data, to_read, 0); + read = static_cast(recv(socket_fd_, data, to_read, 0)); } if (read > 0) { @@ -490,10 +490,11 @@ int HttpSocket::Write(const char* data, int len, int timeout_in_ms) { } while (to_send > 0) { int sent; - if (secure_connect_) + if (secure_connect_) { sent = SSL_write(ssl_, data, to_send); - else - sent = send(socket_fd_, data, to_send, 0); + } else { + sent = static_cast(send(socket_fd_, data, to_send, 0)); + } if (sent > 0) { to_send -= sent; diff --git a/libwvdrmengine/cdm/core/test/test_base.cpp b/libwvdrmengine/cdm/core/test/test_base.cpp index a256c42e..86100bf5 100644 --- a/libwvdrmengine/cdm/core/test/test_base.cpp +++ b/libwvdrmengine/cdm/core/test/test_base.cpp @@ -199,7 +199,7 @@ std::string WvCdmTestBase::SignHMAC(const std::string& message, const std::vector& key) { uint8_t signature[SHA256_DIGEST_LENGTH]; unsigned int md_len = SHA256_DIGEST_LENGTH; - HMAC(EVP_sha256(), &key[0], key.size(), + HMAC(EVP_sha256(), &key[0], static_cast(key.size()), reinterpret_cast(message.data()), message.size(), signature, &md_len); std::string result(signature, signature + SHA256_DIGEST_LENGTH); diff --git a/libwvdrmengine/oemcrypto/test/oec_key_deriver.cpp b/libwvdrmengine/oemcrypto/test/oec_key_deriver.cpp index f6abdb6a..17415478 100644 --- a/libwvdrmengine/oemcrypto/test/oec_key_deriver.cpp +++ b/libwvdrmengine/oemcrypto/test/oec_key_deriver.cpp @@ -147,8 +147,9 @@ void KeyDeriver::ServerSignBuffer(const uint8_t* data, size_t data_length, ASSERT_EQ(mac_key_server_.size(), MAC_KEY_SIZE); signature->assign(SHA256_DIGEST_LENGTH, 0); unsigned int sig_len = SHA256_DIGEST_LENGTH; - ASSERT_TRUE(HMAC(EVP_sha256(), mac_key_server_.data(), mac_key_server_.size(), - data, data_length, signature->data(), &sig_len)); + ASSERT_TRUE(HMAC(EVP_sha256(), mac_key_server_.data(), + static_cast(mac_key_server_.size()), data, data_length, + signature->data(), &sig_len)); } void KeyDeriver::ClientSignBuffer(const vector& buffer, @@ -156,8 +157,9 @@ void KeyDeriver::ClientSignBuffer(const vector& buffer, ASSERT_EQ(mac_key_client_.size(), MAC_KEY_SIZE); signature->assign(SHA256_DIGEST_LENGTH, 0); unsigned int sig_len = SHA256_DIGEST_LENGTH; - ASSERT_TRUE(HMAC(EVP_sha256(), mac_key_client_.data(), mac_key_client_.size(), - buffer.data(), buffer.size(), signature->data(), &sig_len)); + ASSERT_TRUE(HMAC(EVP_sha256(), mac_key_client_.data(), + static_cast(mac_key_client_.size()), buffer.data(), + buffer.size(), signature->data(), &sig_len)); } void KeyDeriver::ClientSignPstReport(const vector& pst_report_buffer, @@ -165,7 +167,8 @@ void KeyDeriver::ClientSignPstReport(const vector& pst_report_buffer, ASSERT_EQ(mac_key_client_.size(), MAC_KEY_SIZE); signature->assign(SHA_DIGEST_LENGTH, 0); unsigned int sig_len = SHA_DIGEST_LENGTH; - ASSERT_TRUE(HMAC(EVP_sha1(), mac_key_client_.data(), mac_key_client_.size(), + ASSERT_TRUE(HMAC(EVP_sha1(), mac_key_client_.data(), + static_cast(mac_key_client_.size()), &pst_report_buffer[SHA_DIGEST_LENGTH], pst_report_buffer.size() - SHA_DIGEST_LENGTH, signature->data(), &sig_len)); diff --git a/libwvdrmengine/oemcrypto/test/oec_session_util.cpp b/libwvdrmengine/oemcrypto/test/oec_session_util.cpp index 6a3b85cd..a5775ea3 100644 --- a/libwvdrmengine/oemcrypto/test/oec_session_util.cpp +++ b/libwvdrmengine/oemcrypto/test/oec_session_util.cpp @@ -112,9 +112,9 @@ OEMCryptoResult DecryptCTR(OEMCrypto_SESSION session_id, const uint8_t* key, } // namespace -int GetRandBytes(unsigned char* buf, int num) { +int GetRandBytes(unsigned char* buf, size_t num) { // returns 1 on success, -1 if not supported, or 0 if other failure. - return RAND_bytes(buf, num); + return RAND_bytes(buf, static_cast(num)); } // Does the boilerplate to fill out sample and subsample descriptions for @@ -162,7 +162,9 @@ void ctr128_inc64(int64_t increaseBy, uint8_t* iv) { uint32_t htonl_fnc(uint32_t x) { return htonl(x); } void dump_boringssl_error() { - while (unsigned long err = ERR_get_error()) { + // BoringSSL and OpenSSL disagree about what the type of an error code is, so + // we must use "auto" here. + while (auto err = ERR_get_error()) { char buffer[120]; ERR_error_string_n(err, buffer, sizeof(buffer)); cout << "BoringSSL Error -- " << buffer << "\n"; @@ -1373,7 +1375,8 @@ void Session::GenerateDerivedKeysFromSessionKey() { } void Session::TestDecryptCTR(bool select_key_first, - OEMCryptoResult expected_result, int key_index) { + OEMCryptoResult expected_result, + size_t key_index) { OEMCryptoResult select_result = OEMCrypto_SUCCESS; if (select_key_first) { // Select the key (from FillSimpleMessage) @@ -1433,7 +1436,7 @@ void Session::TestDecryptResult(OEMCryptoResult expected_result, } } -void Session::TestSelectExpired(unsigned int key_index) { +void Session::TestSelectExpired(size_t key_index) { if (global_features.api_version >= 13) { OEMCryptoResult status = OEMCrypto_SelectKey( session_id(), license().keys[key_index].key_id, @@ -1477,7 +1480,7 @@ void Session::LoadOEMCert(bool verify_cert) { // Load the public cert's key into public_rsa_ and verify, if requested for (size_t i = 0; certs && i < static_cast(sk_X509_num(certs)); ++i) { - X509* x509_cert = sk_X509_value(certs, i); + X509* x509_cert = sk_X509_value(certs, static_cast(i)); boringssl_ptr pubkey(X509_get_pubkey(x509_cert)); ASSERT_TRUE(pubkey.NotNull()); if (i == 0) { @@ -1493,7 +1496,8 @@ void Session::LoadOEMCert(bool verify_cert) { X509_NAME* name = X509_get_subject_name(x509_cert); printf(" OEM Certificate Name: %s\n", - X509_NAME_oneline(name, buffer.data(), buffer.size())); + X509_NAME_oneline(name, buffer.data(), + static_cast(buffer.size()))); boringssl_ptr store(X509_STORE_new()); ASSERT_TRUE(store.NotNull()); boringssl_ptr store_ctx( @@ -1523,7 +1527,8 @@ void Session::PreparePublicKey(const uint8_t* rsa_key, size_t rsa_key_length) { rsa_key_length = sizeof(kTestRSAPKCS8PrivateKeyInfo2_2048); } uint8_t* p = const_cast(rsa_key); - boringssl_ptr bio(BIO_new_mem_buf(p, rsa_key_length)); + boringssl_ptr bio( + BIO_new_mem_buf(p, static_cast(rsa_key_length))); ASSERT_TRUE(bio.NotNull()); boringssl_ptr pkcs8_pki( d2i_PKCS8_PRIV_KEY_INFO_bio(bio.get(), nullptr)); @@ -1621,8 +1626,9 @@ void Session::VerifyRSASignature(const vector& message, int size; // RSA_public_decrypt decrypts the signature, and then verifies that // it was padded with RSA PKCS1 padding. - size = RSA_public_decrypt(signature_length, signature, padded_digest.data(), - public_rsa_, RSA_PKCS1_PADDING); + size = RSA_public_decrypt(static_cast(signature_length), signature, + padded_digest.data(), public_rsa_, + RSA_PKCS1_PADDING); EXPECT_GT(size, 0); padded_digest.resize(size); EXPECT_EQ(message, padded_digest); @@ -1639,9 +1645,9 @@ bool Session::GenerateRSASessionKey(vector* session_key, } *session_key = wvcdm::a2b_hex("6fa479c731d2770b6a61a5d1420bb9d1"); enc_session_key->assign(RSA_size(public_rsa_), 0); - int status = RSA_public_encrypt(session_key->size(), &(session_key->front()), - &(enc_session_key->front()), public_rsa_, - RSA_PKCS1_OAEP_PADDING); + int status = RSA_public_encrypt( + static_cast(session_key->size()), &(session_key->front()), + &(enc_session_key->front()), public_rsa_, RSA_PKCS1_OAEP_PADDING); int size = static_cast(RSA_size(public_rsa_)); if (status != size) { cout << "GenerateRSASessionKey error encrypting session key.\n"; diff --git a/libwvdrmengine/oemcrypto/test/oec_session_util.h b/libwvdrmengine/oemcrypto/test/oec_session_util.h index d2bde5e8..49716bf9 100644 --- a/libwvdrmengine/oemcrypto/test/oec_session_util.h +++ b/libwvdrmengine/oemcrypto/test/oec_session_util.h @@ -117,7 +117,7 @@ struct EntitledContentKeyData { }; // returns 1 on success, -1 if not supported, or 0 if other failure. -int GetRandBytes(unsigned char* buf, int num); +int GetRandBytes(unsigned char* buf, size_t num); void GenerateSimpleSampleDescription(const std::vector& in, std::vector& out, @@ -534,10 +534,10 @@ class Session { // Encrypt some data and pass to OEMCrypto_DecryptCENC to verify decryption. void TestDecryptCTR(bool select_key_first = true, OEMCryptoResult expected_result = OEMCrypto_SUCCESS, - int key_index = 0); + size_t key_index = 0); // Verify that an attempt to select an expired key either succeeds, or gives // an actionable error code. - void TestSelectExpired(unsigned int key_index); + void TestSelectExpired(size_t key_index); // Calls OEMCrypto_GetOEMPublicCertificate and OEMCrypto_LoadOEMPrivateKey and // loads the OEM cert's public rsa key into public_rsa_. void LoadOEMCert(bool verify_cert = false);