Skip to content

Commit

Permalink
vtls: encapsulate SSL backend-specific data
Browse files Browse the repository at this point in the history
So far, all of the SSL backends' private data has been declared as
part of the ssl_connect_data struct, in one big #if .. #elif .. #endif
block.

This can only work as long as the SSL backend is a compile-time option,
something we want to change in the next commits.

Therefore, let's encapsulate the exact data needed by each SSL backend
into a private struct, and let's avoid bleeding any SSL backend-specific
information into urldata.h. This is also necessary to allow multiple SSL
backends to be compiled in at the same time, as e.g. OpenSSL's and
CyaSSL's headers cannot be included in the same .c file.

To avoid too many malloc() calls, we simply append the private structs
to the connectdata struct in allocate_conn().

This requires us to take extra care of alignment issues: struct fields
often need to be aligned on certain boundaries e.g. 32-bit values need to
be stored at addresses that divide evenly by 4 (= 32 bit / 8
bit-per-byte).

We do that by assuming that no SSL backend's private data contains any
fields that need to be aligned on boundaries larger than `long long`
(typically 64-bit) would need. Under this assumption, we simply add a
dummy field of type `long long` to the `struct connectdata` struct. This
field will never be accessed but acts as a placeholder for the four
instances of ssl_backend_data instead. the size of each ssl_backend_data
struct is stored in the SSL backend-specific metadata, to allow
allocate_conn() to know how much extra space to allocate, and how to
initialize the ssl[sockindex]->backend and proxy_ssl[sockindex]->backend
pointers.

This would appear to be a little complicated at first, but is really
necessary to encapsulate the private data of each SSL backend correctly.
And we need to encapsulate thusly if we ever want to allow selecting
CyaSSL and OpenSSL at runtime, as their headers cannot be included within
the same .c file (there are just too many conflicting definitions and
declarations for that).

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
  • Loading branch information
