Re: iked, don't return NULL in print_host

2017-12-12 Thread Jeremie Courreges-Anglas
On Tue, Dec 12 2017, Patrick Wildt  wrote:
> On Tue, Dec 12, 2017 at 03:32:50PM +0100, Patrick Wildt wrote:
>> 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 = 0x7f7da8c2 "Ì]\001", __ss_pad2 = 30024042514159, 
>> > __ss_pad3 = 0x7f7da8d0 "Ьýÿ\177\177"}, msg_locallen = 128, 
>> >   msg_peer = {ss_len = 16 '\020', ss_family = 2 '\002', 
>> > __ss_pad1 = 0x7f7da9ca "\001ô>0\036\005", __ss_pad2 = 0, 
>> > __ss_pad3 = 0x7f7da9d8 ""}, 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().

... and then cmsg handling should update *tolen.

>> Opinions?

The getsockname(2) doesn't seem to perform anything useful: the ikev2
process gets a fully initialized sockaddr (len, family, address,
port) along with the socket, and recvfromto() could just care about
filling the address using cmsg parsing.

>> Patrick
>
> Markus points out that socket_getaddr() is promised a pointer to a
> sockaddr_storage, so we don't need to pass the length and can just
> initialize sslen correctly.

ok jca@ for socket_getaddr(), also ok for recvfromto() since the code is
confusing as is, but if I'm not mistaken we could just remove the
getsockname(2) call.

> diff --git a/sbin/iked/util.c b/sbin/iked/util.c
> index 1d4cee2c424..e67dae27d8a 100644
> --- a/sbin/iked/util.c
> +++ b/sbin/iked/util.c
> @@ -93,7 +93,7 @@ socket_setport(struct sockaddr *sa, in_port_t port)
>  int
>  socket_getaddr(int s, struct sockaddr_storage *ss)
>  {
> - socklen_t sslen;
> + socklen_t sslen = sizeof(*ss);
>  
>   return (getsockname(s, (struct sockaddr *)ss, &sslen));
>  }
> @@ -366,7 +366,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;
>

-- 
jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF  DDCC 0DFA 74AE 1524 E7EE



Re: iked, don't return NULL in print_host

2017-12-12 Thread Patrick Wildt
On Tue, Dec 12, 2017 at 03:32:50PM +0100, Patrick Wildt wrote:
> 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 = 0x7f7da8c2 "Ì]\001", __ss_pad2 = 30024042514159, 
> > __ss_pad3 = 0x7f7da8d0 "Ьýÿ\177\177"}, msg_locallen = 128, 
> >   msg_peer = {ss_len = 16 '\020', ss_family = 2 '\002', 
> > __ss_pad1 = 0x7f7da9ca "\001ô>0\036\005", __ss_pad2 = 0, 
> > __ss_pad3 = 0x7f7da9d8 ""}, 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

Markus points out that socket_getaddr() is promised a pointer to a
sockaddr_storage, so we don't need to pass the length and can just
initialize sslen correctly.

diff --git a/sbin/iked/util.c b/sbin/iked/util.c
index 1d4cee2c424..e67dae27d8a 100644
--- a/sbin/iked/util.c
+++ b/sbin/iked/util.c
@@ -93,7 +93,7 @@ socket_setport(struct sockaddr *sa, in_port_t port)
 int
 socket_getaddr(int s, struct sockaddr_storage *ss)
 {
-   socklen_t sslen;
+   socklen_t sslen = sizeof(*ss);
 
return (getsockname(s, (struct sockaddr *)ss, &sslen));
 }
@@ -366,7 +366,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;



Re: iked, don't return NULL in print_host

2017-12-12 Thread Patrick Wildt
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 = 0x7f7da8c2 "Ì]\001", __ss_pad2 = 30024042514159, 
> __ss_pad3 = 0x7f7da8d0 "Ьýÿ\177\177"}, msg_locallen = 128, 
>   msg_peer = {ss_len = 16 '\020', ss_family = 2 '\002', 
> __ss_pad1 = 0x7f7da9ca "\001ô>0\036\005", __ss_pad2 = 0, 
> __ss_pad3 = 0x7f7da9d8 ""}, 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;



Re: iked, don't return NULL in print_host

