From: Frank Lichtenheld <[email protected]> Using the defines directly avoids the issue with sign conversion by letting the compiler deduce the types of the literals.
I do not think that using an intermediate variable increases the readability. Change-Id: I3bcb4cb8e17ff80a5cb8acdaf18d1072159184e5 Signed-off-by: Frank Lichtenheld <[email protected]> Acked-by: Razvan Cojocaru <[email protected]> Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/2158 --- This change was reviewed on Gerrit and approved by at least one developer. I request to merge it to master. Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/2158 This mail reflects revision 5 of this Change. Acked-by according to Gerrit (reflected above): Razvan Cojocaru <[email protected]> diff --git a/src/openvpn/crypto.c b/src/openvpn/crypto.c index a4c2683..6955760 100644 --- a/src/openvpn/crypto.c +++ b/src/openvpn/crypto.c @@ -81,7 +81,6 @@ const struct key_ctx *ctx = &opt->key_ctx_bi.encrypt; uint8_t *mac_out = NULL; - const int mac_len = OPENVPN_AEAD_TAG_LENGTH; /* IV, packet-ID and implicit IV required for this mode. */ ASSERT(ctx->cipher); @@ -142,7 +141,7 @@ dmsg(D_PACKET_CONTENT, "ENCRYPT FROM: %s", format_hex(BPTR(buf), BLENZ(buf), 80, &gc)); /* Buffer overflow check */ - if (!buf_safe(&work, buf->len + mac_len + cipher_ctx_block_size(ctx->cipher))) + if (!buf_safe(&work, BLENZ(buf) + OPENVPN_AEAD_TAG_LENGTH + cipher_ctx_block_size(ctx->cipher))) { msg(D_CRYPT_ERRORS, "ENCRYPT: buffer size error, bc=%d bo=%d bl=%d wc=%d wo=%d wl=%d", buf->capacity, buf->offset, buf->len, work.capacity, work.offset, work.len); @@ -156,7 +155,7 @@ if (!use_epoch_data_format) { /* Reserve space for authentication tag */ - mac_out = buf_write_alloc(&work, mac_len); + mac_out = buf_write_alloc(&work, OPENVPN_AEAD_TAG_LENGTH); ASSERT(mac_out); } @@ -177,12 +176,12 @@ if (use_epoch_data_format) { /* Reserve space for authentication tag */ - mac_out = buf_write_alloc(&work, mac_len); + mac_out = buf_write_alloc(&work, OPENVPN_AEAD_TAG_LENGTH); ASSERT(mac_out); } /* Write authentication tag */ - ASSERT(cipher_ctx_get_tag(ctx->cipher, mac_out, mac_len)); + ASSERT(cipher_ctx_get_tag(ctx->cipher, mac_out, OPENVPN_AEAD_TAG_LENGTH)); *buf = work; @@ -453,9 +452,6 @@ CRYPT_DROP("Decryption failed verification limit reached."); } - const int tag_size = OPENVPN_AEAD_TAG_LENGTH; - - ASSERT(frame); ASSERT(buf->len > 0); ASSERT(ctx->cipher); @@ -470,14 +466,14 @@ ASSERT(packet_id_initialized(&opt->packet_id)); /* Ensure that the packet size is long enough */ - int min_packet_len = packet_id_size(false) + tag_size + 1; + size_t min_packet_len = packet_id_size(false) + OPENVPN_AEAD_TAG_LENGTH + 1; if (use_epoch_data_format) { min_packet_len += sizeof(uint32_t); } - if (buf->len < min_packet_len) + if (BLENZ(buf) < min_packet_len) { CRYPT_ERROR("missing IV info, missing tag or no payload"); } @@ -540,24 +536,24 @@ } } - const int ad_size = (int)(BPTR(buf) - ad_start); + const uintptr_t ad_size = (uintptr_t)(BPTR(buf) - ad_start); uint8_t *tag_ptr = NULL; int data_len = 0; if (use_epoch_data_format) { - data_len = BLEN(buf) - tag_size; + data_len = BLEN(buf) - OPENVPN_AEAD_TAG_LENGTH; tag_ptr = BPTR(buf) + data_len; } else { tag_ptr = BPTR(buf); - ASSERT(buf_advance(buf, tag_size)); + ASSERT(buf_advance(buf, OPENVPN_AEAD_TAG_LENGTH)); data_len = BLEN(buf); } - dmsg(D_PACKET_CONTENT, "DECRYPT MAC: %s", format_hex(tag_ptr, tag_size, 0, &gc)); + dmsg(D_PACKET_CONTENT, "DECRYPT MAC: %s", format_hex(tag_ptr, OPENVPN_AEAD_TAG_LENGTH, 0, &gc)); dmsg(D_PACKET_CONTENT, "DECRYPT FROM: %s", format_hex(BPTR(buf), BLENZ(buf), 0, &gc)); /* Buffer overflow check (should never fail) */ @@ -567,7 +563,7 @@ } /* feed in tag and the authenticated data */ - ASSERT(cipher_ctx_update_ad(ctx->cipher, ad_start, ad_size)); + ASSERT(cipher_ctx_update_ad(ctx->cipher, ad_start, (int)ad_size)); dmsg(D_PACKET_CONTENT, "DECRYPT AD: %s", format_hex(ad_start, ad_size, 0, &gc)); /* Decrypt and authenticate packet */ @@ -578,7 +574,7 @@ } ASSERT(buf_inc_len(&work, outlen)); - if (!cipher_ctx_final_check_tag(ctx->cipher, BPTR(&work) + outlen, &outlen, tag_ptr, tag_size)) + if (!cipher_ctx_final_check_tag(ctx->cipher, BPTR(&work) + outlen, &outlen, tag_ptr, OPENVPN_AEAD_TAG_LENGTH)) { ctx->failed_verifications++; CRYPT_DROP("packet tag authentication failed"); @@ -1118,20 +1114,19 @@ static void generate_key_random(struct key *key) { - int cipher_len = MAX_CIPHER_KEY_LENGTH; - int hmac_len = MAX_HMAC_KEY_LENGTH; - struct gc_arena gc = gc_new(); CLEAR(*key); - if (!rand_bytes(key->cipher, cipher_len) || !rand_bytes(key->hmac, hmac_len)) + if (!rand_bytes(key->cipher, MAX_CIPHER_KEY_LENGTH) + || !rand_bytes(key->hmac, MAX_HMAC_KEY_LENGTH)) { msg(M_FATAL, "ERROR: Random number generator cannot obtain entropy for key generation"); } dmsg(D_SHOW_KEY_SOURCE, "Cipher source entropy: %s", - format_hex(key->cipher, cipher_len, 0, &gc)); - dmsg(D_SHOW_KEY_SOURCE, "HMAC source entropy: %s", format_hex(key->hmac, hmac_len, 0, &gc)); + format_hex(key->cipher, MAX_CIPHER_KEY_LENGTH, 0, &gc)); + dmsg(D_SHOW_KEY_SOURCE, "HMAC source entropy: %s", + format_hex(key->hmac, MAX_HMAC_KEY_LENGTH, 0, &gc)); gc_free(&gc); } diff --git a/src/openvpn/crypto_epoch.c b/src/openvpn/crypto_epoch.c index 243b5a2..40e8cec 100644 --- a/src/openvpn/crypto_epoch.c +++ b/src/openvpn/crypto_epoch.c @@ -38,11 +38,6 @@ #include "buffer.h" #include "integer.h" -#if defined(__GNUC__) || defined(__clang__) -#pragma GCC diagnostic push -#pragma GCC diagnostic ignored "-Wsign-conversion" -#endif - void ovpn_hkdf_expand(const uint8_t *secret, const uint8_t *info, size_t info_len, uint8_t *out, size_t out_len) @@ -51,26 +46,23 @@ hmac_ctx_init(hmac_ctx, secret, "SHA256"); ASSERT(info_len <= INT_MAX); - const unsigned int digest_size = SHA256_DIGEST_LENGTH; /* T(0) = empty string */ uint8_t t_prev[SHA256_DIGEST_LENGTH]; - unsigned int t_prev_len = 0; - for (uint8_t block = 1; (block - 1) * digest_size < out_len; block++) + for (uint8_t block = 1; (block - 1u) * SHA256_DIGEST_LENGTH < out_len; block++) { hmac_ctx_reset(hmac_ctx); /* calculate T(block) */ - hmac_ctx_update(hmac_ctx, t_prev, t_prev_len); + hmac_ctx_update(hmac_ctx, t_prev, block == 1 ? 0 : SHA256_DIGEST_LENGTH); hmac_ctx_update(hmac_ctx, info, (int)info_len); hmac_ctx_update(hmac_ctx, &block, 1); hmac_ctx_final(hmac_ctx, t_prev); - t_prev_len = digest_size; /* Copy a full hmac output or remaining bytes */ - size_t out_offset = (block - 1) * digest_size; - size_t copylen = min_size(digest_size, out_len - out_offset); + size_t out_offset = (block - 1u) * SHA256_DIGEST_LENGTH; + size_t copylen = min_size(SHA256_DIGEST_LENGTH, out_len - out_offset); memcpy(out + out_offset, t_prev, copylen); } @@ -463,7 +455,3 @@ epoch_iterate_send_key(opt); } } - -#if defined(__GNUC__) || defined(__clang__) -#pragma GCC diagnostic pop -#endif diff --git a/src/openvpn/packet_id.h b/src/openvpn/packet_id.h index 48752d3..782dd58 100644 --- a/src/openvpn/packet_id.h +++ b/src/openvpn/packet_id.h @@ -312,7 +312,7 @@ const char *packet_id_net_print(const struct packet_id_net *pin, bool print_timestamp, struct gc_arena *gc); -static inline int +static inline unsigned int packet_id_size(bool long_form) { return sizeof(packet_id_type) + (long_form ? sizeof(net_time_t) : 0); diff --git a/src/openvpn/tls_crypt.c b/src/openvpn/tls_crypt.c index 2dadc4f..7eb83ce 100644 --- a/src/openvpn/tls_crypt.c +++ b/src/openvpn/tls_crypt.c @@ -52,7 +52,7 @@ return create_kt("AES-256-CTR", "SHA256", "tls-crypt"); } -int +unsigned int tls_crypt_buf_overhead(void) { return packet_id_size(true) + TLS_CRYPT_TAG_SIZE + TLS_CRYPT_BLOCK_SIZE; diff --git a/src/openvpn/tls_crypt.h b/src/openvpn/tls_crypt.h index d179cbb..8e5567f 100644 --- a/src/openvpn/tls_crypt.h +++ b/src/openvpn/tls_crypt.h @@ -130,7 +130,7 @@ * Returns the maximum overhead (in bytes) added to the destination buffer by * tls_crypt_wrap(). */ -int tls_crypt_buf_overhead(void); +unsigned int tls_crypt_buf_overhead(void); /** * Wrap a control channel packet (both authenticates and encrypts the data). diff --git a/tests/unit_tests/openvpn/test_tls_crypt.c b/tests/unit_tests/openvpn/test_tls_crypt.c index 2d31c00..00de73a 100644 --- a/tests/unit_tests/openvpn/test_tls_crypt.c +++ b/tests/unit_tests/openvpn/test_tls_crypt.c @@ -232,11 +232,6 @@ } } -#if defined(__GNUC__) || defined(__clang__) -#pragma GCC diagnostic push -#pragma GCC diagnostic ignored "-Wsign-conversion" -#endif - /** * Check that short messages are successfully wrapped-and-unwrapped. */ @@ -778,10 +773,6 @@ tls_wrap_free(&wrap_ctx); } -#if defined(__GNUC__) || defined(__clang__) -#pragma GCC diagnostic pop -#endif - static void test_tls_crypt_v2_write_server_key_file(void **state) { _______________________________________________ Openvpn-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/openvpn-devel
