openssl
/
openssl
mirror of https://github.com/openssl/openssl
1
0
Fork 0
Code Issues Releases Wiki Activity

OpenSSL 3.2.0, QUIC, macOS, error 56 on connected UDP socket 

current `translate_msg()` function attempts to set `->msg_name`
(and `->msg_namelen`) with `BIO`'s peer name (connection destination)
regardless if underlying socket is connected or not. Such implementation
uncovers differences in socket implementation between various OSes.

As we have learned hard way `sendmsg()` and `sendmmsg()` on `OpenBSD`
and (`MacOS` too) fail to send messages with `->msg_name` being
set on connected socket. In such case the caller receives
`EISCON` errro.

I think `translate_msg()` caller should provide a hint to indicate
whether we deal with connected (or un-connected) socket. For
connected sockets the peer's name should not be set/filled
by `translate_msg()`. On the other hand if socket is un-connected,
then `translate_msg()` must populate `->msg_name` and `->msg_namelen`
members.

The caller can use `getpeername(2)` to see if socket is
connected. If `getpeername()` succeeds then we must be dealing
with connected socket and `translate_msg()` must not set
`->msg_name` and `->msg_namelen` members. If `getpeername(2)`
fails, then `translate_msg()` must provide peer's name (destination
address) in `->msg_name` and set `->msg_namelen` accordingly.

The propposed fix introduces `is_connected()` function,
which applies `getpeername()` to socket bound to `BIO` instance.
The `dgram_sendmmsg()` uses `is_connected()` as a hint
for `translate_msg()` function, so msghdr gets initialized
with respect to socket state.

The change also modifies existing `test/quic_client_test.c`
so it also covers the case of connected socket. To keep
things simple we can introduce optional argument `connect_first`
to `./quic_client_test` function. Without `connect_first`
the test run as usual. With `connect_first` the test creates
and connects socket first. Then it passes such socket to
`BIO` sub-system to perform `QUIC` connect test as usual.

Fixes #23251

Reviewed-by: Neil Horman <nhorman@openssl.org>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/23396)
This commit is contained in:
Alexandr Nedvedicky 2024-01-26 08:05:47 +01:00 committed by Tomas Mraz
parent 4ffef97d37
commit c062403abd
2 changed files with 88 additions and 24 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

28
crypto/bio/bss_dgram.c
View File

