netinet: Disallow connections to INADDR_ANY

[emaste]

Summary

Previously connect() or sendto() to INADDR_ANY reached some socket bound to some host interface address. Although this was intentional it was an artifact of a different era, and is not desirable now.

In 417b35a97b7669eb0bf417b43e97cccbedbce6f9 @markj added support to disallow connect() to INADDR_ANY. Take the next logical step and disable it by default.

[We can do a call for testing on -current first if desired. I'm certain that all potential fallout will not be found by such a request, though.]

Relnotes: Yes
PR: 280705

[markj]

[We can do a call for testing on -current first if desired. I'm certain that all potential fallout will not be found by such a request, though.]

It might be worth giving a heads-up anyway, just so people have a hint if something stops working.

[emaste]

It might be worth giving a heads-up anyway, just so people have a hint if something stops working.

Yeah, at least a post to -net and -current stating that the default changed, ask people to test if possible but at least be on the lookout for new misbehaviour, and advice on setting the sysctl to revert if necessary.

[jhb]

Does this need an entry in RELNOTES as a followup commit perhaps?

[emaste]

Does this need an entry in RELNOTES as a followup commit perhaps?

It will need a release note, indeed. We have at least two ways that can happen today -- as a result of Relnotes: Yes and by adding to RELNOTES. I believe RELNOTES was originally introduced to capture release notes where Relnotes: Yes was forgotten. It is probably better to capture the important aspect of the change at the time it happens and by the author of the change though, and perhaps we should deprecate Relnotes: Yes

[jhb]

In D47740#1088770, @emaste wrote:

Does this need an entry in RELNOTES as a followup commit perhaps?

It will need a release note, indeed. We have at least two ways that can happen today -- as a result of Relnotes: Yes and by adding to RELNOTES. I believe RELNOTES was originally introduced to capture release notes where Relnotes: Yes was forgotten. It is probably better to capture the important aspect of the change at the time it happens and by the author of the change though, and perhaps we should deprecate Relnotes: Yes

I think of Relnotes: yes as being a quick way to signal that someone _else_ should write a release note, whereas adding an entry to RELNOTES means that the release engineering team can just use that language without rewriting it. For trivial things like a version bump that is obvious from the commit log, I think Relnotes: yes is ok, but for anything non-trivial I think RELNOTES is better and we should encourage it.

sys/netinet/in_pcb.c

 #include <stdlib.h>
 #include <string.h>
 #include <unistd.h>
 #include <libxo/xo.h>
 
 #define JP_USER         0x01000000
 #define JP_OPT          0x02000000
 
-#define JLS_XO_VERSION  "1"
+#define JLS_XO_VERSION  "2"
 
 #define PRINT_DEFAULT   0x01
 #define PRINT_HEADER    0x02
 #define PRINT_NAMEVAL   0x04
 #define PRINT_QUOTED    0x08
 #define PRINT_SKIP      0x10
 #define PRINT_VERBOSE   0x20
 #define PRINT_JAIL_NAME 0x40

 #endif
 
 static int add_param(const char *name, void *value, size_t valuelen,
                 struct jailparam *source, unsigned flags);
 static int sort_param(const void *a, const void *b);
 static char *noname(const char *name);
 static char *nononame(const char *name);
 static int print_jail(int pflags, int jflags);
+static int special_print(int pflags, struct jailparam *param);
 static void quoted_print(int pflags, char *name, char *value);
+static void emit_ip_addr_list(int af_family, const char *list_name,
+                struct jailparam *param);
 
 int
 main(int argc, char **argv)
 {
         char *dot, *ep, *jname, *pname;
         int c, i, jflags, jid, lastjid, pflags, spc;
 
         argc = xo_parse_args(argc, argv);
nononame(const char *name)
         return nname;
 }
 
 static int
 print_jail(int pflags, int jflags)
 {
         char *nname, *xo_nname;
         char **param_values;
-        int i, ai, jid, count, n, spc;
-        char ipbuf[INET6_ADDRSTRLEN];
+        int i, jid, n, spc;
 
         jid = jailparam_get(params, nparams, jflags);
         if (jid < 0)
                 return jid;
 
         xo_open_instance("jail");
 
         if (pflags & PRINT_VERBOSE) {
                 xo_emit("{:jid/%6d}{P:  }{:hostname/%-29.29s/%s}{P: }"
                     "{:path/%.74s/%s}\n",
                     *(int *)params[0].jp_value,
                     (char *)params[1].jp_value,
                     (char *)params[2].jp_value);
                 xo_emit("{P:        }{:name/%-29.29s/%s}{P: }{:state/%.74s}\n",
                     (char *)params[3].jp_value,
                     *(int *)params[4].jp_value ? "DYING" : "ACTIVE");
                 xo_emit("{P:        }{:cpusetid/%d}\n", *(int *)params[5].jp_value);
                 n = 6;
 #ifdef INET
                 if (ip4_ok && !strcmp(params[n].jp_name, "ip4.addr")) {
-                        count = params[n].jp_valuelen / sizeof(struct in_addr);
-                        for (ai = 0; ai < count; ai++)
-                                if (inet_ntop(AF_INET,
-                                    &((struct in_addr *)params[n].jp_value)[ai],
-                                    ipbuf, sizeof(ipbuf)) == NULL)
-                                        xo_err(1, "inet_ntop");
-                                else {
-                                        xo_emit("{P:        }{l:ipv4_addrs}{P:\n}", ipbuf);
-                                }
+                        emit_ip_addr_list(AF_INET, "ipv4_addrs", params + n);
                         n++;
                 }
 #endif
 #ifdef INET6
                 if (ip6_ok && !strcmp(params[n].jp_name, "ip6.addr")) {
-                        count = params[n].jp_valuelen / sizeof(struct in6_addr);
-                        for (ai = 0; ai < count; ai++)
-                                if (inet_ntop(AF_INET6,
-                                    &((struct in6_addr *)
-                                        params[n].jp_value)[ai],
-                                    ipbuf, sizeof(ipbuf)) == NULL)
-                                        xo_err(1, "inet_ntop");
-                                else
-                                        xo_emit("{P:        }{l:ipv6_addrs}{P:\n}", ipbuf);
+                        emit_ip_addr_list(AF_INET6, "ipv6_addrs", params + n);
                         n++;
                 }
 #endif
         } else if (pflags & PRINT_DEFAULT) {
                 if (pflags & PRINT_JAIL_NAME)
                         xo_emit("{P: }{:name/%-15s/%s}{P: }",
                             (char *)params[0].jp_value);
                 else
for (i = spc = 0; i < nparams; i++) {
                                                 xo_emit("{d:/%s}", nname);
                                                 free(nname);
                                         }
                                         free(xo_nname);
                                         continue;
                                 }
                                 xo_emit("{d:%s}=", params[i].jp_name);
                         }
-                        if (params[i].jp_valuelen == 0) {
-                                if (pflags & PRINT_QUOTED)
-                                        xo_emit("{P:\"\"}");
-                                else if (!(pflags & PRINT_NAMEVAL))
-                                        xo_emit("{P:-}");
-                        } else {
+                        if (!special_print(pflags, params + i))
                                 quoted_print(pflags, params[i].jp_name, param_values[i]);
-                        }
-                }
                 xo_emit("{P:\n}");
                 for (i = 0; i < nparams; i++)
                         if (params[i].jp_flags & JP_USER)
                                 free(param_values[i]);
         }
 
         xo_close_instance("jail");
         return (jid);
