search

Mbed-TLS / mbedtls

An open source, portable, easy to use, readable and flexible TLS library, and reference implementation of the PSA Cryptography API. Releases are on a varying cadence, typically around 3 - 6 months between releases.
https://www.trustedfirmware.org/projects/mbed-tls/
Other
5.51k stars 2.6k forks source link

Multiple warnings in ssl_tls, bignum etc. (Conversions, sign mismatch) #1576

Open jepaan opened 6 years ago

jepaan commented 6 years ago

Description

Bug

OS
windows

mbed TLS build:
Version: git commit 4ca9a45
OS version: Windows 7 Configuration: No modifications Compiler and options: Visual Studio 2015 (v140) with warning level 4 (/W4) Additional environment information:

Peer device TLS stack and version
OpenSSL|GnuTls|Chrome|NSS(Firefox)|SecureChannel (IIS/Internet Explorer/Edge)|Other
Version:

Expected behaviour
Project compiles with no warnings

Actual behaviour
mbedtls\library\x509_create.c(177): warning C4244: '=': conversion from 'int' to 'unsigned char', possible loss of data mbedtls\library\ssl_tls.c(1411): warning C4244: '=': conversion from 'int' to 'unsigned char', possible loss of data mbedtls\library\ssl_tls.c(1692): warning C4244: '=': conversion from 'int' to 'unsigned char', possible loss of data mbedtls\library\ssl_tls.c(2556): warning C4244: '=': conversion from 'int' to 'unsigned char', possible loss of data mbedtls\library\ssl_tls.c(6352): warning C4244: '=': conversion from 'int' to 'unsigned char', possible loss of data mbedtls\library\ssl_tls.c(6353): warning C4244: '=': conversion from 'int' to 'unsigned char', possible loss of data mbedtls\library\ssl_tls.c(6358): warning C4244: '=': conversion from 'int' to 'unsigned char', possible loss of data mbedtls\library\ssl_tls.c(6359): warning C4244: '=': conversion from 'int' to 'unsigned char', possible loss of data mbedtls\library\ssl_cli.c(467): warning C4244: '=': conversion from 'const unsigned int' to 'unsigned char', possible loss of data mbedtls\library\ssl_cli.c(926): warning C4310: cast truncates constant value mbedtls\library\x509_crl.c(575): warning C4701: potentially uninitialized local variable 'use_len' used mbedtls\library\rsa.c(2228): warning C4244: '=': conversion from 'int' to 'unsigned char', possible loss of data mbedtls\library\net_sockets.c(476): warning C4389: '==': signed/unsigned mismatch mbedtls\library\net_sockets.c(483): warning C4389: '==': signed/unsigned mismatch mbedtls\library\net_sockets.c(582): warning C4389: '==': signed/unsigned mismatch mbedtls\library\md.c(459): warning C4244: 'return': conversion from 'const int' to 'unsigned char', possible loss of data mbedtls\library\ecp_curves.c(1317): warning C4127: conditional expression is constant mbedtls\library\ecp.c(493): warning C4244: '=': conversion from 'int' to 'unsigned char', possible loss of data mbedtls\library\ecp.c(1649): warning C4244: '=': conversion from 'int' to 'unsigned char', possible loss of data mbedtls\library\ctr_drbg.c(186): warning C4244: '=': conversion from 'int' to 'unsigned char', possible loss of data mbedtls\library\blowfish.c(79): warning C4132: 'S': const object should be initialized mbedtls\library\bignum.c(1260): warning C4245: '=': conversion from 'int' to 'mbedtls_mpi_uint', signed/unsigned mismatch mbedtls\library\bignum.c(1262): warning C4245: 'return': conversion from 'int' to 'mbedtls_mpi_uint', signed/unsigned mismatch mbedtls\library\bignum.c(1388): warning C4245: '=': conversion from 'int' to 'mbedtls_mpi_uint', signed/unsigned mismatch mbedtls\library\asn1write.c(243): warning C4244: '=': conversion from 'int' to 'unsigned char', possible loss of data

Related to: https://github.com/ARMmbed/mbedtls/issues/1367

I would be happy to contribute to this, but I'm unsure what the preferred solution is. Members of structs such as mbedtls_ssl_context are declared as int but treated as unsigned char.

RonEld commented 6 years ago

@jepaan Thank you for raising this issue! We are happy to receive contributions from the community. You are welcome to make your contribution, as long as you follow our coding standards, and sign the Contributor License Agreement, as you think is appropriate. We will work with you if we see any issues. Note that modifying the members in mbedtls_ssl_context is breaking the ABI, so it is less preferred, unless there is no other way. When I tried reproducing this issue, most of the warnings I received were onditional expression is constant Haven't you received such warnings?

ciarmcom commented 6 years ago

ARM Internal Ref: IOTSSL-2263

jepaan commented 6 years ago

@RonEld I would be happy to contribute. However I'm unsure exactly how.

For instance in this case: mbedtls\library\asn1write.c(243): warning C4244: '=': conversion from 'int' to 'unsigned char', possible loss of data

This warning is related to a comment a few lines above: // TODO negative values and values larger than 128 // DER format assumes 2s complement for numbers, so the leftmost bit // should be 0 for positive numbers and 1 for negative numbers. //

Will you accept a commit which casts away the warning, thus hiding the TODO, or are you only interested in a commit which implements the TODO?

simonbutcher commented 6 years ago

I think there's a pre-existing PR to address these issues with PR #858.