On Wed, Nov 29, 2017 at 07:50:21PM +0100, Claudio Jeker wrote:
> More or less. It seems that msg_local is garbage:
> $3 = {msg_data = 0x1b4e541fa4c0, msg_offset = 0, msg_local = {
> ss_len = 128 '\200', ss_family = 192 'À',
> __ss_pad1 = 0x7f7ffffda8c2 "Ì]\001", __ss_pad2 = 30024042514159,
> __ss_pad3 = 0x7f7ffffda8d0 "Ьýÿ\177\177"}, msg_locallen = 128,
> msg_peer = {ss_len = 16 '\020', ss_family = 2 '\002',
> __ss_pad1 = 0x7f7ffffda9ca "\001ô>0\036\005", __ss_pad2 = 0,
> __ss_pad3 = 0x7f7ffffda9d8 ""}, msg_peerlen = 16,
>
> Why so I don't know.
I found it. socket_getaddr is called to retrieve information about the
socket behind the file descriptor. If I understand correctly, we need
to pass the length of the struct to getsockname() so that it knows how
far it can fill the struct. So far we always passed stack garbage, so
sometimes it worked, sometimes not. Then getsockname() does not change
the struct, so sockaddr_storage ss is still stack garbarge, and that
ends up in msg_local.
I think recvfromto() suffers the same. The caller ikev2_msg_cb() does
pass the correct size to recvfromto(), but then recvfromto() uncondi-
tionally overwrites it with zero before calling getsockname().
Opinions?
Patrick
diff --git a/sbin/iked/iked.h b/sbin/iked/iked.h
index 9deca37f2d0..a2e4a16fd89 100644
--- a/sbin/iked/iked.h
+++ b/sbin/iked/iked.h
@@ -939,7 +939,7 @@ int socket_af(struct sockaddr *, in_port_t);
in_port_t
socket_getport(struct sockaddr *);
int socket_setport(struct sockaddr *, in_port_t);
-int socket_getaddr(int, struct sockaddr_storage *);
+int socket_getaddr(int, struct sockaddr_storage *, socklen_t);
int socket_bypass(int, struct sockaddr *);
int udp_bind(struct sockaddr *, in_port_t);
ssize_t sendtofrom(int, void *, size_t, int, struct sockaddr *,
diff --git a/sbin/iked/ikev2.c b/sbin/iked/ikev2.c
index 411c6751c37..fcace627136 100644
--- a/sbin/iked/ikev2.c
+++ b/sbin/iked/ikev2.c
@@ -944,7 +944,7 @@ ikev2_init_ike_sa_peer(struct iked *env, struct iked_policy
*pol,
goto closeonly;
if (pol->pol_local.addr.ss_family == AF_UNSPEC) {
- if (socket_getaddr(sock->sock_fd, &ss) == -1)
+ if (socket_getaddr(sock->sock_fd, &ss, sizeof(ss)) == -1)
goto closeonly;
} else
memcpy(&ss, &pol->pol_local.addr, pol->pol_local.addr.ss_len);
diff --git a/sbin/iked/util.c b/sbin/iked/util.c
index 1d4cee2c424..babbce8ac9e 100644
--- a/sbin/iked/util.c
+++ b/sbin/iked/util.c
@@ -91,10 +91,8 @@ socket_setport(struct sockaddr *sa, in_port_t port)
}
int
-socket_getaddr(int s, struct sockaddr_storage *ss)
+socket_getaddr(int s, struct sockaddr_storage *ss, socklen_t sslen)
{
- socklen_t sslen;
-
return (getsockname(s, (struct sockaddr *)ss, &sslen));
}
@@ -366,7 +364,6 @@ recvfromto(int s, void *buf, size_t len, int flags, struct
sockaddr *from,
return (-1);
*fromlen = from->sa_len;
- *tolen = 0;
if (getsockname(s, to, tolen) != 0)
*tolen = 0;
