From ba22ae8c6dac89b5e3fa07511f508e8b3efcd8dd Mon Sep 17 00:00:00 2001 From: Vladislav Vaintroub Date: Tue, 28 Mar 2017 21:56:34 +0000 Subject: [PATCH] Fix schannel and other socket io bugs on Windows. - Fix a breaking change where SP_PROT_TLS1_2_CLIENT bit was set in Cred.grbitEnabledProtocols by default. This makes any SSL connection to Windows server fai, because all yassl-based servers, in all MariaDB versions would abort a client that is trying to use TLS1.2 (This is covered MDEV-12190) As a consequence, client on Windows would not connect to any server on Windows. For compatibility reasons therefore, 1.2 should NOT be used by clients by default, otherwise it will break applications connectivity to both Oracle MySQL and MariaDB's yassl based servers. This also holds after MDEV-12190 is fixed, because older servers will be used for a while. - Cred.dwFlag was missing SCH_CRED_NO_DEFAULT_CREDS flag, which resulted in a popup on the build servers, asking to insert a smartcard during client SSL handshake. Smaller bugs in schannel : cipher_name() was returning pointer to member of stack allocated struct Socket IO fixes - errno rather than WSAGetLastError() was tested on Windows in pvio_socket_write/pvio_socket_read. Fixed by using socker_errno and simplided function to avoid numerious #ifdef _WIN32 - simplified vio_set_blocking, it had a rather inefficient implementation setting the same flags with ioctlsocket/fcntl over and over again. --- include/ma_global.h | 2 +- libmariadb/secure/schannel.c | 29 ++------ plugins/pvio/pvio_socket.c | 138 +++++++++++++++++++---------------- 3 files changed, 82 insertions(+), 87 deletions(-) diff --git a/include/ma_global.h b/include/ma_global.h index 5c48f89a0..576a318bb 100644 --- a/include/ma_global.h +++ b/include/ma_global.h @@ -697,7 +697,7 @@ typedef off_t os_off_t; #if defined(_WIN32) #define socket_errno WSAGetLastError() #define SOCKET_EINTR WSAEINTR -#define SOCKET_EAGAIN WSAEINPROGRESS +#define SOCKET_EAGAIN WSAEWOULDBLOCK #define SOCKET_ENFILE ENFILE #define SOCKET_EMFILE EMFILE #define SOCKET_EWOULDBLOCK WSAEWOULDBLOCK diff --git a/libmariadb/secure/schannel.c b/libmariadb/secure/schannel.c index 731cc2474..e9292bdbb 100644 --- a/libmariadb/secure/schannel.c +++ b/libmariadb/secure/schannel.c @@ -59,14 +59,12 @@ cipher_map[] = "TLS_RSA_WITH_RC4_128_SHA", "RC4-SHA", { CALG_RSA_KEYX, CALG_RC4, CALG_SHA1, CALG_RSA_SIGN } }, -#ifdef MARIADB_UNSECURE { 0x000A, PROT_SSL3, "TLS_RSA_WITH_3DES_EDE_CBC_SHA", "DES-CBC3-SHA", {CALG_RSA_KEYX, CALG_3DES, CALG_SHA1, CALG_DSS_SIGN} }, -#endif { 0x0013, PROT_TLS1_0 | PROT_TLS1_2 | PROT_SSL3, @@ -293,7 +291,6 @@ static size_t set_cipher(char * cipher_str, DWORD protocol, ALG_ID *arr , size_t my_bool ma_tls_connect(MARIADB_TLS *ctls) { - my_bool blocking; MYSQL *mysql; SCHANNEL_CRED Cred; MARIADB_PVIO *pvio; @@ -309,10 +306,6 @@ my_bool ma_tls_connect(MARIADB_TLS *ctls) pvio= ctls->pvio; sctx= (SC_CTX *)ctls->ssl; - /* Set socket to blocking if not already set */ - if (!(blocking= pvio->methods->is_blocking(pvio))) - pvio->methods->blocking(pvio, TRUE, 0); - mysql= pvio->mysql; if (ma_tls_set_client_certs(ctls)) @@ -349,19 +342,11 @@ my_bool ma_tls_connect(MARIADB_TLS *ctls) } Cred.dwVersion= SCHANNEL_CRED_VERSION; - Cred.dwMaximumCipherStrength = 0; - if (mysql->options.extension) - { - Cred.dwMinimumCipherStrength = 0; - } - Cred.dwFlags |= SCH_CRED_NO_SERVERNAME_CHECK | - SCH_CRED_MANUAL_CRED_VALIDATION | - SCH_USE_STRONG_CRYPTO; + Cred.dwFlags = SCH_CRED_NO_SERVERNAME_CHECK | SCH_CRED_NO_DEFAULT_CREDS | SCH_CRED_MANUAL_CRED_VALIDATION; if (sctx->client_cert_ctx) { - Cred.dwFlags |= SCH_CRED_NO_DEFAULT_CREDS; Cred.cCreds = 1; Cred.paCred = &sctx->client_cert_ctx; } @@ -375,9 +360,7 @@ my_bool ma_tls_connect(MARIADB_TLS *ctls) Cred.grbitEnabledProtocols|= SP_PROT_TLS1_2_CLIENT; } if (!Cred.grbitEnabledProtocols) - Cred.grbitEnabledProtocols= SP_PROT_TLS1_0_CLIENT | - SP_PROT_TLS1_1_CLIENT | - SP_PROT_TLS1_2_CLIENT; + Cred.grbitEnabledProtocols = SP_PROT_TLS1_0_CLIENT | SP_PROT_TLS1_1_CLIENT; if ((sRet= AcquireCredentialsHandleA(NULL, UNISP_NAME_A, SECPKG_CRED_OUTBOUND, NULL, &Cred, NULL, NULL, &sctx->CredHdl, NULL)) != SEC_E_OK) @@ -522,16 +505,16 @@ int ma_tls_verify_server_cert(MARIADB_TLS *ctls) return rc; } -static const char *cipher_name(SecPkgContext_CipherInfo CipherInfo) +static const char *cipher_name(const SecPkgContext_CipherInfo *CipherInfo) { int i; for(i = 0; i < sizeof(cipher_map)/sizeof(cipher_map[0]) ; i++) { - if (CipherInfo.dwCipherSuite == cipher_map[i].cipher_id) + if (CipherInfo->dwCipherSuite == cipher_map[i].cipher_id) return cipher_map[i].openssl_name; } - return CipherInfo.szCipherSuite; + return ""; }; const char *ma_tls_get_cipher(MARIADB_TLS *ctls) @@ -550,7 +533,7 @@ const char *ma_tls_get_cipher(MARIADB_TLS *ctls) if (sRet != SEC_E_OK) return NULL; - return cipher_name(CipherInfo); + return cipher_name(&CipherInfo); } unsigned int ma_tls_get_finger_print(MARIADB_TLS *ctls, char *fp, unsigned int len) diff --git a/plugins/pvio/pvio_socket.c b/plugins/pvio/pvio_socket.c index f51eea2c0..b8a25d7b8 100644 --- a/plugins/pvio/pvio_socket.c +++ b/plugins/pvio/pvio_socket.c @@ -52,9 +52,12 @@ #include #include #include +#define IS_SOCKET_EINTR(err) (err == SOCKET_EINTR) #else #include #define O_NONBLOCK 1 +#define MSG_DONTWAIT 0 +#define IS_SOCKET_EINTR(err) 0 #endif #ifndef SOCKET_ERROR @@ -63,6 +66,16 @@ #define DNS_TIMEOUT 30 +#ifndef O_NONBLOCK +#if defined(O_NDELAY) +#define O_NONBLOCK O_NODELAY +#elif defined (O_FNDELAY) +#define O_NONBLOCK O_FNDELAY +#else +#error socket blocking is not supported on this platform +#endif +#endif + /* Function prototypes */ my_bool pvio_socket_set_timeout(MARIADB_PVIO *pvio, enum enum_pvio_timeout type, int timeout); @@ -88,6 +101,8 @@ static int pvio_socket_init(char *unused1, int unused3, va_list); static int pvio_socket_end(void); +static ssize_t ma_send(my_socket socket, const uchar *buffer, size_t length, int flags); +static ssize_t ma_recv(my_socket socket, uchar *buffer, size_t length, int flags); struct st_ma_pvio_methods pvio_socket_methods= { pvio_socket_set_timeout, @@ -265,20 +280,26 @@ int pvio_socket_get_timeout(MARIADB_PVIO *pvio, enum enum_pvio_timeout type) */ ssize_t pvio_socket_read(MARIADB_PVIO *pvio, uchar *buffer, size_t length) { - ssize_t r= -1; - int read_flags= 0; - struct st_pvio_socket *csock= NULL; + ssize_t r; + int read_flags= MSG_DONTWAIT; + struct st_pvio_socket *csock; + int timeout; if (!pvio || !pvio->data) return -1; csock= (struct st_pvio_socket *)pvio->data; + timeout = pvio->timeout[PVIO_READ_TIMEOUT]; - if (pvio_socket_wait_io_or_timeout(pvio, TRUE, pvio->timeout[PVIO_READ_TIMEOUT]) < 1) - return -1; - do { - r= recv(csock->socket, (void *)buffer, length, read_flags); - } while (r == -1 && errno == EINTR); + while ((r = ma_recv(csock->socket, (void *)buffer, length, read_flags)) == -1) + { + int err = socket_errno; + if ((err != SOCKET_EAGAIN && err != SOCKET_EWOULDBLOCK) || timeout == 0) + return r; + + if (pvio_socket_wait_io_or_timeout(pvio, TRUE, timeout) < 1) + return -1; + } return r; } /* }}} */ @@ -328,22 +349,33 @@ ssize_t pvio_socket_async_read(MARIADB_PVIO *pvio, uchar *buffer, size_t length) } /* }}} */ -#ifndef _WIN32 -ssize_t ma_send(int socket, const uchar *buffer, size_t length, int flags) +static ssize_t ma_send(my_socket socket, const uchar *buffer, size_t length, int flags) { ssize_t r; -#if !defined(MSG_NOSIGNAL) && !defined(SO_NOSIGPIPE) +#if !defined(MSG_NOSIGNAL) && !defined(SO_NOSIGPIPE) && !defined(_WIN32) struct sigaction act, oldact; act.sa_handler= SIG_IGN; sigaction(SIGPIPE, &act, &oldact); #endif - r= send(socket, buffer, length, flags); -#if !defined(MSG_NOSIGNAL) && !defined(SO_NOSIGPIPE) + do { + r = send(socket, buffer, IF_WIN((int)length,length), flags); + } + while (r == -1 && IS_SOCKET_EINTR(socket_errno)); +#if !defined(MSG_NOSIGNAL) && !defined(SO_NOSIGPIPE) && !defined(_WIN32) sigaction(SIGPIPE, &oldact, NULL); #endif return r; } -#endif + +static ssize_t ma_recv(my_socket socket, uchar *buffer, size_t length, int flags) +{ + ssize_t r; + do { + r = recv(socket, buffer, IF_WIN((int)length, length), flags); + } + while (r == -1 && IS_SOCKET_EINTR(socket_errno)); + return r; +} /* {{{ pvio_socket_async_write */ /* @@ -416,41 +448,27 @@ ssize_t pvio_socket_async_write(MARIADB_PVIO *pvio, const uchar *buffer, size_t */ ssize_t pvio_socket_write(MARIADB_PVIO *pvio, const uchar *buffer, size_t length) { - ssize_t r= -1; - struct st_pvio_socket *csock= NULL; -#ifndef _WIN32 + ssize_t r; + struct st_pvio_socket *csock; + int timeout; int send_flags= MSG_DONTWAIT; #ifdef MSG_NOSIGNAL send_flags|= MSG_NOSIGNAL; -#endif #endif if (!pvio || !pvio->data) return -1; csock= (struct st_pvio_socket *)pvio->data; + timeout = pvio->timeout[PVIO_WRITE_TIMEOUT]; - do { -#ifndef _WIN32 - r = ma_send(csock->socket, buffer, length, send_flags); -#else - r = send(csock->socket, buffer, length, 0); -#endif - } while (r == -1 && errno == EINTR); - - while (r == -1 && (errno == EAGAIN || errno == EWOULDBLOCK) && - pvio->timeout[PVIO_WRITE_TIMEOUT] != 0) + while ((r = ma_send(csock->socket, (void *)buffer, length,send_flags)) == -1) { - if (pvio_socket_wait_io_or_timeout(pvio, FALSE, pvio->timeout[PVIO_WRITE_TIMEOUT]) < 1) + int err = socket_errno; + if ((err != SOCKET_EAGAIN && err != SOCKET_EWOULDBLOCK)|| timeout == 0) + return r; + if (pvio_socket_wait_io_or_timeout(pvio, FALSE, timeout) < 1) return -1; - do { -#ifndef _WIN32 - r= ma_send(csock->socket, buffer, length, send_flags); -#else - r = send(socket, buffer, length, 0); -#endif - } while (r == -1 && errno == EINTR); } - return r; } /* }}} */ @@ -508,6 +526,7 @@ int pvio_socket_wait_io_or_timeout(MARIADB_PVIO *pvio, my_bool is_read, int time else if (rc == 0) { rc= SOCKET_ERROR; + WSASetLastError(WSAETIMEDOUT); errno= ETIMEDOUT; } else if (FD_ISSET(csock->socket, &exc_fds)) @@ -516,6 +535,7 @@ int pvio_socket_wait_io_or_timeout(MARIADB_PVIO *pvio, my_bool is_read, int time int len = sizeof(int); if (getsockopt(csock->socket, SOL_SOCKET, SO_ERROR, (char *)&err, &len) != SOCKET_ERROR) { + WSASetLastError(err); errno= err; } rc= SOCKET_ERROR; @@ -528,50 +548,42 @@ int pvio_socket_wait_io_or_timeout(MARIADB_PVIO *pvio, my_bool is_read, int time my_bool pvio_socket_blocking(MARIADB_PVIO *pvio, my_bool block, my_bool *previous_mode) { - int *sd_flags, save_flags; - my_bool tmp; - struct st_pvio_socket *csock= NULL; + my_bool is_blocking; + struct st_pvio_socket *csock; + int new_fcntl_mode; if (!pvio || !pvio->data) return 1; - csock= (struct st_pvio_socket *)pvio->data; - sd_flags= &csock->fcntl_mode; - save_flags= csock->fcntl_mode; + csock = (struct st_pvio_socket *)pvio->data; - if (!previous_mode) - previous_mode= &tmp; + is_blocking = (csock->fcntl_mode & O_NONBLOCK)?1:0; + if (previous_mode) + *previous_mode = is_blocking; + + if (is_blocking == block) + return 0; + + if (block) + new_fcntl_mode = csock->fcntl_mode | O_NONBLOCK; + else + new_fcntl_mode = csock->fcntl_mode & ~O_NONBLOCK; #ifdef _WIN32 - *previous_mode= (*sd_flags & O_NONBLOCK) != 0; - *sd_flags = (block) ? *sd_flags & ~O_NONBLOCK : *sd_flags | O_NONBLOCK; { - ulong arg= 1 - block; + ulong arg = block ? 0 : 1; if (ioctlsocket(csock->socket, FIONBIO, (void *)&arg)) { - csock->fcntl_mode= save_flags; return(WSAGetLastError()); } } #else -#if defined(O_NONBLOCK) - *previous_mode= (*sd_flags & O_NONBLOCK) != 0; - *sd_flags = (block) ? *sd_flags & ~O_NONBLOCK : *sd_flags | O_NONBLOCK; -#elif defined(O_NDELAY) - *previous_mode= (*sd_flags & O_NODELAY) != 0; - *sd_flags = (block) ? *sd_flags & ~O_NODELAY : *sd_flags | O_NODELAY; -#elif defined(FNDELAY) - *previous_mode= (*sd_flags & O_FNDELAY) != 0; - *sd_flags = (block) ? *sd_flags & ~O_FNDELAY : *sd_flags | O_FNDELAY; -#else -#error socket blocking is not supported on this platform -#endif - if (fcntl(csock->socket, F_SETFL, *sd_flags) == -1) + if (fcntl(csock->socket, F_SETFL, new_fcntl_mode) == -1) { - csock->fcntl_mode= save_flags; return errno; } #endif + csock->fcntl_mode = new_fcntl_mode; return 0; }