2017-11-29 Thread Claudio Jeker
On Wed, Nov 29, 2017 at 05:32:11PM +0100, Patrick Wildt wrote:
> On Wed, Nov 29, 2017 at 04:32:24PM +0100, Claudio Jeker wrote:
> > On Wed, Nov 29, 2017 at 02:43:45AM +0100, Jeremie Courreges-Anglas wrote:
> > > On Wed, Nov 29 2017, Claudio Jeker  wrote:
> > > > On Wed, Nov 29, 2017 at 01:59:06AM +0100, Claudio Jeker wrote:
> > > >> Seen in my log file:
> > > >> Nov 28 17:47:22 dramaqueen iked: vfprintf %s NULL in "%s: %s %s from 
> > > >> %s to
> > > >> %s ms gid %u, %ld bytes%s"
> > > >> 
> > > >> and
> > > >> 
> > > >> Nov 29 01:02:39 dramaqueen iked[49967]: ikev2_msg_send: IKE_SA_INIT
> > > >> request from (null) to 62.48.30.5:500 msgid 0, 438 bytes
> > > >> 
> > > >> The problem seems to be in print_host so try to not return NULL in 
> > > >> there.
> > > >> Maybe we could return something else but this is a start IMO.
> > > >
> > > > beck@ prefers to just print unknown instead  of the gai_strerror -- 
> > > > guess
> > > > that is more sensible.
> > > 
> > > Why would getnameinfo(NI_NUMERICHOST) fail here?  The code in
> > > ikev2_msg.c is:
> > > 
> > > --8<--
> > >   log_info("%s: %s %s from %s to %s msgid %u, %ld bytes%s", __func__,
> > >   print_map(exchange, ikev2_exchange_map),
> > >   (flags & IKEV2_FLAG_RESPONSE) ? "response" : "request",
> > >   print_host((struct sockaddr *)&msg->msg_local, NULL, 0),
> > >   print_host((struct sockaddr *)&msg->msg_peer, NULL, 0),
> > >   betoh32(hdr->ike_msgid),
> > >   ibuf_length(buf), isnatt ? ", NAT-T" : "");
> > > -->8--
> > > 
> > > Maybe msg->msg_local is corrupt?
> > > 
> > 
> > I assume so much. gai_strerror returns "system error" not very helpful.
> 
> I would like to find out where that comes from.  What's your setup?  I
> guess you can reproduce it easily?

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 = 0x7f7da8c2 "Ì]\001", __ss_pad2 = 30024042514159, 
__ss_pad3 = 0x7f7da8d0 "Ьýÿ\177\177"}, msg_locallen = 128, 
  msg_peer = {ss_len = 16 '\020', ss_family = 2 '\002', 
__ss_pad1 = 0x7f7da9ca "\001ô>0\036\005", __ss_pad2 = 0, 
__ss_pad3 = 0x7f7da9d8 ""}, msg_peerlen = 16, 

Why so I don't know.

Config is:
ikev2 "tunnel" active esp proto ipencap from 10.83.66.133/32 to 10.83.66.6/32 \
peer 62.48.30.5 \
ikesa group modp2048 \
srcid dramaqueen.zyd.ch rsa
ikev2 "tunnel" active esp proto ipv6 from 10.83.66.133/32 to 10.83.66.6/32 \
peer 62.48.30.5 \
ikesa group modp2048 \
srcid dramaqueen6.zyd.ch

The other end does not listen to iked so the connections never succeed. 
Maybe this helps.

-- 
:wq Claudio



Re: iked, don't return NULL in print_host

