From 22ce9b2d08a52e399bf2ab86851952d727be034d Mon Sep 17 00:00:00 2001 From: David Benjamin Date: Sun, 6 Mar 2016 19:26:52 -0500 Subject: [PATCH] SSL_set_fd should create socket BIOs, not fd BIOs. In OpenSSL, they create socket BIOs. The distinction isn't important on UNIX. On Windows, file descriptors are provided by the C runtime, while sockets must use separate recv and send APIs. Document how these APIs are intended to work. Also add a TODO to resolve the SOCKET vs int thing. This code assumes that Windows HANDLEs only use the bottom 32 bits of precision. (Which is currently true and probably will continue to be true for the foreseeable future[*], but it'd be nice to do this right.) Thanks to Gisle Vanem and Daniel Stenberg for reporting the bug. [*] Both so Windows can continue to run 32-bit programs and because of all the random UNIX software, like OpenSSL and ourselves, out there which happily assumes sockets are ints. Change-Id: I67408c218572228cb1a7d269892513cda4261c82 Reviewed-on: https://boringssl-review.googlesource.com/7333 Reviewed-by: David Benjamin --- include/openssl/bio.h | 22 +++++++++++++++++++--- include/openssl/ssl.h | 24 +++++++++++++++++++----- ssl/ssl_lib.c | 10 +++++----- 3 files changed, 43 insertions(+), 13 deletions(-) diff --git a/include/openssl/bio.h b/include/openssl/bio.h index a2f0d83e..75afada9 100644 --- a/include/openssl/bio.h +++ b/include/openssl/bio.h @@ -435,12 +435,18 @@ OPENSSL_EXPORT BIO *BIO_new_fd(int fd, int close_flag); /* BIO_set_fd sets the file descriptor of |bio| to |fd|. If |close_flag| is * non-zero then |fd| will be closed when |bio| is. It returns one on success - * or zero on error. */ + * or zero on error. + * + * This function may also be used with socket BIOs (see |BIO_s_socket| and + * |BIO_new_socket|). */ OPENSSL_EXPORT int BIO_set_fd(BIO *bio, int fd, int close_flag); /* BIO_get_fd returns the file descriptor currently in use by |bio| or -1 if * |bio| does not wrap a file descriptor. If there is a file descriptor and - * |out_fd| is not NULL, it also sets |*out_fd| to the file descriptor. */ + * |out_fd| is not NULL, it also sets |*out_fd| to the file descriptor. + * + * This function may also be used with socket BIOs (see |BIO_s_socket| and + * |BIO_new_socket|). */ OPENSSL_EXPORT int BIO_get_fd(BIO *bio, int *out_fd); @@ -520,7 +526,17 @@ OPENSSL_EXPORT int BIO_set_read_buffer_size(BIO *bio, int buffer_size); OPENSSL_EXPORT int BIO_set_write_buffer_size(BIO *bio, int buffer_size); -/* Socket BIOs. */ +/* Socket BIOs. + * + * Socket BIOs behave like file descriptor BIOs but wrap the system's |recv| + * and |send| functions. This is relevant for Windows systems where file + * descriptors, provided by C runtime for use with |read| and |write|, are not + * interchangeable with sockets. + * + * Socket BIOs may be used with |BIO_set_fd| and |BIO_get_fd|. + * + * TODO(davidben): Add separate APIs and fix the internals to use |SOCKET|s + * around rather than rely on int casts. */ OPENSSL_EXPORT const BIO_METHOD *BIO_s_socket(void); diff --git a/include/openssl/ssl.h b/include/openssl/ssl.h index be5776c7..b34fa563 100644 --- a/include/openssl/ssl.h +++ b/include/openssl/ssl.h @@ -251,25 +251,39 @@ OPENSSL_EXPORT int SSL_get_fd(const SSL *ssl); /* SSL_get_rfd returns the file descriptor that |ssl| is configured to read * from. If |ssl|'s read |BIO| is not configured or doesn't wrap a file - * descriptor then it returns -1. */ + * descriptor then it returns -1. + * + * Note: On Windows, this may return either a file descriptor or a socket (cast + * to int), depending on whether |ssl| was configured with a file descriptor or + * socket |BIO|. */ OPENSSL_EXPORT int SSL_get_rfd(const SSL *ssl); /* SSL_get_wfd returns the file descriptor that |ssl| is configured to write * to. If |ssl|'s write |BIO| is not configured or doesn't wrap a file - * descriptor then it returns -1. */ + * descriptor then it returns -1. + * + * Note: On Windows, this may return either a file descriptor or a socket (cast + * to int), depending on whether |ssl| was configured with a file descriptor or + * socket |BIO|. */ OPENSSL_EXPORT int SSL_get_wfd(const SSL *ssl); /* SSL_set_fd configures |ssl| to read from and write to |fd|. It returns one * on success and zero on allocation error. The caller retains ownership of - * |fd|. */ + * |fd|. + * + * On Windows, |fd| is cast to a |SOCKET| and used with Winsock APIs. */ OPENSSL_EXPORT int SSL_set_fd(SSL *ssl, int fd); /* SSL_set_rfd configures |ssl| to read from |fd|. It returns one on success and - * zero on allocation error. The caller retains ownership of |fd|. */ + * zero on allocation error. The caller retains ownership of |fd|. + * + * On Windows, |fd| is cast to a |SOCKET| and used with Winsock APIs. */ OPENSSL_EXPORT int SSL_set_rfd(SSL *ssl, int fd); /* SSL_set_wfd configures |ssl| to write to |fd|. It returns one on success and - * zero on allocation error. The caller retains ownership of |fd|. */ + * zero on allocation error. The caller retains ownership of |fd|. + * + * On Windows, |fd| is cast to a |SOCKET| and used with Winsock APIs. */ OPENSSL_EXPORT int SSL_set_wfd(SSL *ssl, int fd); /* SSL_do_handshake continues the current handshake. If there is none or the diff --git a/ssl/ssl_lib.c b/ssl/ssl_lib.c index c4cc3d72..c1e6a004 100644 --- a/ssl/ssl_lib.c +++ b/ssl/ssl_lib.c @@ -999,7 +999,7 @@ int SSL_get_wfd(const SSL *ssl) { } int SSL_set_fd(SSL *ssl, int fd) { - BIO *bio = BIO_new(BIO_s_fd()); + BIO *bio = BIO_new(BIO_s_socket()); if (bio == NULL) { OPENSSL_PUT_ERROR(SSL, ERR_R_BUF_LIB); return 0; @@ -1011,9 +1011,9 @@ int SSL_set_fd(SSL *ssl, int fd) { int SSL_set_wfd(SSL *ssl, int fd) { if (ssl->rbio == NULL || - BIO_method_type(ssl->rbio) != BIO_TYPE_FD || + BIO_method_type(ssl->rbio) != BIO_TYPE_SOCKET || BIO_get_fd(ssl->rbio, NULL) != fd) { - BIO *bio = BIO_new(BIO_s_fd()); + BIO *bio = BIO_new(BIO_s_socket()); if (bio == NULL) { OPENSSL_PUT_ERROR(SSL, ERR_R_BUF_LIB); return 0; @@ -1028,9 +1028,9 @@ int SSL_set_wfd(SSL *ssl, int fd) { } int SSL_set_rfd(SSL *ssl, int fd) { - if (ssl->wbio == NULL || BIO_method_type(ssl->wbio) != BIO_TYPE_FD || + if (ssl->wbio == NULL || BIO_method_type(ssl->wbio) != BIO_TYPE_SOCKET || BIO_get_fd(ssl->wbio, NULL) != fd) { - BIO *bio = BIO_new(BIO_s_fd()); + BIO *bio = BIO_new(BIO_s_socket()); if (bio == NULL) { OPENSSL_PUT_ERROR(SSL, ERR_R_BUF_LIB); return 0;