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 <davidben@google.com>
diff --git a/include/openssl/bio.h b/include/openssl/bio.h
index a2f0d83..75afada 100644
--- a/include/openssl/bio.h
+++ b/include/openssl/bio.h
@@ -435,12 +435,18 @@
/* 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_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 be5776c..b34fa56 100644
--- a/include/openssl/ssl.h
+++ b/include/openssl/ssl.h
@@ -251,25 +251,39 @@
/* 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 c4cc3d7..c1e6a00 100644
--- a/ssl/ssl_lib.c
+++ b/ssl/ssl_lib.c
@@ -999,7 +999,7 @@
}
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_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_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;