2017-11-29 Thread Jeremie Courreges-Anglas
On Wed, Nov 29 2017, Patrick Wildt  wrote:
> On Wed, Nov 29, 2017 at 04:32:24PM +0100, Claudio Jeker wrote:
>> On Wed, Nov 29, 2017 at 02:43:45AM +0100, Jeremie Courreges-Anglas wrote:
>> > On Wed, Nov 29 2017, Claudio Jeker  wrote:
>> > > On Wed, Nov 29, 2017 at 01:59:06AM +0100, Claudio Jeker wrote:
>> > >> Seen in my log file:
>> > >> Nov 28 17:47:22 dramaqueen iked: vfprintf %s NULL in "%s: %s %s from %s 
>> > >> to
>> > >> %s ms gid %u, %ld bytes%s"
>> > >> 
>> > >> and
>> > >> 
>> > >> Nov 29 01:02:39 dramaqueen iked[49967]: ikev2_msg_send: IKE_SA_INIT
>> > >> request from (null) to 62.48.30.5:500 msgid 0, 438 bytes
>> > >> 
>> > >> The problem seems to be in print_host so try to not return NULL in 
>> > >> there.
>> > >> Maybe we could return something else but this is a start IMO.
>> > >
>> > > beck@ prefers to just print unknown instead  of the gai_strerror -- guess
>> > > that is more sensible.
>> > 
>> > Why would getnameinfo(NI_NUMERICHOST) fail here?  The code in
>> > ikev2_msg.c is:
>> > 
>> > --8<--
>> >log_info("%s: %s %s from %s to %s msgid %u, %ld bytes%s", __func__,
>> >print_map(exchange, ikev2_exchange_map),
>> >(flags & IKEV2_FLAG_RESPONSE) ? "response" : "request",
>> >print_host((struct sockaddr *)&msg->msg_local, NULL, 0),
>> >print_host((struct sockaddr *)&msg->msg_peer, NULL, 0),
>> >betoh32(hdr->ike_msgid),
>> >ibuf_length(buf), isnatt ? ", NAT-T" : "");
>> > -->8--
>> > 
>> > Maybe msg->msg_local is corrupt?
>> > 
>> 
>> I assume so much. gai_strerror returns "system error" not very helpful.
>
> I would like to find out where that comes from.  What's your setup?  I
> guess you can reproduce it easily?

btw, "system error" may not be helpful, but afaik if you get EAI_SYSTEM
you're supposed to look at errno.  (asr has a mechanism to save and
restore the proper errno).

-- 
jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF  DDCC 0DFA 74AE 1524 E7EE



Re: iked, don't return NULL in print_host

2017-11-29 Thread Patrick Wildt
On Wed, Nov 29, 2017 at 04:32:24PM +0100, Claudio Jeker wrote:
> On Wed, Nov 29, 2017 at 02:43:45AM +0100, Jeremie Courreges-Anglas wrote:
> > On Wed, Nov 29 2017, Claudio Jeker  wrote:
> > > On Wed, Nov 29, 2017 at 01:59:06AM +0100, Claudio Jeker wrote:
> > >> Seen in my log file:
> > >> Nov 28 17:47:22 dramaqueen iked: vfprintf %s NULL in "%s: %s %s from %s 
> > >> to
> > >> %s ms gid %u, %ld bytes%s"
> > >> 
> > >> and
> > >> 
> > >> Nov 29 01:02:39 dramaqueen iked[49967]: ikev2_msg_send: IKE_SA_INIT
> > >> request from (null) to 62.48.30.5:500 msgid 0, 438 bytes
> > >> 
> > >> The problem seems to be in print_host so try to not return NULL in there.
> > >> Maybe we could return something else but this is a start IMO.
> > >
> > > beck@ prefers to just print unknown instead  of the gai_strerror -- guess
> > > that is more sensible.
> > 
> > Why would getnameinfo(NI_NUMERICHOST) fail here?  The code in
> > ikev2_msg.c is:
> > 
> > --8<--
> > log_info("%s: %s %s from %s to %s msgid %u, %ld bytes%s", __func__,
> > print_map(exchange, ikev2_exchange_map),
> > (flags & IKEV2_FLAG_RESPONSE) ? "response" : "request",
> > print_host((struct sockaddr *)&msg->msg_local, NULL, 0),
> > print_host((struct sockaddr *)&msg->msg_peer, NULL, 0),
> > betoh32(hdr->ike_msgid),
> > ibuf_length(buf), isnatt ? ", NAT-T" : "");
> > -->8--
> > 
> > Maybe msg->msg_local is corrupt?
> > 
> 
> I assume so much. gai_strerror returns "system error" not very helpful.

I would like to find out where that comes from.  What's your setup?  I
guess you can reproduce it easily?



Re: iked, don't return NULL in print_host

