Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

gmtime_r and mbedtls_time on Windows #1198

Merged
merged 4 commits into from
Jul 24, 2018
Merged

gmtime_r and mbedtls_time on Windows #1198

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
512b4ee
Use gmtime_r to fix thread-safety issue, and use mbedtls_time on Windows
NWilson Dec 5, 2017
2682edf
Fix build using -std=c99
NWilson Dec 5, 2017
61fa436
Address review comments - tidy usage of macros to use minimal values
NWilson Jun 25, 2018
a5fbfd7
Enable snprintf on FreeBSD
k-stachowiak Jul 6, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions ChangeLog
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,13 @@
mbed TLS ChangeLog (Sorted per branch, date)

= mbed TLS x.x.x branch released xxxx-xx-xx

Changes
* Allow overriding the time on Windows via the platform-time abstraction.
Fixed by Nick Wilson.
* Use gmtime_r/gmtime_s for thread-safety. Fixed by Nick Wilson.
* Fix build using -std=c99. Fixed by Nick Wilson.

= mbed TLS 2.11.0 branch released 2018-06-18

Features
Expand Down
3 changes: 0 additions & 3 deletions include/mbedtls/threading.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,9 +99,6 @@ extern int (*mbedtls_mutex_unlock)( mbedtls_threading_mutex_t *mutex );
#if defined(MBEDTLS_FS_IO)
extern mbedtls_threading_mutex_t mbedtls_threading_readdir_mutex;
#endif
#if defined(MBEDTLS_HAVE_TIME_DATE)
extern mbedtls_threading_mutex_t mbedtls_threading_gmtime_mutex;
#endif
#endif /* MBEDTLS_THREADING_C */

