Attention is currently required from: plaisthos.
Hello plaisthos,
I'd like you to do a code review.
Please visit
http://gerrit.openvpn.net/c/openvpn/+/1559?usp=email
to review the following change.
Change subject: ssl_mbedtls: Avoid conversion and sign-compare warnings
......................................................................
ssl_mbedtls: Avoid conversion and sign-compare warnings
Change-Id: I777a3a5f4f137432a19746972e2aad1732184feb
Signed-off-by: Frank Lichtenheld <[email protected]>
---
M src/openvpn/ssl_mbedtls.c
M src/openvpn/ssl_verify_mbedtls.c
2 files changed, 17 insertions(+), 31 deletions(-)
git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/59/1559/1
diff --git a/src/openvpn/ssl_mbedtls.c b/src/openvpn/ssl_mbedtls.c
index a9506ef..edc2ba9 100644
--- a/src/openvpn/ssl_mbedtls.c
+++ b/src/openvpn/ssl_mbedtls.c
@@ -624,12 +624,6 @@
return 0;
}
-#if defined(__GNUC__) || defined(__clang__)
-#pragma GCC diagnostic push
-#pragma GCC diagnostic ignored "-Wconversion"
-#pragma GCC diagnostic ignored "-Wsign-compare"
-#endif
-
#if MBEDTLS_VERSION_NUMBER < 0x04000000
/**
* external_pkcs1_sign implements a mbed TLS rsa_sign_func callback, that uses
@@ -656,7 +650,6 @@
{
struct external_context *const ctx = ctx_voidptr;
int rv;
- uint8_t *to_sign = NULL;
size_t asn_len = 0, oid_size = 0;
const char *oid = NULL;
@@ -688,11 +681,13 @@
asn_len = 10 + oid_size;
}
- if ((SIZE_MAX - hashlen) < asn_len || ctx->signature_length < (asn_len +
hashlen))
+ if (ctx->signature_length < (asn_len + hashlen)
+ || (asn_len + hashlen) > UINT8_MAX)
{
return MBEDTLS_ERR_RSA_BAD_INPUT_DATA;
}
+ uint8_t *to_sign = NULL;
ALLOC_ARRAY_CLEAR(to_sign, uint8_t, asn_len + hashlen);
uint8_t *p = to_sign;
if (md_alg != MBEDTLS_MD_NONE)
@@ -707,20 +702,20 @@
* Digest ::= OCTET STRING
*/
*p++ = MBEDTLS_ASN1_SEQUENCE | MBEDTLS_ASN1_CONSTRUCTED;
- *p++ = (unsigned char)(0x08 + oid_size + hashlen);
+ *p++ = (uint8_t)(0x08 + oid_size + hashlen);
*p++ = MBEDTLS_ASN1_SEQUENCE | MBEDTLS_ASN1_CONSTRUCTED;
- *p++ = (unsigned char)(0x04 + oid_size);
+ *p++ = (uint8_t)(0x04 + oid_size);
*p++ = MBEDTLS_ASN1_OID;
- *p++ = oid_size & 0xFF;
+ *p++ = (uint8_t)oid_size;
memcpy(p, oid, oid_size);
p += oid_size;
*p++ = MBEDTLS_ASN1_NULL;
*p++ = 0x00;
*p++ = MBEDTLS_ASN1_OCTET_STRING;
- *p++ = hashlen;
+ *p++ = (uint8_t)hashlen;
/* Double-check ASN length */
- ASSERT(asn_len == p - to_sign);
+ ASSERT(asn_len == (uintptr_t)(p - to_sign));
}
/* Copy the hash to be signed */
@@ -810,7 +805,7 @@
goto cleanup;
}
- if (openvpn_base64_decode(dst_b64, dst, (int)dst_len) != dst_len)
+ if (openvpn_base64_decode(dst_b64, dst, (int)dst_len) != (int)dst_len)
{
goto cleanup;
}
@@ -923,6 +918,8 @@
{
size_t read_len = 0;
+ ASSERT(out_len <= INT_MAX);
+
if (in->first_block == NULL)
{
return MBEDTLS_ERR_SSL_WANT_READ;
@@ -930,7 +927,7 @@
while (in->first_block != NULL && read_len < out_len)
{
- int block_len = in->first_block->length - in->data_start;
+ size_t block_len = in->first_block->length - in->data_start;
if (block_len <= out_len - read_len)
{
buffer_entry *cur_entry = in->first_block;
@@ -956,7 +953,7 @@
}
}
- return read_len;
+ return (int)read_len;
}
static int
@@ -975,6 +972,8 @@
return MBEDTLS_ERR_NET_SEND_FAILED;
}
+ ASSERT(len <= INT_MAX);
+
new_block->length = len;
new_block->next_block = NULL;
@@ -992,7 +991,7 @@
out->last_block = new_block;
- return len;
+ return (int)len;
}
static int
@@ -1012,7 +1011,7 @@
static void
my_debug(void *ctx, int level, const char *file, int line, const char *str)
{
- int my_loglevel = (level < 3) ? D_TLS_DEBUG_MED : D_TLS_DEBUG;
+ unsigned int my_loglevel = (level < 3) ? D_TLS_DEBUG_MED : D_TLS_DEBUG;
msg(my_loglevel, "mbed TLS msg (%s:%d): %s", file, line, str);
}
@@ -1048,10 +1047,6 @@
#endif /* MBEDTLS_VERSION_NUMBER < 0x040000 */
}
-#if defined(__GNUC__) || defined(__clang__)
-#pragma GCC diagnostic pop
-#endif
-
int
tls_version_max(void)
{
diff --git a/src/openvpn/ssl_verify_mbedtls.c b/src/openvpn/ssl_verify_mbedtls.c
index 7495085..ad5479c 100644
--- a/src/openvpn/ssl_verify_mbedtls.c
+++ b/src/openvpn/ssl_verify_mbedtls.c
@@ -565,11 +565,6 @@
gc_free(&gc);
}
-#if defined(__GNUC__) || defined(__clang__)
-#pragma GCC diagnostic push
-#pragma GCC diagnostic ignored "-Wsign-compare"
-#endif
-
/*
* Save X509 fields to environment, using the naming convention:
*
@@ -678,10 +673,6 @@
return fFound;
}
-#if defined(__GNUC__) || defined(__clang__)
-#pragma GCC diagnostic pop
-#endif
-
result_t
x509_verify_cert_eku(mbedtls_x509_crt *cert, const char *const expected_oid)
{
--
To view, visit http://gerrit.openvpn.net/c/openvpn/+/1559?usp=email
To unsubscribe, or for help writing mail filters, visit
http://gerrit.openvpn.net/settings?usp=email
Gerrit-MessageType: newchange
Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: I777a3a5f4f137432a19746972e2aad1732184feb
Gerrit-Change-Number: 1559
Gerrit-PatchSet: 1
Gerrit-Owner: flichtenheld <[email protected]>
Gerrit-Reviewer: plaisthos <[email protected]>
Gerrit-CC: openvpn-devel <[email protected]>
Gerrit-Attention: plaisthos <[email protected]>
_______________________________________________
Openvpn-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