2017-11-29 Thread Claudio Jeker
On Wed, Nov 29, 2017 at 02:43:45AM +0100, Jeremie Courreges-Anglas wrote:
> On Wed, Nov 29 2017, Claudio Jeker  wrote:
> > On Wed, Nov 29, 2017 at 01:59:06AM +0100, Claudio Jeker wrote:
> >> Seen in my log file:
> >> Nov 28 17:47:22 dramaqueen iked: vfprintf %s NULL in "%s: %s %s from %s to
> >> %s ms gid %u, %ld bytes%s"
> >> 
> >> and
> >> 
> >> Nov 29 01:02:39 dramaqueen iked[49967]: ikev2_msg_send: IKE_SA_INIT
> >> request from (null) to 62.48.30.5:500 msgid 0, 438 bytes
> >> 
> >> The problem seems to be in print_host so try to not return NULL in there.
> >> Maybe we could return something else but this is a start IMO.
> >
> > beck@ prefers to just print unknown instead  of the gai_strerror -- guess
> > that is more sensible.
> 
> Why would getnameinfo(NI_NUMERICHOST) fail here?  The code in
> ikev2_msg.c is:
> 
> --8<--
>   log_info("%s: %s %s from %s to %s msgid %u, %ld bytes%s", __func__,
>   print_map(exchange, ikev2_exchange_map),
>   (flags & IKEV2_FLAG_RESPONSE) ? "response" : "request",
>   print_host((struct sockaddr *)&msg->msg_local, NULL, 0),
>   print_host((struct sockaddr *)&msg->msg_peer, NULL, 0),
>   betoh32(hdr->ike_msgid),
>   ibuf_length(buf), isnatt ? ", NAT-T" : "");
> -->8--
> 
> Maybe msg->msg_local is corrupt?
> 

I assume so much. gai_strerror returns "system error" not very helpful.

-- 
:wq Claudio



Re: iked, don't return NULL in print_host

2017-11-28 Thread Jeremie Courreges-Anglas
On Wed, Nov 29 2017, Claudio Jeker  wrote:
> On Wed, Nov 29, 2017 at 01:59:06AM +0100, Claudio Jeker wrote:
>> Seen in my log file:
>> Nov 28 17:47:22 dramaqueen iked: vfprintf %s NULL in "%s: %s %s from %s to
>> %s ms gid %u, %ld bytes%s"
>> 
>> and
>> 
>> Nov 29 01:02:39 dramaqueen iked[49967]: ikev2_msg_send: IKE_SA_INIT
>> request from (null) to 62.48.30.5:500 msgid 0, 438 bytes
>> 
>> The problem seems to be in print_host so try to not return NULL in there.
>> Maybe we could return something else but this is a start IMO.
>
> beck@ prefers to just print unknown instead  of the gai_strerror -- guess
> that is more sensible.

Why would getnameinfo(NI_NUMERICHOST) fail here?  The code in
ikev2_msg.c is:

--8<--
log_info("%s: %s %s from %s to %s msgid %u, %ld bytes%s", __func__,
print_map(exchange, ikev2_exchange_map),
(flags & IKEV2_FLAG_RESPONSE) ? "response" : "request",
print_host((struct sockaddr *)&msg->msg_local, NULL, 0),
print_host((struct sockaddr *)&msg->msg_peer, NULL, 0),
betoh32(hdr->ike_msgid),
ibuf_length(buf), isnatt ? ", NAT-T" : "");
-->8--

Maybe msg->msg_local is corrupt?

-- 
jca | PGP : 0x1524E7EE / 5135 92C1 AD36 5293 2BDF  DDCC 0DFA 74AE 1524 E7EE



Re: iked, don't return NULL in print_host

2017-11-28 Thread Bob Beck
ok beck@