#ifdef __cplusplus
Expand Down
5 changes: 5 additions & 0 deletions library/entropy_poll.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,11 @@
* This file is part of mbed TLS (https://tls.mbed.org)
*/

#if defined(__linux__)
/* Ensure that syscall() is available even when compiling with -std=c99 */
#define _GNU_SOURCE
#endif

#if !defined(MBEDTLS_CONFIG_FILE)
#include "mbedtls/config.h"
#else
Expand Down
5 changes: 5 additions & 0 deletions library/net_sockets.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,11 @@
* This file is part of mbed TLS (https://tls.mbed.org)
*/

/* Enable definition of getaddrinfo() even when compiling with -std=c99. Must
* be set before config.h, which pulls in glibc's features.h indirectly.
* Harmless on other platforms. */
#define _POSIX_C_SOURCE 200112L

#if !defined(MBEDTLS_CONFIG_FILE)
#include "mbedtls/config.h"
#else
Expand Down
9 changes: 0 additions & 9 deletions library/threading.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -114,9 +114,6 @@ void mbedtls_threading_set_alt( void (*mutex_init)( mbedtls_threading_mutex_t *
#if defined(MBEDTLS_FS_IO)
mbedtls_mutex_init( &mbedtls_threading_readdir_mutex );
#endif
#if defined(MBEDTLS_HAVE_TIME_DATE)
mbedtls_mutex_init( &mbedtls_threading_gmtime_mutex );
#endif
}

/*
Expand All @@ -127,9 +124,6 @@ void mbedtls_threading_free_alt( void )
#if defined(MBEDTLS_FS_IO)
mbedtls_mutex_free( &mbedtls_threading_readdir_mutex );
#endif
#if defined(MBEDTLS_HAVE_TIME_DATE)
mbedtls_mutex_free( &mbedtls_threading_gmtime_mutex );
#endif
}
#endif /* MBEDTLS_THREADING_ALT */

Expand All @@ -142,8 +136,5 @@ void mbedtls_threading_free_alt( void )
#if defined(MBEDTLS_FS_IO)
mbedtls_threading_mutex_t mbedtls_threading_readdir_mutex MUTEX_INIT;
#endif
#if defined(MBEDTLS_HAVE_TIME_DATE)
mbedtls_threading_mutex_t mbedtls_threading_gmtime_mutex MUTEX_INIT;
#endif

#endif /* MBEDTLS_THREADING_C */
46 changes: 11 additions & 35 deletions library/x509.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,10 @@
* http://www.itu.int/ITU-T/studygroups/com17/languages/X.690-0207.pdf
*/

/* Ensure gmtime_r is available even with -std=c99; must be included before
* config.h, which pulls in glibc's features.h. Harmless on other platforms. */
#define _POSIX_C_SOURCE 200112L

#if !defined(MBEDTLS_CONFIG_FILE)
#include "mbedtls/config.h"
#else
Expand Down Expand Up @@ -59,14 +63,10 @@
#define mbedtls_snprintf snprintf
#endif


#if defined(MBEDTLS_HAVE_TIME)
#include "mbedtls/platform_time.h"
#endif

#if defined(_WIN32) && !defined(EFIX64) && !defined(EFI32)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change makes the code a lot cleaner, however it will prevent us from backporting it as it definitely breaks WinCE compatibility. If you whish this PR to be backported to 2.7 and 2.1 versions please withdraw this change. Otherwise it is good.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm happy with no backporting, so I've left this change in. It's not an urgent fix.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is a functional change to a feature, and is more an improvement than a bug fix, so no backport is necessary in my view.

#include <windows.h>
#else
#if defined(MBEDTLS_HAVE_TIME_DATE)
#include <time.h>
#endif

Expand Down Expand Up @@ -903,36 +903,18 @@ int mbedtls_x509_key_size_helper( char *buf, size_t buf_size, const char *name )
* Set the time structure to the current time.
* Return 0 on success, non-zero on failure.
*/
#if defined(_WIN32) && !defined(EFIX64) && !defined(EFI32)
static int x509_get_current_time( mbedtls_x509_time *now )
{
SYSTEMTIME st;

GetSystemTime( &st );

now->year = st.wYear;
now->mon = st.wMonth;
now->day = st.wDay;
now->hour = st.wHour;
now->min = st.wMinute;
now->sec = st.wSecond;

return( 0 );
}
#else
static int x509_get_current_time( mbedtls_x509_time *now )
{
struct tm *lt;
struct tm *lt, tm_buf;
mbedtls_time_t tt;
int ret = 0;

#if defined(MBEDTLS_THREADING_C)
if( mbedtls_mutex_lock( &mbedtls_threading_gmtime_mutex ) != 0 )
return( MBEDTLS_ERR_THREADING_MUTEX_ERROR );
#endif

tt = mbedtls_time( NULL );
lt = gmtime( &tt );
#if defined(_WIN32) && !defined(EFIX64) && !defined(EFI32)
lt = gmtime_s( &tm_buf, &tt ) == 0 ? &tm_buf : NULL;
#else
lt = gmtime_r( &tt, &tm_buf );
#endif

if( lt == NULL )
ret = -1;
Expand All @@ -946,14 +928,8 @@ static int x509_get_current_time( mbedtls_x509_time *now )
now->sec = lt->tm_sec;
}

#if defined(MBEDTLS_THREADING_C)
if( mbedtls_mutex_unlock( &mbedtls_threading_gmtime_mutex ) != 0 )
return( MBEDTLS_ERR_THREADING_MUTEX_ERROR );
#endif

return( ret );
}
#endif /* _WIN32 && !EFIX64 && !EFI32 */

/*
* Return 0 if before <= after, 1 otherwise
Expand Down
5 changes: 5 additions & 0 deletions programs/aes/aescrypt2.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,11 @@
* This file is part of mbed TLS (https://tls.mbed.org)
*/

/* Enable definition of fileno() even when compiling with -std=c99. Must be
* set before config.h, which pulls in glibc's features.h indirectly.
* Harmless on other platforms. */
#define _POSIX_C_SOURCE 1

#if !defined(MBEDTLS_CONFIG_FILE)
#include "mbedtls/config.h"
#else
Expand Down
5 changes: 5 additions & 0 deletions programs/aes/crypt_and_hash.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,11 @@
* This file is part of mbed TLS (https://tls.mbed.org)
*/

/* Enable definition of fileno() even when compiling with -std=c99. Must be
* set before config.h, which pulls in glibc's features.h indirectly.
* Harmless on other platforms. */
#define _POSIX_C_SOURCE 1

#if !defined(MBEDTLS_CONFIG_FILE)
#include "mbedtls/config.h"
#else
Expand Down
5 changes: 5 additions & 0 deletions programs/ssl/ssl_mail_client.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,11 @@
* This file is part of mbed TLS (https://tls.mbed.org)
*/

/* Enable definition of gethostname() even when compiling with -std=c99. Must
* be set before config.h, which pulls in glibc's features.h indirectly.
* Harmless on other platforms. */
#define _POSIX_C_SOURCE 200112L

#if !defined(MBEDTLS_CONFIG_FILE)
#include "mbedtls/config.h"
#else
Expand Down
5 changes: 5 additions & 0 deletions tests/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,11 @@ if(NOT PERL_FOUND)
message(FATAL_ERROR "Cannot build test suites without Perl")
endif()

# Enable definition of various functions used throughout the testsuite
# (gethostname, strdup, fileno...) even when compiling with -std=c99. Harmless
# on non-POSIX platforms.
add_definitions("-D_POSIX_C_SOURCE=200809L")

function(add_test_suite suite_name)
if(ARGV1)
set(data_name ${ARGV1})
Expand Down
5 changes: 5 additions & 0 deletions tests/Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,11 @@ LOCAL_LDFLAGS = -L../library \
-lmbedx509$(SHARED_SUFFIX) \
-lmbedcrypto$(SHARED_SUFFIX)

# Enable definition of various functions used throughout the testsuite
# (gethostname, strdup, fileno...) even when compiling with -std=c99. Harmless
# on non-POSIX platforms.
LOCAL_CFLAGS += -D_POSIX_C_SOURCE=200809L

ifndef SHARED
DEP=../library/libmbedcrypto.a ../library/libmbedx509.a ../library/libmbedtls.a
else
Expand Down
1 change: 1 addition & 0 deletions tests/suites/helpers.function
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ typedef UINT32 uint32_t;

#if defined(__unix__) || (defined(__APPLE__) && defined(__MACH__))
#include <unistd.h>
#include <strings.h>
#endif

/*----------------------------------------------------------------------------*/
Expand Down