@ -560,6 +560,8 @@ static long dgram_ctrl(BIO *b, int cmd, long num, void *ptr)
socklen_t addr_len;
BIO_ADDR addr;
# endif
struct sockaddr_storage ss;
socklen_t ss_len = sizeof(ss);
data = (bio_dgram_data *)b->ptr;
@ -577,6 +579,10 @@ static long dgram_ctrl(BIO *b, int cmd, long num, void *ptr)
b->shutdown = (int)num;
b->init = 1;
dgram_update_local_addr(b);
if (getpeername(b->num, (struct sockaddr *)&ss, &ss_len) == 0) {
BIO_ADDR_make(&data->peer, BIO_ADDR_sockaddr((BIO_ADDR *)&ss));
data->connected = 1;
}
# if defined(SUPPORT_LOCAL_ADDR)
if (data->local_addr_enabled) {
if (enable_local_addr(b, 1) < 1)
@ -1067,19 +1073,27 @@ static void translate_msg_win(BIO *b, WSAMSG *mh, WSABUF *iov,
static void translate_msg(BIO *b, struct msghdr *mh, struct iovec *iov,
unsigned char *control, BIO_MSG *msg)
{
bio_dgram_data *data;
iov->iov_base = msg->data;
iov->iov_len = msg->data_len;
/* macOS requires msg_namelen be 0 if msg_name is NULL */
mh->msg_name = msg->peer != NULL ? &msg->peer->sa : NULL;
if (msg->peer != NULL && dgram_get_sock_family(b) == AF_INET)
mh->msg_namelen = sizeof(struct sockaddr_in);
data = (bio_dgram_data *)b->ptr;
if (data->connected == 0) {
/* macOS requires msg_namelen be 0 if msg_name is NULL */
mh->msg_name = msg->peer != NULL ? &msg->peer->sa : NULL;
if (msg->peer != NULL && dgram_get_sock_family(b) == AF_INET)
mh->msg_namelen = sizeof(struct sockaddr_in);
# if OPENSSL_USE_IPV6
else if (msg->peer != NULL && dgram_get_sock_family(b) == AF_INET6)
mh->msg_namelen = sizeof(struct sockaddr_in6);
else if (msg->peer != NULL && dgram_get_sock_family(b) == AF_INET6)
mh->msg_namelen = sizeof(struct sockaddr_in6);
# endif
else
else
mh->msg_namelen = 0;
} else {
mh->msg_name = NULL;
mh->msg_namelen = 0;
}
mh->msg_iov = iov;
mh->msg_iovlen = 1;

84
test/quic_client_test.c
View File

@ -18,6 +18,9 @@
static const char msg1[] = "GET LICENSE.txt\r\n";
static char msg2[16000];
#define DST_PORT 4433
#define DST_ADDR 0x7f000001UL
static int is_want(SSL *s, int ret)
{
int ec = SSL_get_error(s, ret);
@ -25,42 +28,47 @@ static int is_want(SSL *s, int ret)
return ec == SSL_ERROR_WANT_READ || ec == SSL_ERROR_WANT_WRITE;
}
static int test_quic_client(void)
static int test_quic_client_ex(int fd_arg)
{
int testresult = 0, ret;
int c_fd = INVALID_SOCKET;
int c_fd;
BIO *c_net_bio = NULL, *c_net_bio_own = NULL;
BIO_ADDR *s_addr_ = NULL;
struct in_addr ina = {0};
SSL_CTX *c_ctx = NULL;
SSL *c_ssl = NULL;
short port = 4433;
short port = DST_PORT;
int c_connected = 0, c_write_done = 0, c_shutdown = 0;
size_t l = 0, c_total_read = 0;
OSSL_TIME start_time;
unsigned char alpn[] = { 8, 'h', 't', 't', 'p', '/', '0', '.', '9' };
ina.s_addr = htonl(0x7f000001UL);
/* Setup test client. */
c_fd = BIO_socket(AF_INET, SOCK_DGRAM, IPPROTO_UDP, 0);
if (!TEST_int_ne(c_fd, INVALID_SOCKET))
goto err;
if (fd_arg == INVALID_SOCKET) {
/* Setup test client. */
c_fd = BIO_socket(AF_INET, SOCK_DGRAM, IPPROTO_UDP, 0);
if (!TEST_int_ne(c_fd, INVALID_SOCKET))
goto err;
if (!TEST_true(BIO_socket_nbio(c_fd, 1)))
goto err;
if (!TEST_true(BIO_socket_nbio(c_fd, 1)))
goto err;
if (!TEST_ptr(s_addr_ = BIO_ADDR_new()))
goto err;
if (!TEST_ptr(s_addr_ = BIO_ADDR_new()))
goto err;
if (!TEST_true(BIO_ADDR_rawmake(s_addr_, AF_INET, &ina, sizeof(ina),
htons(port))))
goto err;
ina.s_addr = htonl(DST_ADDR);
if (!TEST_true(BIO_ADDR_rawmake(s_addr_, AF_INET, &ina, sizeof(ina),
htons(port))))
goto err;
} else {
c_fd = fd_arg;
}
if (!TEST_ptr(c_net_bio = c_net_bio_own = BIO_new_dgram(c_fd, 0)))
goto err;
if (!BIO_dgram_set_peer(c_net_bio, s_addr_))
/* connected socket does not need to set peer */
if (s_addr_ != NULL && !BIO_dgram_set_peer(c_net_bio, s_addr_))
goto err;
if (!TEST_ptr(c_ctx = SSL_CTX_new(OSSL_QUIC_client_method())))
@ -157,11 +165,51 @@ err:
SSL_CTX_free(c_ctx);
BIO_ADDR_free(s_addr_);
BIO_free(c_net_bio_own);
if (c_fd != INVALID_SOCKET)
if (fd_arg == INVALID_SOCKET && c_fd != INVALID_SOCKET)
BIO_closesocket(c_fd);
return testresult;
}
static int test_quic_client(void)
{
return (test_quic_client_ex(INVALID_SOCKET));
}
static int test_quic_client_connect_first(void)
{
struct sockaddr_in sin = {0};
int c_fd;
int rv;
#ifdef SA_LEN
sin.sin_len = sizeof(struct sockaddr_in);
#endif
sin.sin_family = AF_INET;
sin.sin_port = htons(DST_PORT);
sin.sin_addr.s_addr = htonl(DST_ADDR);
c_fd = socket(AF_INET, SOCK_DGRAM, IPPROTO_UDP);
if (!TEST_int_ne(c_fd, INVALID_SOCKET))
goto err;
if (!TEST_int_eq(connect(c_fd, (const struct sockaddr *)&sin, sizeof(sin)), 0))
goto err;
if (!TEST_true(BIO_socket_nbio(c_fd, 1)))
goto err;
rv = test_quic_client_ex(c_fd);
close(c_fd);
return (rv);
err:
if (c_fd != INVALID_SOCKET)
close(c_fd);
return (0);
}
OPT_TEST_DECLARE_USAGE("certfile privkeyfile\n")
int setup_tests(void)
@ -172,5 +220,7 @@ int setup_tests(void)
}
ADD_TEST(test_quic_client);
ADD_TEST(test_quic_client_connect_first);
return 1;
}