On Wed, Nov 29, 2017 at 02:17:21AM +0100, Claudio Jeker wrote:
> On Wed, Nov 29, 2017 at 01:59:06AM +0100, Claudio Jeker wrote:
> > Seen in my log file:
> > Nov 28 17:47:22 dramaqueen iked: vfprintf %s NULL in "%s: %s %s from %s to
> > %s ms gid %u, %ld bytes%s"
> > 
> > and
> > 
> > Nov 29 01:02:39 dramaqueen iked[49967]: ikev2_msg_send: IKE_SA_INIT
> > request from (null) to 62.48.30.5:500 msgid 0, 438 bytes
> > 
> > The problem seems to be in print_host so try to not return NULL in there.
> > Maybe we could return something else but this is a start IMO.
> 
> beck@ prefers to just print unknown instead  of the gai_strerror -- guess
> that is more sensible.
> 
> -- 
> :wq Claudio
> 
> Index: util.c
> ===
> RCS file: /cvs/src/sbin/iked/util.c,v
> retrieving revision 1.33
> diff -u -p -r1.33 util.c
> --- util.c9 Jan 2017 14:49:21 -   1.33
> +++ util.c29 Nov 2017 01:16:21 -
> @@ -654,8 +654,8 @@ print_host(struct sockaddr *sa, char *bu
>  
>   if (getnameinfo(sa, sa->sa_len,
>   buf, len, NULL, 0, NI_NUMERICHOST) != 0) {
> - buf[0] = '\0';
> - return (NULL);
> + strlcpy(buf, "unknown", len);
> + return (buf);
>   }
>  
>   if ((port = socket_getport(sa)) != 0) {
> 



Re: iked, don't return NULL in print_host

2017-11-28 Thread Claudio Jeker
On Wed, Nov 29, 2017 at 01:59:06AM +0100, Claudio Jeker wrote:
> Seen in my log file:
> Nov 28 17:47:22 dramaqueen iked: vfprintf %s NULL in "%s: %s %s from %s to
> %s ms gid %u, %ld bytes%s"
> 
> and
> 
> Nov 29 01:02:39 dramaqueen iked[49967]: ikev2_msg_send: IKE_SA_INIT
> request from (null) to 62.48.30.5:500 msgid 0, 438 bytes
> 
> The problem seems to be in print_host so try to not return NULL in there.
> Maybe we could return something else but this is a start IMO.

beck@ prefers to just print unknown instead  of the gai_strerror -- guess
that is more sensible.

-- 
:wq Claudio

Index: util.c
===
RCS file: /cvs/src/sbin/iked/util.c,v
retrieving revision 1.33
diff -u -p -r1.33 util.c
--- util.c  9 Jan 2017 14:49:21 -   1.33
+++ util.c  29 Nov 2017 01:16:21 -
@@ -654,8 +654,8 @@ print_host(struct sockaddr *sa, char *bu
 
if (getnameinfo(sa, sa->sa_len,
buf, len, NULL, 0, NI_NUMERICHOST) != 0) {
-   buf[0] = '\0';
-   return (NULL);
+   strlcpy(buf, "unknown", len);
+   return (buf);
}
 
if ((port = socket_getport(sa)) != 0) {



iked, don't return NULL in print_host

2017-11-28 Thread Claudio Jeker
Seen in my log file:
Nov 28 17:47:22 dramaqueen iked: vfprintf %s NULL in "%s: %s %s from %s to
%s ms gid %u, %ld bytes%s"

and

Nov 29 01:02:39 dramaqueen iked[49967]: ikev2_msg_send: IKE_SA_INIT
request from (null) to 62.48.30.5:500 msgid 0, 438 bytes

The problem seems to be in print_host so try to not return NULL in there.
Maybe we could return something else but this is a start IMO.
-- 
:wq Claudio


Index: util.c
===
RCS file: /cvs/src/sbin/iked/util.c,v
retrieving revision 1.33
diff -u -p -r1.33 util.c
--- util.c  9 Jan 2017 14:49:21 -   1.33
+++ util.c  29 Nov 2017 00:52:02 -
@@ -639,6 +639,7 @@ print_host(struct sockaddr *sa, char *bu
static int  idx = 0;
charpbuf[7];
in_port_t   port;
+   int rc;
 
if (buf == NULL) {
buf = sbuf[idx];
@@ -652,10 +653,10 @@ print_host(struct sockaddr *sa, char *bu
return (buf);
}
 
-   if (getnameinfo(sa, sa->sa_len,
-   buf, len, NULL, 0, NI_NUMERICHOST) != 0) {
-   buf[0] = '\0';
-   return (NULL);
+   if ((rc = getnameinfo(sa, sa->sa_len,
+   buf, len, NULL, 0, NI_NUMERICHOST)) != 0) {
+   snprintf(buf, len, "error %s", gai_strerror(rc));
+   return (buf);
}
 
if ((port = socket_getport(sa)) != 0) {