dscho authored and bagder committed Aug 28, 2017
1 parent d65e6cc commit 70f1db3
Show file tree
Hide file tree
Showing 13 changed files with 193 additions and 174 deletions.
24 changes: 23 additions & 1 deletion lib/url.c
Original file line number Diff line number Diff line change
Expand Up @@ -4176,7 +4176,12 @@ static void llist_dtor(void *user, void *element)
*/
static struct connectdata *allocate_conn(struct Curl_easy *data)
{
struct connectdata *conn = calloc(1, sizeof(struct connectdata));
#ifdef USE_SSL
#define SSL_EXTRA + 4 * Curl_ssl->sizeof_ssl_backend_data - sizeof(long long)
#else
#define SSL_EXTRA 0
#endif
struct connectdata *conn = calloc(1, sizeof(struct connectdata) + SSL_EXTRA);
if(!conn)
return NULL;

Expand Down Expand Up @@ -4259,6 +4264,23 @@ static struct connectdata *allocate_conn(struct Curl_easy *data)

conn->ip_version = data->set.ipver;

#ifdef USE_SSL
/*
* To save on malloc()s, the SSL backend-specific data has been allocated
* at the end of the connectdata struct.
*/
{
char *p = (char *)&conn->align_data__do_not_use;
conn->ssl[0].backend = (struct ssl_backend_data *)p;
conn->ssl[1].backend =
(struct ssl_backend_data *)(p + Curl_ssl->sizeof_ssl_backend_data);
conn->proxy_ssl[0].backend =
(struct ssl_backend_data *)(p + Curl_ssl->sizeof_ssl_backend_data * 2);
conn->proxy_ssl[1].backend =
(struct ssl_backend_data *)(p + Curl_ssl->sizeof_ssl_backend_data * 3);
}
#endif

#if !defined(CURL_DISABLE_HTTP) && defined(USE_NTLM) && \
defined(NTLM_WB_ENABLED)
conn->ntlm_auth_hlpr_socket = CURL_SOCKET_BAD;
Expand Down
169 changes: 17 additions & 152 deletions lib/urldata.h
Original file line number Diff line number Diff line change
Expand Up @@ -82,71 +82,6 @@
#include "cookie.h"
#include "formdata.h"

#ifdef USE_OPENSSL
#include <openssl/ssl.h>
#ifdef HAVE_OPENSSL_ENGINE_H
#include <openssl/engine.h>
#endif
#endif /* USE_OPENSSL */

#ifdef USE_GNUTLS
#include <gnutls/gnutls.h>
#endif

#ifdef USE_MBEDTLS

#include <mbedtls/ssl.h>
#include <mbedtls/version.h>
#include <mbedtls/entropy.h>
#include <mbedtls/ctr_drbg.h>

#elif defined USE_POLARSSL

#include <polarssl/ssl.h>
#include <polarssl/version.h>
#include <polarssl/entropy.h>
#include <polarssl/ctr_drbg.h>

#endif /* USE_POLARSSL */

#ifdef USE_CYASSL
#undef OCSP_REQUEST /* avoid cyassl/openssl/ssl.h clash with wincrypt.h */
#undef OCSP_RESPONSE /* avoid cyassl/openssl/ssl.h clash with wincrypt.h */
#include <cyassl/openssl/ssl.h>
#endif

#ifdef USE_NSS
#include <nspr.h>
#include <pk11pub.h>
#endif

#ifdef USE_GSKIT
#include <gskssl.h>
#endif

#ifdef USE_AXTLS
#include <axTLS/config.h>
#include <axTLS/ssl.h>
#undef malloc
#undef calloc
#undef realloc
#endif /* USE_AXTLS */

#if defined(USE_SCHANNEL) || defined(USE_WINDOWS_SSPI)
#include "curl_sspi.h"
#endif
#ifdef USE_SCHANNEL
#include <schnlsp.h>
#include <schannel.h>
#endif

#ifdef USE_DARWINSSL
#include <Security/Security.h>
/* For some reason, when building for iOS, the omnibus header above does
* not include SecureTransport.h as of iOS SDK 5.1. */
#include <Security/SecureTransport.h>
#endif

#ifdef HAVE_NETINET_IN_H
#include <netinet/in.h>
#endif
Expand Down Expand Up @@ -236,20 +171,6 @@ enum protection_level {
};
#endif

#ifdef USE_SCHANNEL
/* Structs to store Schannel handles */
struct curl_schannel_cred {
CredHandle cred_handle;
TimeStamp time_stamp;
int refcount;
};

struct curl_schannel_ctxt {
CtxtHandle ctxt_handle;
TimeStamp time_stamp;
};
#endif

/* enum for the nonblocking SSL connection state machine */
typedef enum {
ssl_connect_1,
Expand All @@ -266,6 +187,9 @@ typedef enum {
ssl_connection_complete
} ssl_connection_state;

/* SSL backend-specific data; declared differently by each SSL backend */
struct ssl_backend_data;

/* struct for data related to each SSL connection */
struct ssl_connect_data {
/* Use ssl encrypted communications TRUE/FALSE, not necessarily using it atm
Expand All @@ -274,78 +198,8 @@ struct ssl_connect_data {
bool use;
ssl_connection_state state;
ssl_connect_state connecting_state;
#if defined(USE_OPENSSL)
/* these ones requires specific SSL-types */
SSL_CTX* ctx;
SSL* handle;
X509* server_cert;
#elif defined(USE_GNUTLS)
gnutls_session_t session;
gnutls_certificate_credentials_t cred;
#ifdef USE_TLS_SRP
gnutls_srp_client_credentials_t srp_client_cred;
#endif
#elif defined(USE_MBEDTLS)
mbedtls_ctr_drbg_context ctr_drbg;
mbedtls_entropy_context entropy;
mbedtls_ssl_context ssl;
int server_fd;
mbedtls_x509_crt cacert;
mbedtls_x509_crt clicert;
mbedtls_x509_crl crl;
mbedtls_pk_context pk;
mbedtls_ssl_config config;
const char *protocols[3];
#elif defined(USE_POLARSSL)
ctr_drbg_context ctr_drbg;
entropy_context entropy;
ssl_context ssl;
int server_fd;
x509_crt cacert;
x509_crt clicert;
x509_crl crl;
rsa_context rsa;
#elif defined(USE_CYASSL)
SSL_CTX* ctx;
SSL* handle;
#elif defined(USE_NSS)
PRFileDesc *handle;
char *client_nickname;
struct Curl_easy *data;
struct curl_llist obj_list;
PK11GenericObject *obj_clicert;
#elif defined(USE_GSKIT)
gsk_handle handle;
int iocport;
int localfd;
int remotefd;
#elif defined(USE_AXTLS)
SSL_CTX* ssl_ctx;
SSL* ssl;
#elif defined(USE_SCHANNEL)
struct curl_schannel_cred *cred;
struct curl_schannel_ctxt *ctxt;
SecPkgContext_StreamSizes stream_sizes;
size_t encdata_length, decdata_length;
size_t encdata_offset, decdata_offset;
unsigned char *encdata_buffer, *decdata_buffer;
/* encdata_is_incomplete: if encdata contains only a partial record that
can't be decrypted without another Curl_read_plain (that is, status is
SEC_E_INCOMPLETE_MESSAGE) then set this true. after Curl_read_plain writes
more bytes into encdata then set this back to false. */
bool encdata_is_incomplete;
unsigned long req_flags, ret_flags;
CURLcode recv_unrecoverable_err; /* schannel_recv had an unrecoverable err */
bool recv_sspi_close_notify; /* true if connection closed by close_notify */
bool recv_connection_closed; /* true if connection closed, regardless how */
bool use_alpn; /* true if ALPN is used for this connection */
#elif defined(USE_DARWINSSL)
SSLContextRef ssl_ctx;
curl_socket_t ssl_sockfd;
bool ssl_direction; /* true if writing, false if reading */
size_t ssl_write_buffered_length;
#elif defined(USE_SSL)
#error "SSL backend specific information missing from ssl_connect_data"
#if defined(USE_SSL)
struct ssl_backend_data *backend;
#endif
};

Expand Down Expand Up @@ -1175,6 +1029,16 @@ struct connectdata {
char *unix_domain_socket;
bool abstract_unix_socket;
#endif

#ifdef USE_SSL
/*
* To avoid multiple malloc() calls, the ssl_connect_data structures
* associated with a connectdata struct are allocated in the same block
* as the latter. This field forces alignment to an 8-byte boundary so
* that this all works.
*/
long long *align_data__do_not_use;
#endif
};

/* The end of connectdata. */
Expand Down Expand Up @@ -1429,7 +1293,8 @@ struct UrlState {
ares_channel f.e. */

#if defined(USE_OPENSSL) && defined(HAVE_OPENSSL_ENGINE_H)
ENGINE *engine;
/* void instead of ENGINE to avoid bleeding OpenSSL into this header */
void *engine;
#endif /* USE_OPENSSL */
struct curltime expiretime; /* set this with Curl_expire() only */
struct Curl_tree timenode; /* for the splay stuff */
Expand Down
11 changes: 9 additions & 2 deletions lib/vtls/axtls.c
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,12 @@
#include "curl_memory.h"
#include "memdebug.h"

#define BACKEND connssl
struct ssl_backend_data {
SSL_CTX* ssl_ctx;
SSL* ssl;
};

#define BACKEND connssl->backend

static CURLcode map_error_to_curl(int axtls_err)
{
Expand Down Expand Up @@ -652,7 +657,7 @@ static ssize_t axtls_recv(struct connectdata *conn, /* connection data */
static int Curl_axtls_check_cxn(struct connectdata *conn)
{
/* openssl.c line:
rc = SSL_peek(conn->ssl[FIRSTSOCKET].ssl, (void*)&buf, 1);
rc = SSL_peek(conn->ssl[FIRSTSOCKET].backend->ssl, (void*)&buf, 1);
axTLS compat layer always returns the last argument, so connection is
always alive? */

Expand Down Expand Up @@ -705,6 +710,8 @@ const struct Curl_ssl Curl_ssl_axtls = {
0, /* have_ssl_ctx */
0, /* support_https_proxy */

sizeof(struct ssl_backend_data),

/*
* axTLS has no global init. Everything is done through SSL and SSL_CTX
* structs stored in connectdata structure.
Expand Down
10 changes: 9 additions & 1 deletion lib/vtls/cyassl.c
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,7 @@ and that's a problem since options.h hasn't been included yet. */
#include "x509asn1.h"
#include "curl_printf.h"

#include <cyassl/openssl/ssl.h>
#include <cyassl/ssl.h>
#ifdef HAVE_CYASSL_ERROR_SSL_H
#include <cyassl/error-ssl.h>
Expand Down Expand Up @@ -122,7 +123,12 @@ and that's a problem since options.h hasn't been included yet. */
#endif
#endif

#define BACKEND connssl
struct ssl_backend_data {
SSL_CTX* ctx;
SSL* handle;
};

#define BACKEND connssl->backend

static Curl_recv cyassl_recv;
static Curl_send cyassl_send;
Expand Down Expand Up @@ -984,6 +990,8 @@ const struct Curl_ssl Curl_ssl_cyassl = {
1, /* have_ssl_ctx */
0, /* support_https_proxy */

sizeof(struct ssl_backend_data),

Curl_cyassl_init, /* init */
Curl_none_cleanup, /* cleanup */
Curl_cyassl_version, /* version */
Expand Down
13 changes: 12 additions & 1 deletion lib/vtls/darwinssl.c
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,8 @@
#endif

#include <Security/Security.h>
/* For some reason, when building for iOS, the omnibus header above does
* not include SecureTransport.h as of iOS SDK 5.1. */
#include <Security/SecureTransport.h>
#include <CoreFoundation/CoreFoundation.h>
#include <CommonCrypto/CommonDigest.h>
Expand Down Expand Up @@ -118,7 +120,14 @@
#define ioErr -36
#define paramErr -50

#define BACKEND connssl
struct ssl_backend_data {
SSLContextRef ssl_ctx;
curl_socket_t ssl_sockfd;
bool ssl_direction; /* true if writing, false if reading */
size_t ssl_write_buffered_length;
};

#define BACKEND connssl->backend

/* pinned public key support tests */

Expand Down Expand Up @@ -2893,6 +2902,8 @@ const struct Curl_ssl Curl_ssl_darwinssl = {
0, /* have_ssl_ctx */
0, /* support_https_proxy */

sizeof(struct ssl_backend_data),

Curl_none_init, /* init */
Curl_none_cleanup, /* cleanup */
Curl_darwinssl_version, /* version */
Expand Down
14 changes: 11 additions & 3 deletions lib/vtls/gskit.c
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,14 @@
#define CURL_GSKPROTO_TLSV12_MASK (1 << CURL_GSKPROTO_TLSV12)
#define CURL_GSKPROTO_LAST 5

#define BACKEND connssl
struct ssl_backend_data {
gsk_handle handle;
int iocport;
int localfd;
int remotefd;
};

#define BACKEND connssl->backend

/* Supported ciphers. */
typedef struct {
Expand Down Expand Up @@ -638,7 +645,7 @@ static int pipe_ssloverssl(struct connectdata *conn, int sockindex,
if(FD_ISSET(BACKEND->remotefd, &fds_write)) {
/* Try getting data from HTTPS proxy and pipe it upstream. */
n = 0;
i = gsk_secure_soc_read(connproxyssl->handle,
i = gsk_secure_soc_read(connproxyssl->backend->handle,
buf, sizeof buf, &n);
switch(i) {
case GSK_OK:
Expand All @@ -664,7 +671,7 @@ static int pipe_ssloverssl(struct connectdata *conn, int sockindex,
if(n < 0)
return -1;
if(n) {
i = gsk_secure_soc_write(connproxyssl->handle, buf, n, &m);
i = gsk_secure_soc_write(connproxyssl->backend->handle, buf, n, &m);
if(i != GSK_OK || n != m)
return -1;
ret = 1;
Expand Down Expand Up @@ -1355,6 +1362,7 @@ const struct Curl_ssl Curl_ssl_gskit = {
/* TODO: convert to 1 and fix test #1014 (if need) */
0, /* support_https_proxy */

sizeof(struct ssl_backend_data),

Curl_gskit_init, /* init */
Curl_gskit_cleanup, /* cleanup */
Expand Down

0 comments on commit 70f1db3

Please sign in to comment.