From 487f5e0a62206bae72b421b415cb21e87f296cf3 Mon Sep 17 00:00:00 2001 From: Justin Smith Date: Fri, 4 Apr 2025 14:00:10 -0400 Subject: [PATCH 1/9] Draft dgram impl complete --- crypto/CMakeLists.txt | 1 + crypto/bio/dgram.c | 340 ++++++++++++++++++++++++++++++++++++++++++ include/openssl/bio.h | 66 +++++++- 3 files changed, 402 insertions(+), 5 deletions(-) create mode 100644 crypto/bio/dgram.c diff --git a/crypto/CMakeLists.txt b/crypto/CMakeLists.txt index 563e71b464..1cb265d21a 100644 --- a/crypto/CMakeLists.txt +++ b/crypto/CMakeLists.txt @@ -364,6 +364,7 @@ add_library( bio/bio.c bio/bio_mem.c bio/connect.c + bio/dgram.c bio/errno.c bio/fd.c bio/file.c diff --git a/crypto/bio/dgram.c b/crypto/bio/dgram.c new file mode 100644 index 0000000000..fa9e06d2c3 --- /dev/null +++ b/crypto/bio/dgram.c @@ -0,0 +1,340 @@ +// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 OR ISC + +#include +#include + +#if defined(OPENSSL_WINDOWS) +#include +#include +#include +#include +#else +#include +#include +#include +#include +#endif +#include + +#include "../internal.h" +#include "./internal.h" + +#if !defined(OPENSSL_WINDOWS) +static int closesocket(const int sock) { return close(sock); } +#endif + +/* + * BIO_ADDR_make - non-public routine to fill a BIO_ADDR with the contents + * of a struct sockaddr. + */ +static int BIO_ADDR_make(BIO_ADDR *ap, const struct sockaddr *sa) { + if (sa->sa_family == AF_INET) { + OPENSSL_memcpy(&ap->s_in, sa, sizeof(struct sockaddr_in)); + return 1; + } +#ifdef AF_INET6 + if (sa->sa_family == AF_INET6) { + OPENSSL_memcpy(&ap->s_in6, sa, sizeof(struct sockaddr_in6)); + return 1; + } +#endif +#ifdef AF_UNIX + if (sa->sa_family == AF_UNIX) { + OPENSSL_memcpy(&ap->s_un, sa, sizeof(struct sockaddr_un)); + return 1; + } +#endif + + return 0; +} + +typedef struct bio_dgram_data_st { + BIO_ADDR peer; + unsigned int connected; + unsigned int _errno; + unsigned int mtu; +} bio_dgram_data; + +static socklen_t BIO_ADDR_sockaddr_size(const BIO_ADDR *ap) { + if (ap->sa.sa_family == AF_INET) { + return sizeof(ap->s_in); + } +#ifdef AF_INET6 + if (ap->sa.sa_family == AF_INET6) { + return sizeof(ap->s_in6); + } +#endif +#ifdef AF_UNIX + if (ap->sa.sa_family == AF_UNIX) { + return sizeof(ap->s_un); + } +#endif + return sizeof(*ap); +} + +static struct sockaddr *BIO_ADDR_sockaddr_noconst(BIO_ADDR *ap) { + return &ap->sa; +} + +static const struct sockaddr *BIO_ADDR_sockaddr(const BIO_ADDR *ap) { + return &ap->sa; +} + +static int dgram_write(BIO *bp, const char *in, const int inl) { + bio_dgram_data *data = bp->ptr; + ssize_t result; + + errno = 0; + if (data->connected) { + // With a zero flags argument, send() is equivalent to write(2). + result = send(bp->num, in, inl, 0); + } else { + // If sendto() is used on a connection-mode (SOCK_STREAM, SOCK_SEQPACKET) + // socket, the arguments dest_addr and addrlen are ignored + const socklen_t peerlen = BIO_ADDR_sockaddr_size(&data->peer); + result = sendto(bp->num, in, inl, 0, BIO_ADDR_sockaddr(&data->peer), peerlen); + } + + if (result < INT_MIN || result > INT_MAX) { + abort(); + } + const int ret = result; + + BIO_clear_retry_flags(bp); + if (ret <= 0 && bio_errno_should_retry(ret)) { + BIO_set_retry_write(bp); + data->_errno = errno; + } + return ret; +} + +static int dgram_read(BIO *bp, char *out, const int outl) { + if (!out) { + return 0; + } + + bio_dgram_data *data = bp->ptr; + BIO_ADDR peer = {0}; + socklen_t len = sizeof(peer); + + errno = 0; + + // recvfrom may be used to receive data on a socket whether or not it is + // connection-oriented. + const ssize_t result = recvfrom(bp->num, out, outl, 0, + BIO_ADDR_sockaddr_noconst(&peer), &len); + + if (result < INT_MIN || result > INT_MAX) { + abort(); + } + const int ret = result; + + if (!data->connected && ret >= 0) { + assert(len == BIO_ADDR_sockaddr_size(&peer)); + BIO_ctrl(bp, BIO_CTRL_DGRAM_SET_PEER, 0, &peer); + } + + BIO_clear_retry_flags(bp); + if (ret < 0 && bio_errno_should_retry(ret)) { + BIO_set_retry_read(bp); + data->_errno = errno; + } + + return ret; +} + +static int dgram_puts(BIO *bp, const char *str) { + const size_t len = strlen(str); + if (len > INT_MAX) { + return 0; + } + return dgram_write(bp, str, len); +} + +static int dgram_free(BIO *bp) { + GUARD_PTR(bp); + if (bp->shutdown && bp->init) { + if (0 != closesocket(bp->num)) { + // the show must go on + } + } + bp->init = 0; + bp->flags = 0; + OPENSSL_free(bp->ptr); + return 1; +} + +static long dgram_ctrl(BIO *b, const int cmd, const long num, void *ptr) { + GUARD_PTR(b); + bio_dgram_data *data = b->ptr; + + long ret = 1; + + switch (cmd) { + case BIO_C_SET_FD: + if (0 == dgram_free(b)) { + assert(0); + } + b->num = *(int*)ptr; + b->shutdown = (int)num; + b->init = 1; + break; + case BIO_C_GET_FD: + if (b->init) { + int *ip = ptr; + if (ip) { + *ip = b->num; + } + ret = b->num; + } else { + ret = -1; + } + break; + case BIO_CTRL_GET_CLOSE: + ret = b->shutdown; + break; + case BIO_CTRL_SET_CLOSE: + b->shutdown = (int)num; + break; + case BIO_CTRL_FLUSH: + ret = 1; + break; + case BIO_CTRL_DGRAM_GET_MTU: + ret = data->mtu; + break; + case BIO_CTRL_DGRAM_SET_MTU: + data->mtu = num; + ret = num; + break; + case BIO_CTRL_DGRAM_SET_CONNECTED: + if (ptr != NULL) { + data->connected = 1; + ret = BIO_ADDR_make(&data->peer, BIO_ADDR_sockaddr(ptr)); + } else { + data->connected = 0; + OPENSSL_cleanse(&data->peer, sizeof(data->peer)); + ret = 1; + } + break; + case BIO_CTRL_DGRAM_CONNECT: + ret = BIO_ADDR_make(&data->peer, BIO_ADDR_sockaddr(ptr)); + break; + case BIO_CTRL_DGRAM_GET_PEER: { + const socklen_t size = BIO_ADDR_sockaddr_size(&data->peer); + if (num == 0 || num > size) { + OPENSSL_memcpy(ptr, &data->peer, size); + ret = size; + } else { + ret = 0; + } + break; + } + case BIO_CTRL_DGRAM_SET_PEER: + ret = BIO_ADDR_make(&data->peer, BIO_ADDR_sockaddr(ptr)); + break; + case BIO_CTRL_DGRAM_GET_SEND_TIMER_EXP: + /* fall-through */ + case BIO_CTRL_DGRAM_GET_RECV_TIMER_EXP: { + int d_errno = 0; +# ifdef OPENSSL_WINDOWS + d_errno = (data->_errno == WSAETIMEDOUT); +# else + d_errno = (data->_errno == EAGAIN); +# endif + if (d_errno) { + ret = 1; + data->_errno = 0; + } else + ret = 0; + break; + } + default: + ret = 0; + break; + } + return ret; +} + +static int dgram_new(BIO *bio) { + bio_dgram_data *data = OPENSSL_zalloc(sizeof(*data)); + if (!data) { + return 0; + } + bio->ptr = data; + return 1; +} + +static const BIO_METHOD methods_dgramp = { + .type = BIO_TYPE_DGRAM, + .name = "datagram socket", + .bwrite = dgram_write, + .bread = dgram_read, + .bputs = dgram_puts, + .bgets = NULL, + .ctrl = dgram_ctrl, + .create = dgram_new, + .destroy = dgram_free, + .callback_ctrl = NULL, +}; + +const BIO_METHOD *BIO_s_datagram(void) { return &methods_dgramp; } + +BIO *BIO_new_dgram(int fd, int close_flag) +{ + BIO *ret; + + ret = BIO_new(BIO_s_datagram()); + if (ret == NULL) + return NULL; + BIO_set_fd(ret, fd, close_flag); + return ret; +} + +int BIO_ctrl_dgram_connect(BIO *bp, const BIO_ADDR *peer) { + long ret = BIO_ctrl(bp, BIO_CTRL_DGRAM_CONNECT, 0, (BIO_ADDR*)peer); + if (ret < INT_MIN || ret > INT_MAX) { + return 0; + } + return ret; +} + +int BIO_ctrl_set_connected(BIO* bp, const BIO_ADDR *peer) { + long ret = BIO_ctrl(bp, BIO_CTRL_DGRAM_SET_CONNECTED, 0, (BIO_ADDR*)peer); + if (ret < INT_MIN || ret > INT_MAX) { + return 0; + } + return ret; +} + +int BIO_dgram_recv_timedout(BIO* bp) { + long ret = BIO_ctrl(bp, BIO_CTRL_DGRAM_GET_RECV_TIMER_EXP, 0, NULL); + if (ret < INT_MIN || ret > INT_MAX) { + return 0; + } + return ret; +} + +int BIO_dgram_send_timedout(BIO* bp) { + long ret = BIO_ctrl(bp, BIO_CTRL_DGRAM_GET_SEND_TIMER_EXP, 0, NULL); + if (ret < INT_MIN || ret > INT_MAX) { + return 0; + } + return ret; +} + +int BIO_dgram_get_peer(BIO* bp, BIO_ADDR *peer) { + long ret = BIO_ctrl(bp, BIO_CTRL_DGRAM_GET_PEER, 0, peer); + if (ret < INT_MIN || ret > INT_MAX) { + return 0; + } + return ret; +} + +int BIO_dgram_set_peer(BIO* bp, const BIO_ADDR *peer) { + long ret = BIO_ctrl(bp, BIO_CTRL_DGRAM_SET_PEER, 0, (BIO_ADDR*)peer); + if (ret < INT_MIN || ret > INT_MAX) { + return 0; + } + return ret; +} diff --git a/include/openssl/bio.h b/include/openssl/bio.h index f5f8625c88..b5ed19076e 100644 --- a/include/openssl/bio.h +++ b/include/openssl/bio.h @@ -67,6 +67,18 @@ #include #include +#if defined(OPENSSL_WINDOWS) +#include +#include +#include +#include +#else +#include +#include +#include +#include +#endif + #if defined(__cplusplus) extern "C" { #endif @@ -694,17 +706,36 @@ OPENSSL_EXPORT int BIO_do_connect(BIO *bio); // Datagram BIOs. -// -// TODO(fork): not implemented. -#define BIO_CTRL_DGRAM_QUERY_MTU 40 // as kernel for current MTU + // Data structures +typedef union bio_addr_st { + struct sockaddr sa; +#ifdef AF_INET6 + struct sockaddr_in6 s_in6; +#endif + struct sockaddr_in s_in; +#ifdef AF_UNIX + struct sockaddr_un s_un; +#endif +} BIO_ADDR; + +#define BIO_CTRL_DGRAM_CONNECT 31// BIO dgram special +#define BIO_CTRL_DGRAM_SET_CONNECTED 32 /* allow for an externally connected + * socket to be passed in */ +# define BIO_CTRL_DGRAM_GET_RECV_TIMER_EXP 37 // flag whether the last +# define BIO_CTRL_DGRAM_GET_SEND_TIMER_EXP 38 // I/O operation tiemd out + +#define BIO_CTRL_DGRAM_QUERY_MTU 40 // as kernel for current MTU +#define BIO_CTRL_DGRAM_GET_MTU 41 // get cached value for MTU #define BIO_CTRL_DGRAM_SET_MTU 42 /* set cached value for MTU. want to use - this if asking the kernel fails */ + * this if asking the kernel fails */ -#define BIO_CTRL_DGRAM_MTU_EXCEEDED 43 /* check whether the MTU was exceed in +#define BIO_CTRL_DGRAM_MTU_EXCEEDED 43 /* check whether the MTU was exceeded in the previous write operation. */ +#define BIO_CTRL_DGRAM_SET_PEER 44 // Destination for the data + // BIO_CTRL_DGRAM_SET_NEXT_TIMEOUT is unsupported as it is unused by consumers // and depends on |timeval|, which is not 2038-clean on all platforms. @@ -712,6 +743,31 @@ OPENSSL_EXPORT int BIO_do_connect(BIO *bio); #define BIO_CTRL_DGRAM_GET_FALLBACK_MTU 47 +OPENSSL_EXPORT const BIO_METHOD *BIO_s_datagram(void); + +// TODO: documentation +OPENSSL_EXPORT BIO *BIO_new_dgram(int fd, int close_flag); + +// TODO: documentation +OPENSSL_EXPORT int BIO_ctrl_dgram_connect(BIO *bio, const BIO_ADDR *peer); + +// TODO: documentation +OPENSSL_EXPORT int BIO_ctrl_set_connected(BIO* bp, const BIO_ADDR *sa); + +// TODO: documentation +OPENSSL_EXPORT int BIO_dgram_recv_timedout(BIO* bp); + +// TODO: documentation +OPENSSL_EXPORT int BIO_dgram_send_timedout(BIO* bp); + +// TODO: documentation +OPENSSL_EXPORT int BIO_dgram_get_peer(BIO* bp, BIO_ADDR *sa); + +// TODO: documentation +OPENSSL_EXPORT int BIO_dgram_set_peer(BIO* bp, const BIO_ADDR *sa); + +// TODO: documentation +OPENSSL_EXPORT unsigned int BIO_dgram_get_mtu_overhead(BIO* bp, struct sockaddr *sa); // BIO Pairs. // From 4a46cddd3cf39e3ab6bd98fd486fd31bf97ecf09 Mon Sep 17 00:00:00 2001 From: Justin Smith Date: Tue, 8 Apr 2025 10:28:40 -0400 Subject: [PATCH 2/9] Cleanup dgram and socket_helper usage --- crypto/bio/connect.c | 6 +- crypto/bio/dgram.c | 141 +++++++++++++++++++++++-------------- crypto/bio/internal.h | 9 +-- crypto/bio/socket.c | 4 +- crypto/bio/socket_helper.c | 8 ++- 5 files changed, 101 insertions(+), 67 deletions(-) diff --git a/crypto/bio/connect.c b/crypto/bio/connect.c index 0916d0cfa1..cdbc1db516 100644 --- a/crypto/bio/connect.c +++ b/crypto/bio/connect.c @@ -250,7 +250,7 @@ static int conn_state(BIO *bio, BIO_CONNECT *c) { break; case BIO_CONN_S_BLOCKED_CONNECT: - i = bio_sock_error(bio->num); + i = bio_sock_error_get_and_clear(bio->num); if (i) { if (bio_socket_should_retry(ret)) { BIO_set_flags(bio, (BIO_FLAGS_IO_SPECIAL | BIO_FLAGS_SHOULD_RETRY)); @@ -359,7 +359,7 @@ static int conn_read(BIO *bio, char *out, int out_len) { } } - bio_clear_socket_error(); + bio_clear_socket_error(bio->num); ret = (int)recv(bio->num, out, out_len, 0); BIO_clear_retry_flags(bio); if (ret <= 0) { @@ -383,7 +383,7 @@ static int conn_write(BIO *bio, const char *in, int in_len) { } } - bio_clear_socket_error(); + bio_clear_socket_error(bio->num); ret = (int)send(bio->num, in, in_len, 0); BIO_clear_retry_flags(bio); if (ret <= 0) { diff --git a/crypto/bio/dgram.c b/crypto/bio/dgram.c index fa9e06d2c3..11475971b5 100644 --- a/crypto/bio/dgram.c +++ b/crypto/bio/dgram.c @@ -28,20 +28,23 @@ static int closesocket(const int sock) { return close(sock); } * BIO_ADDR_make - non-public routine to fill a BIO_ADDR with the contents * of a struct sockaddr. */ -static int BIO_ADDR_make(BIO_ADDR *ap, const struct sockaddr *sa) { - if (sa->sa_family == AF_INET) { - OPENSSL_memcpy(&ap->s_in, sa, sizeof(struct sockaddr_in)); + +static int BIO_ADDR_make(BIO_ADDR *bap, const struct sockaddr *sap) { + GUARD_PTR(bap); + GUARD_PTR(sap); + if (sap->sa_family == AF_INET) { + OPENSSL_memcpy(&bap->s_in, sap, sizeof(struct sockaddr_in)); return 1; } #ifdef AF_INET6 - if (sa->sa_family == AF_INET6) { - OPENSSL_memcpy(&ap->s_in6, sa, sizeof(struct sockaddr_in6)); + if (sap->sa_family == AF_INET6) { + OPENSSL_memcpy(&bap->s_in6, sap, sizeof(struct sockaddr_in6)); return 1; } #endif #ifdef AF_UNIX - if (sa->sa_family == AF_UNIX) { - OPENSSL_memcpy(&ap->s_un, sa, sizeof(struct sockaddr_un)); + if (sap->sa_family == AF_UNIX) { + OPENSSL_memcpy(&bap->s_un, sap, sizeof(struct sockaddr_un)); return 1; } #endif @@ -56,44 +59,57 @@ typedef struct bio_dgram_data_st { unsigned int mtu; } bio_dgram_data; -static socklen_t BIO_ADDR_sockaddr_size(const BIO_ADDR *ap) { - if (ap->sa.sa_family == AF_INET) { - return sizeof(ap->s_in); +static socklen_t BIO_ADDR_sockaddr_size(const BIO_ADDR *bap) { + GUARD_PTR(bap); + if (bap->sa.sa_family == AF_INET) { + return sizeof(bap->s_in); } #ifdef AF_INET6 - if (ap->sa.sa_family == AF_INET6) { - return sizeof(ap->s_in6); + if (bap->sa.sa_family == AF_INET6) { + return sizeof(bap->s_in6); } #endif #ifdef AF_UNIX - if (ap->sa.sa_family == AF_UNIX) { - return sizeof(ap->s_un); + if (bap->sa.sa_family == AF_UNIX) { + return sizeof(bap->s_un); } #endif - return sizeof(*ap); + return sizeof(*bap); } -static struct sockaddr *BIO_ADDR_sockaddr_noconst(BIO_ADDR *ap) { - return &ap->sa; + + +static struct sockaddr *BIO_ADDR_sockaddr_noconst(BIO_ADDR *bap) { + GUARD_PTR(bap); + return &bap->sa; } -static const struct sockaddr *BIO_ADDR_sockaddr(const BIO_ADDR *ap) { - return &ap->sa; +static const struct sockaddr *BIO_ADDR_sockaddr(const BIO_ADDR *bap) { + GUARD_PTR(bap); + return &bap->sa; } -static int dgram_write(BIO *bp, const char *in, const int inl) { +static int dgram_write(BIO *bp, const char *in, const int in_len) { + GUARD_PTR(bp); + GUARD_PTR(in); + bio_dgram_data *data = bp->ptr; ssize_t result; - errno = 0; + if (in_len <= 0) { + OPENSSL_PUT_ERROR(BIO, BIO_R_INVALID_ARGUMENT); + return 0; + } + + bio_clear_socket_error(bp->num); if (data->connected) { // With a zero flags argument, send() is equivalent to write(2). - result = send(bp->num, in, inl, 0); + result = send(bp->num, in, in_len, 0); } else { // If sendto() is used on a connection-mode (SOCK_STREAM, SOCK_SEQPACKET) // socket, the arguments dest_addr and addrlen are ignored const socklen_t peerlen = BIO_ADDR_sockaddr_size(&data->peer); - result = sendto(bp->num, in, inl, 0, BIO_ADDR_sockaddr(&data->peer), peerlen); + result = sendto(bp->num, in, in_len, 0, BIO_ADDR_sockaddr(&data->peer), peerlen); } if (result < INT_MIN || result > INT_MAX) { @@ -102,27 +118,26 @@ static int dgram_write(BIO *bp, const char *in, const int inl) { const int ret = result; BIO_clear_retry_flags(bp); - if (ret <= 0 && bio_errno_should_retry(ret)) { + if (ret <= 0 && bio_socket_should_retry(ret)) { BIO_set_retry_write(bp); - data->_errno = errno; + data->_errno = bio_sock_error_get_and_clear(bp->num); } return ret; } -static int dgram_read(BIO *bp, char *out, const int outl) { - if (!out) { - return 0; - } +static int dgram_read(BIO *bp, char *out, const int out_len) { + GUARD_PTR(bp); + GUARD_PTR(out); bio_dgram_data *data = bp->ptr; BIO_ADDR peer = {0}; socklen_t len = sizeof(peer); - errno = 0; + bio_clear_socket_error(bp->num); - // recvfrom may be used to receive data on a socket whether or not it is - // connection-oriented. - const ssize_t result = recvfrom(bp->num, out, outl, 0, + // recvfrom may be used to receive data on a socket regardless of whether + // it's connection-oriented. + const ssize_t result = recvfrom(bp->num, out, out_len, 0, BIO_ADDR_sockaddr_noconst(&peer), &len); if (result < INT_MIN || result > INT_MAX) { @@ -136,19 +151,19 @@ static int dgram_read(BIO *bp, char *out, const int outl) { } BIO_clear_retry_flags(bp); - if (ret < 0 && bio_errno_should_retry(ret)) { + if (ret < 0 && bio_socket_should_retry(ret)) { BIO_set_retry_read(bp); - data->_errno = errno; + data->_errno = bio_sock_error_get_and_clear(bp->num); } return ret; } static int dgram_puts(BIO *bp, const char *str) { - const size_t len = strlen(str); - if (len > INT_MAX) { - return 0; - } + GUARD_PTR(bp); + GUARD_PTR(str); + + const int len = OPENSSL_strnlen(str, INT_MAX); return dgram_write(bp, str, len); } @@ -162,52 +177,58 @@ static int dgram_free(BIO *bp) { bp->init = 0; bp->flags = 0; OPENSSL_free(bp->ptr); + bp->ptr = NULL; return 1; } -static long dgram_ctrl(BIO *b, const int cmd, const long num, void *ptr) { - GUARD_PTR(b); - bio_dgram_data *data = b->ptr; +static long dgram_ctrl(BIO *bp, const int cmd, const long num, void *ptr) { + GUARD_PTR(bp); + bio_dgram_data *data = bp->ptr; long ret = 1; switch (cmd) { case BIO_C_SET_FD: - if (0 == dgram_free(b)) { + GUARD_PTR(ptr); + if (0 == dgram_free(bp)) { assert(0); } - b->num = *(int*)ptr; - b->shutdown = (int)num; - b->init = 1; + bp->num = *(int*)ptr; + bp->shutdown = (int)num; + bp->init = 1; break; case BIO_C_GET_FD: - if (b->init) { + GUARD_PTR(ptr); + if (bp->init) { int *ip = ptr; if (ip) { - *ip = b->num; + *ip = bp->num; } - ret = b->num; + ret = bp->num; } else { ret = -1; } break; case BIO_CTRL_GET_CLOSE: - ret = b->shutdown; + ret = bp->shutdown; break; case BIO_CTRL_SET_CLOSE: - b->shutdown = (int)num; + bp->shutdown = (num != 0); break; case BIO_CTRL_FLUSH: ret = 1; break; case BIO_CTRL_DGRAM_GET_MTU: + GUARD_PTR(data); ret = data->mtu; break; case BIO_CTRL_DGRAM_SET_MTU: + GUARD_PTR(data); data->mtu = num; ret = num; break; case BIO_CTRL_DGRAM_SET_CONNECTED: + GUARD_PTR(data); if (ptr != NULL) { data->connected = 1; ret = BIO_ADDR_make(&data->peer, BIO_ADDR_sockaddr(ptr)); @@ -218,9 +239,13 @@ static long dgram_ctrl(BIO *b, const int cmd, const long num, void *ptr) { } break; case BIO_CTRL_DGRAM_CONNECT: + GUARD_PTR(data); + GUARD_PTR(ptr); ret = BIO_ADDR_make(&data->peer, BIO_ADDR_sockaddr(ptr)); break; case BIO_CTRL_DGRAM_GET_PEER: { + GUARD_PTR(data); + GUARD_PTR(ptr); const socklen_t size = BIO_ADDR_sockaddr_size(&data->peer); if (num == 0 || num > size) { OPENSSL_memcpy(ptr, &data->peer, size); @@ -231,16 +256,26 @@ static long dgram_ctrl(BIO *b, const int cmd, const long num, void *ptr) { break; } case BIO_CTRL_DGRAM_SET_PEER: + GUARD_PTR(data); + GUARD_PTR(ptr); ret = BIO_ADDR_make(&data->peer, BIO_ADDR_sockaddr(ptr)); break; case BIO_CTRL_DGRAM_GET_SEND_TIMER_EXP: - /* fall-through */ case BIO_CTRL_DGRAM_GET_RECV_TIMER_EXP: { + + GUARD_PTR(data); int d_errno = 0; # ifdef OPENSSL_WINDOWS d_errno = (data->_errno == WSAETIMEDOUT); # else - d_errno = (data->_errno == EAGAIN); + /* + * if no data has been transferred and the timeout has been reached, + * then -1 is returned with errno set to EAGAIN or EWOULDBLOCK, + * or EINPROGRESS (for connect(2)) just as if the socket was specified + * to be nonblocking. + */ + d_errno = (data->_errno == EAGAIN) || (data->_errno == EWOULDBLOCK) || + (data->_errno == EINPROGRESS); # endif if (d_errno) { ret = 1; diff --git a/crypto/bio/internal.h b/crypto/bio/internal.h index 06f88c9aa1..a3f7d95b18 100644 --- a/crypto/bio/internal.h +++ b/crypto/bio/internal.h @@ -95,13 +95,11 @@ int bio_ip_and_port_to_socket_and_addr(int *out_sock, // success and zero otherwise. int bio_socket_nbio(int sock, int on); -// bio_clear_socket_error clears the last system socket error. -// -// TODO(fork): remove all callers of this. -void bio_clear_socket_error(void); +// bio_clear_socket_error clears the last socket error on |sock|. +void bio_clear_socket_error(int sock); // bio_sock_error returns the last socket error on |sock|. -int bio_sock_error(int sock); +int bio_sock_error_get_and_clear(int sock); // bio_socket_should_retry returns non-zero if |return_value| indicates an error // and the last socket error indicates that it's non-fatal. @@ -113,7 +111,6 @@ int bio_socket_should_retry(int return_value); // and |errno| indicates that it's non-fatal. int bio_errno_should_retry(int return_value); - #if defined(__cplusplus) } // extern C #endif diff --git a/crypto/bio/socket.c b/crypto/bio/socket.c index c86b618b20..d6eb16b687 100644 --- a/crypto/bio/socket.c +++ b/crypto/bio/socket.c @@ -94,7 +94,7 @@ static int sock_read(BIO *b, char *out, int outl) { return 0; } - bio_clear_socket_error(); + bio_clear_socket_error(b->num); #if defined(OPENSSL_WINDOWS) int ret = recv(b->num, out, outl, 0); #else @@ -110,7 +110,7 @@ static int sock_read(BIO *b, char *out, int outl) { } static int sock_write(BIO *b, const char *in, int inl) { - bio_clear_socket_error(); + bio_clear_socket_error(b->num); #if defined(OPENSSL_WINDOWS) int ret = send(b->num, in, inl, 0); #else diff --git a/crypto/bio/socket_helper.c b/crypto/bio/socket_helper.c index 0b62974b92..217d798dba 100644 --- a/crypto/bio/socket_helper.c +++ b/crypto/bio/socket_helper.c @@ -110,12 +110,14 @@ int bio_socket_nbio(int sock, int on) { #endif } -void bio_clear_socket_error(void) {} +void bio_clear_socket_error(int sock) { + bio_sock_error_get_and_clear(sock); +} -int bio_sock_error(int sock) { +int bio_sock_error_get_and_clear(int sock) { int error; socklen_t error_size = sizeof(error); - + // Get and clear the pending socket error. The SO_ERROR option is read-only. if (getsockopt(sock, SOL_SOCKET, SO_ERROR, (char *)&error, &error_size) < 0) { return 1; } From cd051224d3f17fd4500438c5cde67ecf7e05cb29 Mon Sep 17 00:00:00 2001 From: Justin Smith Date: Tue, 8 Apr 2025 10:50:02 -0400 Subject: [PATCH 3/9] More cleanup --- crypto/bio/bio.c | 4 + crypto/bio/dgram.c | 171 +++++++++++++++++++++++------------------- include/openssl/bio.h | 49 ++++++++---- 3 files changed, 130 insertions(+), 94 deletions(-) diff --git a/crypto/bio/bio.c b/crypto/bio/bio.c index d483003503..10856e2b4f 100644 --- a/crypto/bio/bio.c +++ b/crypto/bio/bio.c @@ -599,6 +599,10 @@ int BIO_set_close(BIO *bio, int close_flag) { return (int)BIO_ctrl(bio, BIO_CTRL_SET_CLOSE, close_flag, NULL); } +int BIO_get_close(BIO *bio) { + return (int)BIO_ctrl(bio, BIO_CTRL_GET_CLOSE, 0, NULL); +} + OPENSSL_EXPORT uint64_t BIO_number_read(const BIO *bio) { return bio->num_read; } diff --git a/crypto/bio/dgram.c b/crypto/bio/dgram.c index 11475971b5..fafc3680ea 100644 --- a/crypto/bio/dgram.c +++ b/crypto/bio/dgram.c @@ -2,20 +2,24 @@ // SPDX-License-Identifier: Apache-2.0 OR ISC #include + +#if !defined(OPENSSL_NO_SOCK) + #include +#include #if defined(OPENSSL_WINDOWS) -#include -#include +typedef SSIZE_T ssize_t; +#if !defined(__MINGW32__) #include -#include +#endif #else #include #include +#include #include #include #endif -#include #include "../internal.h" #include "./internal.h" @@ -24,6 +28,14 @@ static int closesocket(const int sock) { return close(sock); } #endif +#if defined(AF_UNIX) && !defined(OPENSSL_WINDOWS) +// Winsock2 APIs don't support AF_UNIX. +// > The values currently supported are AF_INET or AF_INET6, which are the +// > Internet address family formats for IPv4 and IPv6. +// https://learn.microsoft.com/en-us/windows/win32/api/winsock2/nf-winsock2-socket +#define AWS_LC_HAS_AF_UNIX 1 +#endif + /* * BIO_ADDR_make - non-public routine to fill a BIO_ADDR with the contents * of a struct sockaddr. @@ -42,7 +54,7 @@ static int BIO_ADDR_make(BIO_ADDR *bap, const struct sockaddr *sap) { return 1; } #endif -#ifdef AF_UNIX +#ifdef AWS_LC_HAS_AF_UNIX if (sap->sa_family == AF_UNIX) { OPENSSL_memcpy(&bap->s_un, sap, sizeof(struct sockaddr_un)); return 1; @@ -56,7 +68,6 @@ typedef struct bio_dgram_data_st { BIO_ADDR peer; unsigned int connected; unsigned int _errno; - unsigned int mtu; } bio_dgram_data; static socklen_t BIO_ADDR_sockaddr_size(const BIO_ADDR *bap) { @@ -69,7 +80,7 @@ static socklen_t BIO_ADDR_sockaddr_size(const BIO_ADDR *bap) { return sizeof(bap->s_in6); } #endif -#ifdef AF_UNIX +#ifdef AWS_LC_HAS_AF_UNIX if (bap->sa.sa_family == AF_UNIX) { return sizeof(bap->s_un); } @@ -77,8 +88,6 @@ static socklen_t BIO_ADDR_sockaddr_size(const BIO_ADDR *bap) { return sizeof(*bap); } - - static struct sockaddr *BIO_ADDR_sockaddr_noconst(BIO_ADDR *bap) { GUARD_PTR(bap); return &bap->sa; @@ -93,12 +102,13 @@ static int dgram_write(BIO *bp, const char *in, const int in_len) { GUARD_PTR(bp); GUARD_PTR(in); - bio_dgram_data *data = bp->ptr; ssize_t result; + bio_dgram_data *data = bp->ptr; + GUARD_PTR(data); - if (in_len <= 0) { + if (in_len < 0) { OPENSSL_PUT_ERROR(BIO, BIO_R_INVALID_ARGUMENT); - return 0; + return -1; } bio_clear_socket_error(bp->num); @@ -106,14 +116,16 @@ static int dgram_write(BIO *bp, const char *in, const int in_len) { // With a zero flags argument, send() is equivalent to write(2). result = send(bp->num, in, in_len, 0); } else { - // If sendto() is used on a connection-mode (SOCK_STREAM, SOCK_SEQPACKET) - // socket, the arguments dest_addr and addrlen are ignored + // If a peer address has been pre-specified, sendto may return -1 and set + // errno to[EISCONN]. const socklen_t peerlen = BIO_ADDR_sockaddr_size(&data->peer); - result = sendto(bp->num, in, in_len, 0, BIO_ADDR_sockaddr(&data->peer), peerlen); + result = + sendto(bp->num, in, in_len, 0, BIO_ADDR_sockaddr(&data->peer), peerlen); } if (result < INT_MIN || result > INT_MAX) { - abort(); + OPENSSL_PUT_ERROR(BIO, BIO_R_SYS_LIB); + return -1; } const int ret = result; @@ -128,17 +140,25 @@ static int dgram_write(BIO *bp, const char *in, const int in_len) { static int dgram_read(BIO *bp, char *out, const int out_len) { GUARD_PTR(bp); GUARD_PTR(out); + GUARD_PTR(bp->ptr); - bio_dgram_data *data = bp->ptr; - BIO_ADDR peer = {0}; + BIO_ADDR peer; + // Might be modified by call to `recvfrom`. socklen_t len = sizeof(peer); + bio_dgram_data *data = bp->ptr; bio_clear_socket_error(bp->num); + if (out_len < 0) { + // out_len is cast to `size_t` below. + OPENSSL_PUT_ERROR(BIO, BIO_R_INVALID_ARGUMENT); + return -1; + } + // recvfrom may be used to receive data on a socket regardless of whether // it's connection-oriented. const ssize_t result = recvfrom(bp->num, out, out_len, 0, - BIO_ADDR_sockaddr_noconst(&peer), &len); + BIO_ADDR_sockaddr_noconst(&peer), &len); if (result < INT_MIN || result > INT_MAX) { abort(); @@ -146,7 +166,6 @@ static int dgram_read(BIO *bp, char *out, const int out_len) { const int ret = result; if (!data->connected && ret >= 0) { - assert(len == BIO_ADDR_sockaddr_size(&peer)); BIO_ctrl(bp, BIO_CTRL_DGRAM_SET_PEER, 0, &peer); } @@ -181,6 +200,13 @@ static int dgram_free(BIO *bp) { return 1; } +static int dgram_new(BIO *bio) { + bio->init = 0; + bio->num = -1; + bio->ptr = OPENSSL_zalloc(sizeof(bio_dgram_data)); + return bio->ptr != NULL; +} + static long dgram_ctrl(BIO *bp, const int cmd, const long num, void *ptr) { GUARD_PTR(bp); bio_dgram_data *data = bp->ptr; @@ -190,15 +216,19 @@ static long dgram_ctrl(BIO *bp, const int cmd, const long num, void *ptr) { switch (cmd) { case BIO_C_SET_FD: GUARD_PTR(ptr); - if (0 == dgram_free(bp)) { - assert(0); + int fd = *(int *)ptr; + if (fd < 0) { + // file descriptors must be non-negative. + OPENSSL_PUT_ERROR(BIO, BIO_R_INVALID_ARGUMENT); + return 0; } - bp->num = *(int*)ptr; + dgram_free(bp); + dgram_new(bp); + bp->num = fd; bp->shutdown = (int)num; bp->init = 1; break; case BIO_C_GET_FD: - GUARD_PTR(ptr); if (bp->init) { int *ip = ptr; if (ip) { @@ -213,20 +243,11 @@ static long dgram_ctrl(BIO *bp, const int cmd, const long num, void *ptr) { ret = bp->shutdown; break; case BIO_CTRL_SET_CLOSE: - bp->shutdown = (num != 0); + bp->shutdown = num != 0; break; case BIO_CTRL_FLUSH: ret = 1; break; - case BIO_CTRL_DGRAM_GET_MTU: - GUARD_PTR(data); - ret = data->mtu; - break; - case BIO_CTRL_DGRAM_SET_MTU: - GUARD_PTR(data); - data->mtu = num; - ret = num; - break; case BIO_CTRL_DGRAM_SET_CONNECTED: GUARD_PTR(data); if (ptr != NULL) { @@ -238,16 +259,11 @@ static long dgram_ctrl(BIO *bp, const int cmd, const long num, void *ptr) { ret = 1; } break; - case BIO_CTRL_DGRAM_CONNECT: - GUARD_PTR(data); - GUARD_PTR(ptr); - ret = BIO_ADDR_make(&data->peer, BIO_ADDR_sockaddr(ptr)); - break; case BIO_CTRL_DGRAM_GET_PEER: { GUARD_PTR(data); GUARD_PTR(ptr); const socklen_t size = BIO_ADDR_sockaddr_size(&data->peer); - if (num == 0 || num > size) { + if (num == 0 || num >= (long)size) { OPENSSL_memcpy(ptr, &data->peer, size); ret = size; } else { @@ -255,6 +271,7 @@ static long dgram_ctrl(BIO *bp, const int cmd, const long num, void *ptr) { } break; } + case BIO_CTRL_DGRAM_CONNECT: case BIO_CTRL_DGRAM_SET_PEER: GUARD_PTR(data); GUARD_PTR(ptr); @@ -262,26 +279,26 @@ static long dgram_ctrl(BIO *bp, const int cmd, const long num, void *ptr) { break; case BIO_CTRL_DGRAM_GET_SEND_TIMER_EXP: case BIO_CTRL_DGRAM_GET_RECV_TIMER_EXP: { - GUARD_PTR(data); int d_errno = 0; -# ifdef OPENSSL_WINDOWS +#ifdef OPENSSL_WINDOWS d_errno = (data->_errno == WSAETIMEDOUT); -# else +#else /* - * if no data has been transferred and the timeout has been reached, - * then -1 is returned with errno set to EAGAIN or EWOULDBLOCK, - * or EINPROGRESS (for connect(2)) just as if the socket was specified - * to be nonblocking. - */ - d_errno = (data->_errno == EAGAIN) || (data->_errno == EWOULDBLOCK) || - (data->_errno == EINPROGRESS); -# endif + * if no data has been transferred and the timeout has been reached, + * then -1 is returned with errno set to EAGAIN or EWOULDBLOCK, + * or EINPROGRESS (for connect(2)) just as if the socket was specified + * to be nonblocking. + */ + d_errno = data->_errno == EAGAIN || data->_errno == EWOULDBLOCK || + data->_errno == EINPROGRESS; +#endif if (d_errno) { ret = 1; data->_errno = 0; - } else + } else { ret = 0; + } break; } default: @@ -291,14 +308,7 @@ static long dgram_ctrl(BIO *bp, const int cmd, const long num, void *ptr) { return ret; } -static int dgram_new(BIO *bio) { - bio_dgram_data *data = OPENSSL_zalloc(sizeof(*data)); - if (!data) { - return 0; - } - bio->ptr = data; - return 1; -} + static const BIO_METHOD methods_dgramp = { .type = BIO_TYPE_DGRAM, @@ -315,61 +325,66 @@ static const BIO_METHOD methods_dgramp = { const BIO_METHOD *BIO_s_datagram(void) { return &methods_dgramp; } -BIO *BIO_new_dgram(int fd, int close_flag) -{ - BIO *ret; - - ret = BIO_new(BIO_s_datagram()); - if (ret == NULL) +BIO *BIO_new_dgram(const int fd, const int close_flag) { + BIO *ret = BIO_new(BIO_s_datagram()); + if (ret == NULL) { + return NULL; + } + int result = BIO_set_fd(ret, fd, close_flag); + if (result <= 0) { + BIO_free(ret); return NULL; - BIO_set_fd(ret, fd, close_flag); + } return ret; } int BIO_ctrl_dgram_connect(BIO *bp, const BIO_ADDR *peer) { - long ret = BIO_ctrl(bp, BIO_CTRL_DGRAM_CONNECT, 0, (BIO_ADDR*)peer); + const long ret = BIO_ctrl(bp, BIO_CTRL_DGRAM_CONNECT, 0, (BIO_ADDR *)peer); if (ret < INT_MIN || ret > INT_MAX) { return 0; } return ret; } -int BIO_ctrl_set_connected(BIO* bp, const BIO_ADDR *peer) { - long ret = BIO_ctrl(bp, BIO_CTRL_DGRAM_SET_CONNECTED, 0, (BIO_ADDR*)peer); +int BIO_ctrl_set_connected(BIO *bp, const BIO_ADDR *peer) { + const long ret = + BIO_ctrl(bp, BIO_CTRL_DGRAM_SET_CONNECTED, 0, (BIO_ADDR *)peer); if (ret < INT_MIN || ret > INT_MAX) { return 0; } return ret; } -int BIO_dgram_recv_timedout(BIO* bp) { - long ret = BIO_ctrl(bp, BIO_CTRL_DGRAM_GET_RECV_TIMER_EXP, 0, NULL); +int BIO_dgram_recv_timedout(BIO *bp) { + const long ret = BIO_ctrl(bp, BIO_CTRL_DGRAM_GET_RECV_TIMER_EXP, 0, NULL); if (ret < INT_MIN || ret > INT_MAX) { return 0; } return ret; } -int BIO_dgram_send_timedout(BIO* bp) { - long ret = BIO_ctrl(bp, BIO_CTRL_DGRAM_GET_SEND_TIMER_EXP, 0, NULL); +int BIO_dgram_send_timedout(BIO *bp) { + const long ret = BIO_ctrl(bp, BIO_CTRL_DGRAM_GET_SEND_TIMER_EXP, 0, NULL); if (ret < INT_MIN || ret > INT_MAX) { return 0; } return ret; } -int BIO_dgram_get_peer(BIO* bp, BIO_ADDR *peer) { - long ret = BIO_ctrl(bp, BIO_CTRL_DGRAM_GET_PEER, 0, peer); +int BIO_dgram_get_peer(BIO *bp, BIO_ADDR *peer) { + const long ret = BIO_ctrl(bp, BIO_CTRL_DGRAM_GET_PEER, 0, peer); if (ret < INT_MIN || ret > INT_MAX) { return 0; } return ret; } -int BIO_dgram_set_peer(BIO* bp, const BIO_ADDR *peer) { - long ret = BIO_ctrl(bp, BIO_CTRL_DGRAM_SET_PEER, 0, (BIO_ADDR*)peer); +int BIO_dgram_set_peer(BIO *bp, const BIO_ADDR *peer) { + const long ret = BIO_ctrl(bp, BIO_CTRL_DGRAM_SET_PEER, 0, (BIO_ADDR *)peer); if (ret < INT_MIN || ret > INT_MAX) { return 0; } return ret; } + +#endif diff --git a/include/openssl/bio.h b/include/openssl/bio.h index b5ed19076e..60a1d81aff 100644 --- a/include/openssl/bio.h +++ b/include/openssl/bio.h @@ -70,8 +70,10 @@ #if defined(OPENSSL_WINDOWS) #include #include -#include #include +#if !defined(__MINGW32__) +#include +#endif #else #include #include @@ -340,6 +342,9 @@ OPENSSL_EXPORT size_t BIO_wpending(const BIO *bio); // otherwise. OPENSSL_EXPORT int BIO_set_close(BIO *bio, int close_flag); +// BIO_get_close returns the close flag for |bio|. +OPENSSL_EXPORT int BIO_get_close(BIO *bio); + // BIO_number_read returns the number of bytes that have been read from // |bio|. OPENSSL_EXPORT uint64_t BIO_number_read(const BIO *bio); @@ -714,7 +719,7 @@ typedef union bio_addr_st { struct sockaddr_in6 s_in6; #endif struct sockaddr_in s_in; -#ifdef AF_UNIX +#if defined(AF_UNIX) && !defined(OPENSSL_WINDOWS) struct sockaddr_un s_un; #endif } BIO_ADDR; @@ -743,31 +748,43 @@ typedef union bio_addr_st { #define BIO_CTRL_DGRAM_GET_FALLBACK_MTU 47 +// BIO_s_datagram returns the datagram |BIO_METHOD|. A datagram BIO provides a wrapper +// around datagram sockets. OPENSSL_EXPORT const BIO_METHOD *BIO_s_datagram(void); -// TODO: documentation -OPENSSL_EXPORT BIO *BIO_new_dgram(int fd, int close_flag); +// BIO_new_dgram creates a new datagram BIO wrapping |fd|. If |close_flag| is +// non-zero, then |fd| will be closed when the BIO is freed. +OPENSSL_EXPORT BIO *BIO_new_dgram(const int fd, const int close_flag); -// TODO: documentation -OPENSSL_EXPORT int BIO_ctrl_dgram_connect(BIO *bio, const BIO_ADDR *peer); +// BIO_ctrl_dgram_connect attempts to connect the datagram BIO to the specified +// |peer| address. It returns 1 on success and a non-positive value on error. +OPENSSL_EXPORT int BIO_ctrl_dgram_connect(BIO *bp, const BIO_ADDR *peer); -// TODO: documentation -OPENSSL_EXPORT int BIO_ctrl_set_connected(BIO* bp, const BIO_ADDR *sa); +// BIO_ctrl_set_connected marks the datagram BIO as connected to the specified +// |peer| address. This is used for handling DTLS connection-oriented BIOs. +// It returns 1 on success and a non-positive value on error. +OPENSSL_EXPORT int BIO_ctrl_set_connected(BIO* bp, const BIO_ADDR *peer); -// TODO: documentation +// BIO_dgram_recv_timedout returns 1 if the most recent datagram receive +// operation on |bp| timed out, and a non-positive value otherwise. OPENSSL_EXPORT int BIO_dgram_recv_timedout(BIO* bp); -// TODO: documentation +// BIO_dgram_send_timedout returns 1 if the most recent datagram send +// operation on |bp| timed out, and a non-positive value otherwise. OPENSSL_EXPORT int BIO_dgram_send_timedout(BIO* bp); -// TODO: documentation -OPENSSL_EXPORT int BIO_dgram_get_peer(BIO* bp, BIO_ADDR *sa); +// BIO_dgram_get_peer stores the address of the peer the datagram BIO is +// connected to in |peer|. It returns 1 on success and a non-positive value on error. +OPENSSL_EXPORT int BIO_dgram_get_peer(BIO* bp, BIO_ADDR *peer); -// TODO: documentation -OPENSSL_EXPORT int BIO_dgram_set_peer(BIO* bp, const BIO_ADDR *sa); +// BIO_dgram_set_peer sets the peer address for the datagram BIO to |peer|. +// It returns 1 on success and a non-positive value on error. +OPENSSL_EXPORT int BIO_dgram_set_peer(BIO* bp, const BIO_ADDR *peer); -// TODO: documentation -OPENSSL_EXPORT unsigned int BIO_dgram_get_mtu_overhead(BIO* bp, struct sockaddr *sa); +// BIO_dgram_get_mtu_overhead returns the number of bytes of overhead when sending +// a datagram of the maximum size through |bp| to the specified |peer| address. +// This is used for PMTU discovery in DTLS. +OPENSSL_EXPORT unsigned int BIO_dgram_get_mtu_overhead(BIO* bp, struct sockaddr *peer); // BIO Pairs. // From 14088cc6cc51c18fabfa19458e1d06cee2bb8507 Mon Sep 17 00:00:00 2001 From: Justin Smith Date: Thu, 10 Apr 2025 09:56:17 -0400 Subject: [PATCH 4/9] Add BIO datagram testing --- crypto/bio/bio_socket_test.cc | 494 ++++++++++++++++++++++++++++------ crypto/bio/dgram.c | 2 +- 2 files changed, 413 insertions(+), 83 deletions(-) diff --git a/crypto/bio/bio_socket_test.cc b/crypto/bio/bio_socket_test.cc index 1c4561098f..045437a6ad 100644 --- a/crypto/bio/bio_socket_test.cc +++ b/crypto/bio/bio_socket_test.cc @@ -4,41 +4,40 @@ #include #include -#include #include #include -#include #include #include #include "../internal.h" -#include "../test/file_util.h" #include "../test/test_util.h" #if !defined(OPENSSL_WINDOWS) #include -#include #include #include #include -#include #include #include #else -#include #include +#include OPENSSL_MSVC_PRAGMA(warning(push, 3)) #include #include OPENSSL_MSVC_PRAGMA(warning(pop)) #endif +#if defined(AF_UNIX) && !defined(OPENSSL_WINDOWS) && !defined(OPENSSL_ANDROID) +#define AWS_LC_HAS_AF_UNIX 1 +#endif + #if !defined(OPENSSL_WINDOWS) using Socket = int; #define INVALID_SOCKET (-1) -static int closesocket(int sock) { return close(sock); } +static int closesocket(const int sock) { return close(sock); } static std::string LastSocketError() { return strerror(errno); } #else using Socket = SOCKET; @@ -49,37 +48,6 @@ static std::string LastSocketError() { } #endif -class OwnedSocket { - public: - OwnedSocket() = default; - explicit OwnedSocket(Socket sock) : sock_(sock) {} - OwnedSocket(OwnedSocket &&other) { *this = std::move(other); } - ~OwnedSocket() { reset(); } - OwnedSocket &operator=(OwnedSocket &&other) { - reset(other.release()); - return *this; - } - - bool is_valid() const { return sock_ != INVALID_SOCKET; } - Socket get() const { return sock_; } - Socket release() { - Socket temp = std::move(sock_); - sock_ = INVALID_SOCKET; - return temp; - } - - void reset(Socket sock = INVALID_SOCKET) { - if (is_valid()) { - closesocket(sock_); - } - - sock_ = sock; - } - - private: - Socket sock_ = INVALID_SOCKET; -}; - struct SockaddrStorage { SockaddrStorage() : storage(), len(sizeof(storage)) {} @@ -92,71 +60,237 @@ struct SockaddrStorage { sockaddr_in ToIPv4() const { if (family() != AF_INET || len != sizeof(sockaddr_in)) { - abort(); + ADD_FAILURE() << LastSocketError(); + return sockaddr_in(); // Initialize to zero } - // These APIs were seemingly designed before C's strict aliasing rule, and - // C++'s strict union handling. Make a copy so the compiler does not read - // this as an aliasing violation. - sockaddr_in ret; + // Make a copy so the compiler does not read this as an aliasing violation. + sockaddr_in ret; // Initialize to zero OPENSSL_memcpy(&ret, &storage, sizeof(ret)); return ret; } sockaddr_in6 ToIPv6() const { if (family() != AF_INET6 || len != sizeof(sockaddr_in6)) { - abort(); + ADD_FAILURE() << LastSocketError(); + return sockaddr_in6(); } - // These APIs were seemingly designed before C's strict aliasing rule, and - // C++'s strict union handling. Make a copy so the compiler does not read - // this as an aliasing violation. + // Make a copy so the compiler does not read this as an aliasing violation. sockaddr_in6 ret; OPENSSL_memcpy(&ret, &storage, sizeof(ret)); return ret; } +#ifdef AWS_LC_HAS_AF_UNIX + sockaddr_un ToUnix() const { + if (family() != AF_UNIX || len != sizeof(sockaddr_un)) { + ADD_FAILURE() << LastSocketError(); + return sockaddr_un(); + } + // Make a copy so the compiler does not read this as an aliasing violation. + sockaddr_un ret; + OPENSSL_memcpy(&ret, &storage, sizeof(ret)); + return ret; + } +#endif + BIO_ADDR ToBIO_ADDR() const { + BIO_ADDR bap; + switch (family()) { + case AF_INET: + OPENSSL_memcpy(&bap.s_in, &storage, sizeof(sockaddr_in)); + break; +#ifdef AWS_LC_HAS_AF_UNIX + case AF_UNIX: + OPENSSL_memcpy(&bap.s_un, &storage, sizeof(sockaddr_un)); + break; +#endif + default: + OPENSSL_memcpy(&bap.s_in6, &storage, sizeof(sockaddr_in6)); + break; + } + return bap; + } sockaddr_storage storage; socklen_t len; }; -static OwnedSocket Bind(int family, const sockaddr *addr, socklen_t addr_len) { - OwnedSocket sock(socket(family, SOCK_STREAM, 0)); +static bool operator==(const BIO_ADDR &lhs, const BIO_ADDR &rhs) { + const sockaddr *lhs_sa = reinterpret_cast(&lhs); + const sockaddr *rhs_sa = reinterpret_cast(&rhs); + if (lhs_sa->sa_family != rhs_sa->sa_family) { + return false; + } + + if (lhs_sa->sa_family == AF_INET) { + return !OPENSSL_memcmp(&lhs, &rhs, sizeof(sockaddr_in)); + } +#ifdef AF_INET6 + if (lhs_sa->sa_family == AF_INET6) { + return !OPENSSL_memcmp(&lhs, &rhs, sizeof(sockaddr_in6)); + } +#endif +#ifdef AWS_LC_HAS_AF_UNIX + if (lhs_sa->sa_family == AF_UNIX) { + return !OPENSSL_memcmp(&lhs, &rhs, sizeof(sockaddr_un)); + } +#endif + + return 0; +} + +class OwnedSocket { + public: + OwnedSocket() = default; + explicit OwnedSocket(const Socket sock) : sock_(sock) {} + OwnedSocket(OwnedSocket &&other) { *this = std::move(other); } + ~OwnedSocket() { reset(); } + OwnedSocket &operator=(OwnedSocket &&other) { + reset(other.release()); + return *this; + } + + bool is_valid() const { return sock_ > 0; } + Socket get() const { return sock_; } + Socket release() { + const Socket temp = sock_; + sock_ = INVALID_SOCKET; + return temp; + } + + int type() const { + int type = 0; + socklen_t length = sizeof(int); + getsockopt(get(), SOL_SOCKET, SO_TYPE, (char *)&type, &length); + return type; + } + + SockaddrStorage storage() const { + SockaddrStorage server_addr; + if (0 != getsockname(get(), server_addr.addr_mut(), &server_addr.len)) { + ADD_FAILURE() << LastSocketError(); + } + return server_addr; + } + + void reset(const Socket sock = INVALID_SOCKET) { + if (is_valid()) { + closesocket(sock_); + } + + sock_ = sock; + } + + private: + Socket sock_ = INVALID_SOCKET; +}; + +static OwnedSocket Bind(const int family, const int type, const sockaddr *addr, + const socklen_t addr_len) { + OwnedSocket sock(socket(family, type, 0)); if (!sock.is_valid()) { - return OwnedSocket(); + return {}; } + // If no port given (e.g., addr.sin_port == 0), one is selected arbitrarily. if (bind(sock.get(), addr, addr_len) != 0) { - return OwnedSocket(); + return {}; } return sock; } -static OwnedSocket ListenLoopback(int backlog) { - // Try binding to IPv6. - sockaddr_in6 sin6; - OPENSSL_memset(&sin6, 0, sizeof(sin6)); - sin6.sin6_family = AF_INET6; - if (inet_pton(AF_INET6, "::1", &sin6.sin6_addr) != 1) { - return OwnedSocket(); +#ifdef AWS_LC_HAS_AF_UNIX +static int prepare_unix_domain_socket(sockaddr_un *sun) { + assert(sun != NULL); + + OPENSSL_cleanse(sun, sizeof(sun)); + sun->sun_family = AF_UNIX; + + char dir_buffer[PATH_MAX] = {0}; + char file_buffer[PATH_MAX] = {0}; + + const size_t tmp_dir_len = createTempDirPath(dir_buffer); + const size_t tmp_file_len = createTempFILEpath(file_buffer); + + const size_t tmp_combined_len = tmp_dir_len + tmp_file_len + 1; + if (tmp_dir_len == 0 || tmp_file_len == 0 || + tmp_combined_len >= sizeof(sun->sun_path) || + tmp_combined_len >= PATH_MAX) { + return 0; } - OwnedSocket sock = - Bind(AF_INET6, reinterpret_cast(&sin6), sizeof(sin6)); - if (!sock.is_valid()) { + OPENSSL_memcpy((void *)sun->sun_path, (void *)dir_buffer, tmp_dir_len); + sun->sun_path[tmp_dir_len] = '/'; + OPENSSL_memcpy((void *)(sun->sun_path + tmp_dir_len + 1), (void *)file_buffer, + tmp_file_len); + sun->sun_path[tmp_combined_len] = '\0'; + return 1; +} +#endif + +static OwnedSocket ListenLoopback(int type, int addr_family = 0, + int backlog = 1) { + OwnedSocket sock; + if (addr_family == 0 || addr_family == AF_INET6) { + // Try binding to IPv6 + sockaddr_in6 sin6; + OPENSSL_cleanse(&sin6, sizeof(sin6)); + sin6.sin6_family = AF_INET6; + if (inet_pton(AF_INET6, "::1", &sin6.sin6_addr) != 1) { + ADD_FAILURE() << LastSocketError(); + return OwnedSocket(); + } + sock = Bind(AF_INET6, type, reinterpret_cast(&sin6), + sizeof(sin6)); + } + if (addr_family == AF_INET || (addr_family == 0 && !sock.is_valid())) { // Try binding to IPv4. sockaddr_in sin; - OPENSSL_memset(&sin, 0, sizeof(sin)); + OPENSSL_cleanse(&sin, sizeof(sin)); sin.sin_family = AF_INET; if (inet_pton(AF_INET, "127.0.0.1", &sin.sin_addr) != 1) { + ADD_FAILURE() << LastSocketError(); + return OwnedSocket(); + return OwnedSocket(); + } + sock = Bind(AF_INET, type, reinterpret_cast(&sin), + sizeof(sin)); + + if (!sock.is_valid()) { + ADD_FAILURE() << LastSocketError(); return OwnedSocket(); } - sock = Bind(AF_INET, reinterpret_cast(&sin), sizeof(sin)); - } - if (!sock.is_valid()) { - return OwnedSocket(); } +#ifdef AWS_LC_HAS_AF_UNIX + if (addr_family == AF_UNIX) { + sockaddr_un sun; + if (1 != prepare_unix_domain_socket(&sun)) { + ADD_FAILURE() << LastSocketError(); + return OwnedSocket(); + } + + // Try binding to Unix domain socket + sock = Bind(AF_UNIX, type, reinterpret_cast(&sun), + sizeof(sun)); + if (!sock.is_valid()) { + ADD_FAILURE() << LastSocketError(); + unlink((char *)sun.sun_path); + return OwnedSocket(); + } - if (listen(sock.get(), backlog) != 0) { - return OwnedSocket(); + // Set socket permissions (optional) + if (chmod(sun.sun_path, 0660) == -1) { + ADD_FAILURE() << LastSocketError(); + unlink((char *)sun.sun_path); + return OwnedSocket(); + } + } +#endif + // Mark the socket as being used to accept incoming connection requests using + // accept(2). Socket must be of type SOCK_STREAM or SOCK_SEQPACKET. + if (type == SOCK_STREAM || type == SOCK_SEQPACKET) { + if (listen(sock.get(), backlog) != 0) { + ADD_FAILURE() << LastSocketError(); + return OwnedSocket(); + } } return sock; @@ -179,33 +313,31 @@ static bool SocketSetNonBlocking(Socket sock) { enum class WaitType { kRead, kWrite }; static bool WaitForSocket(Socket sock, WaitType wait_type) { - // Use an arbitrary 5 second timeout, so the test doesn't hang indefinitely if + // Use an arbitrary 5-second timeout, so the test doesn't hang indefinitely if // there's an issue. - static const int kTimeoutSeconds = 5; + static constexpr int kTimeoutSeconds = 5; #if defined(OPENSSL_WINDOWS) fd_set read_set, write_set; FD_ZERO(&read_set); FD_ZERO(&write_set); fd_set *wait_set = wait_type == WaitType::kRead ? &read_set : &write_set; FD_SET(sock, wait_set); - timeval timeout; - timeout.tv_sec = kTimeoutSeconds; - timeout.tv_usec = 0; + timeval timeout = {kTimeoutSeconds, 0}; if (select(0 /* unused on Windows */, &read_set, &write_set, nullptr, &timeout) <= 0) { return false; } return FD_ISSET(sock, wait_set); #else - short events = wait_type == WaitType::kRead ? POLLIN : POLLOUT; - pollfd fd = {/*fd=*/sock, events, /*revents=*/0}; + const short events = wait_type == WaitType::kRead ? POLLIN : POLLOUT; + pollfd fd = {.fd = sock, .events = events, .revents = 0}; return poll(&fd, 1, kTimeoutSeconds * 1000) == 1 && (fd.revents & events); #endif } TEST(BIOTest, SocketConnect) { - static const char kTestMessage[] = "test"; - OwnedSocket listening_sock = ListenLoopback(/*backlog=*/1); + static constexpr char kTestMessage[] = "test"; + const OwnedSocket listening_sock = ListenLoopback(SOCK_STREAM); ASSERT_TRUE(listening_sock.is_valid()) << LastSocketError(); SockaddrStorage addr; @@ -222,7 +354,7 @@ TEST(BIOTest, SocketConnect) { } // Connect to it with a connect BIO. - bssl::UniquePtr bio(BIO_new_connect(hostname)); + const bssl::UniquePtr bio(BIO_new_connect(hostname)); ASSERT_TRUE(bio); // Write a test message to the BIO. This is assumed to be smaller than the @@ -232,7 +364,8 @@ TEST(BIOTest, SocketConnect) { << LastSocketError(); // Accept the socket. - OwnedSocket sock(accept(listening_sock.get(), addr.addr_mut(), &addr.len)); + const OwnedSocket sock( + accept(listening_sock.get(), addr.addr_mut(), &addr.len)); ASSERT_TRUE(sock.is_valid()) << LastSocketError(); // Check the same message is read back out. @@ -240,11 +373,11 @@ TEST(BIOTest, SocketConnect) { ASSERT_EQ(static_cast(sizeof(kTestMessage)), recv(sock.get(), buf, sizeof(buf), 0)) << LastSocketError(); - EXPECT_EQ(Bytes(kTestMessage, sizeof(kTestMessage)), Bytes(buf, sizeof(buf))); + ASSERT_EQ(Bytes(kTestMessage, sizeof(kTestMessage)), Bytes(buf, sizeof(buf))); } TEST(BIOTest, SocketNonBlocking) { - OwnedSocket listening_sock = ListenLoopback(/*backlog=*/1); + OwnedSocket listening_sock = ListenLoopback(SOCK_STREAM); ASSERT_TRUE(listening_sock.is_valid()) << LastSocketError(); // Connect to |listening_sock|. @@ -270,7 +403,7 @@ TEST(BIOTest, SocketNonBlocking) { ASSERT_TRUE(accept_bio); // Exchange data through the socket. - static const char kTestMessage[] = "hello, world"; + static constexpr char kTestMessage[] = "hello, world"; // Reading from |accept_bio| should not block. char buf[sizeof(kTestMessage)]; @@ -325,3 +458,200 @@ TEST(BIOTest, SocketNonBlocking) { EXPECT_EQ(ret, 0) << LastSocketError(); EXPECT_FALSE(BIO_should_read(accept_bio.get())); } + +static bssl::UniquePtr create_server_bio(int addr_family, int type) { + // Create a server socket. + OwnedSocket server_sock(ListenLoopback(type, addr_family)); + if (!server_sock.is_valid()) { + ADD_FAILURE() << LastSocketError(); + return nullptr; + } + + // Wrap the server socket in a BIO. + return bssl::UniquePtr(BIO_new_dgram(server_sock.release(), BIO_CLOSE)); +} + +static bssl::UniquePtr create_client_bio(int addr_family, int type) { + OwnedSocket client_sock; + +#ifdef AWS_LC_HAS_AF_UNIX + // Create a client socket. + if (addr_family == AF_UNIX) { + // Unix domain sockets must be configured with a file in order to + // receive a message. + sockaddr_un sun; + if (1 != prepare_unix_domain_socket(&sun)) { + ADD_FAILURE() << LastSocketError(); + return nullptr; + } + + // Try binding to Unix domain socket + client_sock = Bind(AF_UNIX, type, reinterpret_cast(&sun), + sizeof(sun)); + } else +#endif + { + client_sock.reset(socket(addr_family, type, 0)); + } + if (!client_sock.is_valid()) { + ADD_FAILURE() << LastSocketError(); + return nullptr; + } + + // Wrap the client socket in a BIO. + return bssl::UniquePtr(BIO_new_dgram(client_sock.release(), BIO_CLOSE)); +} + +static void test_send_receive(bssl::UniquePtr &sender_bio, + bssl::UniquePtr &receiver_bio) { + static constexpr char kTestMessage[] = "test"; + + // Send a message + ASSERT_EQ((int)strlen(kTestMessage) + 1, + BIO_write(sender_bio.get(), kTestMessage, sizeof(kTestMessage))) + << LastSocketError(); + // BIO_flush is a no-op, but test it anyway. + ASSERT_EQ(1, BIO_flush(sender_bio.get())) << LastSocketError(); + + // Receive a message + char buff[1024]; + ASSERT_EQ((int)strlen(kTestMessage) + 1, + BIO_read(receiver_bio.get(), buff, sizeof(buff))) + << LastSocketError(); + + // Verify the message received matches the message sent. + ASSERT_EQ(Bytes(buff), Bytes(kTestMessage)); +} + +class BIODgramTest : public testing::TestWithParam { + // You can implement all the usual fixture class members here. + // To access the test parameter, call GetParam() from class + // TestWithParam. +}; + + +#if defined(AF_INET6) +#if defined(AWS_LC_HAS_AF_UNIX) +INSTANTIATE_TEST_SUITE_P(AddrFamily, BIODgramTest, + testing::Values(AF_INET, AF_INET6, AF_UNIX)); +#else +INSTANTIATE_TEST_SUITE_P(AddrFamily, BIODgramTest, + testing::Values(AF_INET, AF_INET6)); +#endif +#elif defined(AWS_LC_HAS_AF_UNIX) +INSTANTIATE_TEST_SUITE_P(AddrFamily, BIODgramTest, + testing::Values(AF_INET, AF_UNIX)); +#else +INSTANTIATE_TEST_SUITE_P(AddrFamily, BIODgramTest, testing::Values(AF_INET)); +#endif + +TEST_P(BIODgramTest, SocketDatagramSetPeer) { + int addr_family = GetParam(); + // Wrap the server socket in a BIO. + bssl::UniquePtr server_bio = create_server_bio(addr_family, SOCK_DGRAM); + ASSERT_TRUE(server_bio) << LastSocketError(); + ASSERT_EQ(1, BIO_get_close(server_bio.get())) << LastSocketError(); + + OwnedSocket server_sock(BIO_get_fd(server_bio.get(), NULL)); + ASSERT_EQ(1, BIO_set_close(server_bio.get(), BIO_NOCLOSE)) + << LastSocketError(); + SockaddrStorage server_addr = server_sock.storage(); + + // Get the server socket's address + bssl::UniquePtr client_bio = + create_client_bio(server_addr.family(), SOCK_DGRAM); + ASSERT_TRUE(client_bio) << LastSocketError(); + + // "Connect" the client to server + BIO_ADDR bio_server_addr = server_addr.ToBIO_ADDR(); + ASSERT_EQ(1, BIO_dgram_set_peer(client_bio.get(), &bio_server_addr)) + << LastSocketError(); + + // Get peer + BIO_ADDR bio_server_addr_copy; + ASSERT_GT(BIO_dgram_get_peer(client_bio.get(), &bio_server_addr_copy), 0) + << LastSocketError(); + + ASSERT_EQ(bio_server_addr, bio_server_addr_copy) << LastSocketError(); + + test_send_receive(client_bio, server_bio); + test_send_receive(server_bio, client_bio); +} + +TEST_P(BIODgramTest, SocketDatagramSetConnected) { + int addr_family = GetParam(); + // Wrap the server socket in a BIO. + bssl::UniquePtr server_bio = create_server_bio(addr_family, SOCK_DGRAM); + ASSERT_TRUE(server_bio) << LastSocketError(); + + OwnedSocket server_sock(BIO_get_fd(server_bio.get(), NULL)); + ASSERT_EQ(1, BIO_set_close(server_bio.get(), BIO_NOCLOSE)) + << LastSocketError(); + SockaddrStorage server_addr = server_sock.storage(); + + // Get the server socket's address + bssl::UniquePtr client_bio = + create_client_bio(server_addr.family(), SOCK_DGRAM); + ASSERT_TRUE(client_bio) << LastSocketError(); + + int client_fd = 0; + int result = BIO_get_fd(client_bio.get(), &client_fd); + ASSERT_EQ(result, client_fd); + ASSERT_EQ(1, BIO_set_close(server_bio.get(), BIO_NOCLOSE)) + << LastSocketError(); + + // "Connect" the client to server + ASSERT_EQ(connect(client_fd, server_addr.addr(), server_addr.len), 0) + << LastSocketError(); + BIO_ADDR server_bio_addr = server_addr.ToBIO_ADDR(); + ASSERT_EQ(1, BIO_ctrl_set_connected(client_bio.get(), &server_bio_addr)) + << LastSocketError(); + + test_send_receive(client_bio, server_bio); + test_send_receive(server_bio, client_bio); + + ASSERT_EQ(1, BIO_ctrl_set_connected(client_bio.get(), NULL)) + << LastSocketError(); + + static constexpr char kTestMessage[] = "test"; + + // Behavior is different on Linux + int expected_result = +#if defined(OPENSSL_LINUX) + addr_family == AF_INET6 + ? (int)OPENSSL_strnlen((const char *)kTestMessage, 32) + 1 + : -1; +#else + -1; +#endif + + ASSERT_EQ(expected_result, + BIO_write(client_bio.get(), kTestMessage, sizeof(kTestMessage))) + << LastSocketError(); +} + +TEST_P(BIODgramTest, SocketDatagramConnect) { + int addr_family = GetParam(); + // Wrap the server socket in a BIO. + bssl::UniquePtr server_bio = create_server_bio(addr_family, SOCK_DGRAM); + ASSERT_TRUE(server_bio) << LastSocketError(); + + OwnedSocket server_sock(BIO_get_fd(server_bio.get(), NULL)); + ASSERT_EQ(1, BIO_set_close(server_bio.get(), BIO_NOCLOSE)) + << LastSocketError(); + SockaddrStorage server_addr = server_sock.storage(); + + // Get the server socket's address + bssl::UniquePtr client_bio = + create_client_bio(server_addr.family(), SOCK_DGRAM); + ASSERT_TRUE(client_bio) << LastSocketError(); + + // "Connect" the client to server + BIO_ADDR server_bio_addr = server_addr.ToBIO_ADDR(); + ASSERT_EQ(1, BIO_ctrl_dgram_connect(client_bio.get(), &server_bio_addr)) + << LastSocketError(); + ; + + test_send_receive(client_bio, server_bio); + test_send_receive(server_bio, client_bio); +} diff --git a/crypto/bio/dgram.c b/crypto/bio/dgram.c index fafc3680ea..1bd1ef3d6f 100644 --- a/crypto/bio/dgram.c +++ b/crypto/bio/dgram.c @@ -28,7 +28,7 @@ typedef SSIZE_T ssize_t; static int closesocket(const int sock) { return close(sock); } #endif -#if defined(AF_UNIX) && !defined(OPENSSL_WINDOWS) +#if defined(AF_UNIX) && !defined(OPENSSL_WINDOWS) && !defined(OPENSSL_ANDROID) // Winsock2 APIs don't support AF_UNIX. // > The values currently supported are AF_INET or AF_INET6, which are the // > Internet address family formats for IPv4 and IPv6. From b39903dc39028b36b1dcaf45029e074a5c5dbb3d Mon Sep 17 00:00:00 2001 From: Justin Smith Date: Fri, 11 Apr 2025 21:02:11 -0400 Subject: [PATCH 5/9] Some CodeBuild environments don't support IPv6 --- crypto/bio/bio_socket_test.cc | 20 +++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/crypto/bio/bio_socket_test.cc b/crypto/bio/bio_socket_test.cc index 045437a6ad..281f5510a0 100644 --- a/crypto/bio/bio_socket_test.cc +++ b/crypto/bio/bio_socket_test.cc @@ -463,7 +463,10 @@ static bssl::UniquePtr create_server_bio(int addr_family, int type) { // Create a server socket. OwnedSocket server_sock(ListenLoopback(type, addr_family)); if (!server_sock.is_valid()) { - ADD_FAILURE() << LastSocketError(); + if (addr_family != AF_INET6) { + // Some CodeBuild environments don't support IPv6 + ADD_FAILURE() << LastSocketError(); + } return nullptr; } @@ -549,6 +552,11 @@ TEST_P(BIODgramTest, SocketDatagramSetPeer) { int addr_family = GetParam(); // Wrap the server socket in a BIO. bssl::UniquePtr server_bio = create_server_bio(addr_family, SOCK_DGRAM); + if (!server_bio && addr_family == AF_INET6) { + // Some CodeBuild environments don't support IPv6 + GTEST_SKIP() << "IPv6 not supported"; + return; + } ASSERT_TRUE(server_bio) << LastSocketError(); ASSERT_EQ(1, BIO_get_close(server_bio.get())) << LastSocketError(); @@ -582,6 +590,11 @@ TEST_P(BIODgramTest, SocketDatagramSetConnected) { int addr_family = GetParam(); // Wrap the server socket in a BIO. bssl::UniquePtr server_bio = create_server_bio(addr_family, SOCK_DGRAM); + if (!server_bio && addr_family == AF_INET6) { + // Some CodeBuild environments don't support IPv6 + GTEST_SKIP() << "IPv6 not supported"; + return; + } ASSERT_TRUE(server_bio) << LastSocketError(); OwnedSocket server_sock(BIO_get_fd(server_bio.get(), NULL)); @@ -634,6 +647,11 @@ TEST_P(BIODgramTest, SocketDatagramConnect) { int addr_family = GetParam(); // Wrap the server socket in a BIO. bssl::UniquePtr server_bio = create_server_bio(addr_family, SOCK_DGRAM); + if (!server_bio && addr_family == AF_INET6) { + // Some CodeBuild environments don't support IPv6 + GTEST_SKIP() << "IPv6 not supported"; + return; + } ASSERT_TRUE(server_bio) << LastSocketError(); OwnedSocket server_sock(BIO_get_fd(server_bio.get(), NULL)); From 3c82a6f745e780d7d1f4b062588a51c235e090b5 Mon Sep 17 00:00:00 2001 From: Justin Smith Date: Wed, 30 Apr 2025 07:57:52 -0400 Subject: [PATCH 6/9] Improve test coverage --- crypto/bio/bio_socket_test.cc | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/crypto/bio/bio_socket_test.cc b/crypto/bio/bio_socket_test.cc index 281f5510a0..ba9802159a 100644 --- a/crypto/bio/bio_socket_test.cc +++ b/crypto/bio/bio_socket_test.cc @@ -526,6 +526,28 @@ static void test_send_receive(bssl::UniquePtr &sender_bio, ASSERT_EQ(Bytes(buff), Bytes(kTestMessage)); } +static void test_puts_receive(bssl::UniquePtr &sender_bio, + bssl::UniquePtr &receiver_bio) { + static constexpr char kTestMessage[] = "test"; + + // Send a message + ASSERT_EQ((int)strlen(kTestMessage), + BIO_puts(sender_bio.get(), kTestMessage)) + << LastSocketError(); + // BIO_flush is a no-op, but test it anyway. + ASSERT_EQ(1, BIO_flush(sender_bio.get())) << LastSocketError(); + + // Receive a message. + char buff[1024]; + // `BIO_puts` does not send the \0 byte at the end of the string. + ASSERT_EQ((int)strlen(kTestMessage), + BIO_read(receiver_bio.get(), buff, sizeof(buff))) + << LastSocketError(); + + // Verify the message received matches the message sent. + ASSERT_EQ(Bytes(buff), Bytes(kTestMessage)); +} + class BIODgramTest : public testing::TestWithParam { // You can implement all the usual fixture class members here. // To access the test parameter, call GetParam() from class @@ -583,7 +605,10 @@ TEST_P(BIODgramTest, SocketDatagramSetPeer) { ASSERT_EQ(bio_server_addr, bio_server_addr_copy) << LastSocketError(); test_send_receive(client_bio, server_bio); + ASSERT_EQ(0, BIO_dgram_send_timedout(client_bio.get())); + ASSERT_EQ(0, BIO_dgram_recv_timedout(server_bio.get())); test_send_receive(server_bio, client_bio); + test_puts_receive(client_bio, server_bio); } TEST_P(BIODgramTest, SocketDatagramSetConnected) { From a5409a55a85a2a62081b502b88b055efd56a7bf6 Mon Sep 17 00:00:00 2001 From: Justin Smith Date: Wed, 30 Apr 2025 08:18:06 -0400 Subject: [PATCH 7/9] Use OPENSSL_cleanse where appropriate --- crypto/bio/bio_mem.c | 2 +- crypto/bio/bio_socket_test.cc | 2 ++ crypto/bio/socket_helper.c | 4 ++-- 3 files changed, 5 insertions(+), 3 deletions(-) diff --git a/crypto/bio/bio_mem.c b/crypto/bio/bio_mem.c index 54243f9df4..4425209cad 100644 --- a/crypto/bio/bio_mem.c +++ b/crypto/bio/bio_mem.c @@ -217,7 +217,7 @@ static long mem_ctrl(BIO *bio, int cmd, long num, void *ptr) { b->data -= b->max - b->length; b->length = b->max; } else { - OPENSSL_memset(b->data, 0, b->max); + OPENSSL_cleanse(b->data, b->max); b->length = 0; } } diff --git a/crypto/bio/bio_socket_test.cc b/crypto/bio/bio_socket_test.cc index ba9802159a..6150ba3d8a 100644 --- a/crypto/bio/bio_socket_test.cc +++ b/crypto/bio/bio_socket_test.cc @@ -518,6 +518,7 @@ static void test_send_receive(bssl::UniquePtr &sender_bio, // Receive a message char buff[1024]; + OPENSSL_cleanse(buff, sizeof(buff)); ASSERT_EQ((int)strlen(kTestMessage) + 1, BIO_read(receiver_bio.get(), buff, sizeof(buff))) << LastSocketError(); @@ -539,6 +540,7 @@ static void test_puts_receive(bssl::UniquePtr &sender_bio, // Receive a message. char buff[1024]; + OPENSSL_cleanse(buff, sizeof(buff)); // `BIO_puts` does not send the \0 byte at the end of the string. ASSERT_EQ((int)strlen(kTestMessage), BIO_read(receiver_bio.get(), buff, sizeof(buff))) diff --git a/crypto/bio/socket_helper.c b/crypto/bio/socket_helper.c index 217d798dba..e30b8c451e 100644 --- a/crypto/bio/socket_helper.c +++ b/crypto/bio/socket_helper.c @@ -51,7 +51,7 @@ int bio_ip_and_port_to_socket_and_addr(int *out_sock, *out_sock = -1; - OPENSSL_memset(&hint, 0, sizeof(hint)); + OPENSSL_cleanse(&hint,sizeof(hint)); hint.ai_family = AF_UNSPEC; hint.ai_socktype = SOCK_STREAM; @@ -72,7 +72,7 @@ int bio_ip_and_port_to_socket_and_addr(int *out_sock, if ((size_t) cur->ai_addrlen > sizeof(struct sockaddr_storage)) { continue; } - OPENSSL_memset(out_addr, 0, sizeof(struct sockaddr_storage)); + OPENSSL_cleanse(out_addr, sizeof(struct sockaddr_storage)); OPENSSL_memcpy(out_addr, cur->ai_addr, cur->ai_addrlen); *out_addr_length = cur->ai_addrlen; From 1d8b2a4454cd5334c8873189d4c089468f41e2b1 Mon Sep 17 00:00:00 2001 From: Justin Smith Date: Wed, 30 Apr 2025 11:55:02 -0400 Subject: [PATCH 8/9] Fix aws-lc-sys bindgen for Windows --- include/openssl/bio.h | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/include/openssl/bio.h b/include/openssl/bio.h index 60a1d81aff..b130418913 100644 --- a/include/openssl/bio.h +++ b/include/openssl/bio.h @@ -68,12 +68,11 @@ #include #if defined(OPENSSL_WINDOWS) -#include +// Due to name conflicts, we must prevent "wincrypt.h" from being included +#define NOCRYPT #include #include -#if !defined(__MINGW32__) -#include -#endif +#undef NOCRYPT #else #include #include From bbe156b363c5e1112b3bcd5cb7fdd420bd3db9b4 Mon Sep 17 00:00:00 2001 From: Justin Smith Date: Wed, 7 May 2025 11:37:13 -0400 Subject: [PATCH 9/9] PR feedback --- crypto/bio/dgram.c | 4 +++- crypto/bio/internal.h | 2 +- include/openssl/bio.h | 10 ++++++---- 3 files changed, 10 insertions(+), 6 deletions(-) diff --git a/crypto/bio/dgram.c b/crypto/bio/dgram.c index 1bd1ef3d6f..971b6363b6 100644 --- a/crypto/bio/dgram.c +++ b/crypto/bio/dgram.c @@ -161,7 +161,8 @@ static int dgram_read(BIO *bp, char *out, const int out_len) { BIO_ADDR_sockaddr_noconst(&peer), &len); if (result < INT_MIN || result > INT_MAX) { - abort(); + OPENSSL_PUT_ERROR(BIO, BIO_R_SYS_LIB); + return -1; } const int ret = result; @@ -194,6 +195,7 @@ static int dgram_free(BIO *bp) { } } bp->init = 0; + bp->num = -1; bp->flags = 0; OPENSSL_free(bp->ptr); bp->ptr = NULL; diff --git a/crypto/bio/internal.h b/crypto/bio/internal.h index a3f7d95b18..dc2ac03e39 100644 --- a/crypto/bio/internal.h +++ b/crypto/bio/internal.h @@ -98,7 +98,7 @@ int bio_socket_nbio(int sock, int on); // bio_clear_socket_error clears the last socket error on |sock|. void bio_clear_socket_error(int sock); -// bio_sock_error returns the last socket error on |sock|. +// bio_sock_error_get_and_clear clears and returns the last socket error on |sock|. int bio_sock_error_get_and_clear(int sock); // bio_socket_should_retry returns non-zero if |return_value| indicates an error diff --git a/include/openssl/bio.h b/include/openssl/bio.h index b130418913..91444f29b4 100644 --- a/include/openssl/bio.h +++ b/include/openssl/bio.h @@ -723,7 +723,7 @@ typedef union bio_addr_st { #endif } BIO_ADDR; -#define BIO_CTRL_DGRAM_CONNECT 31// BIO dgram special +#define BIO_CTRL_DGRAM_CONNECT 31 // BIO dgram special #define BIO_CTRL_DGRAM_SET_CONNECTED 32 /* allow for an externally connected * socket to be passed in */ @@ -765,12 +765,14 @@ OPENSSL_EXPORT int BIO_ctrl_dgram_connect(BIO *bp, const BIO_ADDR *peer); OPENSSL_EXPORT int BIO_ctrl_set_connected(BIO* bp, const BIO_ADDR *peer); // BIO_dgram_recv_timedout returns 1 if the most recent datagram receive -// operation on |bp| timed out, and a non-positive value otherwise. +// operation on |bp| timed out, and a non-positive value otherwise. Any error +// for this socket gets reset by this call. OPENSSL_EXPORT int BIO_dgram_recv_timedout(BIO* bp); // BIO_dgram_send_timedout returns 1 if the most recent datagram send -// operation on |bp| timed out, and a non-positive value otherwise. -OPENSSL_EXPORT int BIO_dgram_send_timedout(BIO* bp); +// operation on |bp| timed out, and a non-positive value otherwise. Any error +// for this socket gets reset by this call. +OPENSSL_EXPORT int BIO_dgram_send_timedout(BIO *bp); // BIO_dgram_get_peer stores the address of the peer the datagram BIO is // connected to in |peer|. It returns 1 on success and a non-positive value on error.