if (qc && pflags & PRINT_QUOTED)
                 xo_emit("{P:/%c}", qc);
 
         xo_emit(param_name_value, value);
 
         free(param_name_value);
 
         if (qc && pflags & PRINT_QUOTED)
                 xo_emit("{P:/%c}", qc);
+}
+
+static int
+special_print(int pflags, struct jailparam *param)
+{
+        int ip_as_list;
+
+        switch (xo_get_style(NULL)) {
+        case XO_STYLE_JSON:
+        case XO_STYLE_XML:
+                ip_as_list = 1;
+                break;
+        default:
+                ip_as_list = 0;
+        }
+
+        if (!ip_as_list && param->jp_valuelen == 0) {
+                if (pflags & PRINT_QUOTED)
+                        xo_emit("{P:\"\"}");
+                else if (!(pflags & PRINT_NAMEVAL))
+                        xo_emit("{P:-}");
+        } else if (ip_as_list && !strcmp(param->jp_name, "ip4.addr")) {
+                emit_ip_addr_list(AF_INET, param->jp_name, param);
+        } else if (ip_as_list && !strcmp(param->jp_name, "ip6.addr")) {
+                emit_ip_addr_list(AF_INET6, param->jp_name, param);
+        } else {
+                return 0;
+        }
+
+        return 1;
+}
+
+static void
+emit_ip_addr_list(int af_family, const char *list_name, struct jailparam *param)
+{
+        char ipbuf[INET6_ADDRSTRLEN];
+        size_t addr_len;
+        const char *emit_str;
+        int ai, count;
+
+        switch (af_family) {
+        case AF_INET:
+                addr_len = sizeof(struct in_addr);
+                emit_str = "{P:        }{ql:ipv4_addr}{P:\n}";
+                break;
+        case AF_INET6:
+                addr_len = sizeof(struct in6_addr);
+                emit_str = "{P:        }{ql:ipv6_addr}{P:\n}";
+                break;
+        default:
+                xo_err(1, "unsupported af_family");
+                return;
+        }
+
+        count = param->jp_valuelen / addr_len;
+
+        xo_open_list(list_name);
+        for (ai = 0; ai < count; ai++) {
+                if (inet_ntop(af_family,
+                    ((uint8_t *)param->jp_value) + addr_len * ai,
+                    ipbuf, sizeof(ipbuf)) == NULL) {
+                        xo_err(1, "inet_ntop");
+                } else {
+                        xo_emit(emit_str, ipbuf);
+                }
+        }
+        xo_close_list(list_name);
 }