Re: follow up to 'once rule' expiration
On Thu, Jul 18, 2019 at 09:46:58AM +0200, Alexandr Nedvedicky wrote:
> Hello,
>
> I've just realized my suggestion [1] to lteo@ was not complete. The single
> atomic_cas() used as I've suggested is not sufficient measure. The code
> should also do a non-atomic test to check, whether the rule is not expired
> yet.
>
> the non-atomic check deals with scenario, where competing thread arrives
> to atomic_cas() well before the current thread.
>
> OK?
Tested and makes sense to me. ok lteo@
> thanks and
> regards
> sashan
>
> [1] https://marc.info/?l=openbsd-bugs=156341942417148=2
>
> 8<---8<---8<--8<
> diff --git a/sys/net/pf.c b/sys/net/pf.c
> index b858c43963b..b001f185665 100644
> --- a/sys/net/pf.c
> +++ b/sys/net/pf.c
> @@ -3963,8 +3963,9 @@ pf_test_rule(struct pf_pdesc *pd, struct pf_rule **rm,
> struct pf_state **sm,
>* insert an expired rule to gcl.
>*/
> rule_flag = r->rule_flag;
> - if (atomic_cas_uint(>rule_flag, rule_flag,
> - rule_flag | PFRULE_EXPIRED) == rule_flag) {
> + if (((rule_flag & PFRULE_EXPIRED) == 0) &&
> + atomic_cas_uint(>rule_flag, rule_flag,
> + rule_flag | PFRULE_EXPIRED) == rule_flag) {
> r->exptime = time_second;
> SLIST_INSERT_HEAD(_rule_gcl, r, gcle);
> }
pf(4) man page: sync with net/pfvar.h
This syncs the pf(4) man page with the latest net/pfvar.h (r1.490).
ok?
Index: pf.4
===
RCS file: /cvs/src/share/man/man4/pf.4,v
retrieving revision 1.91
diff -u -p -r1.91 pf.4
--- pf.418 Feb 2019 13:11:44 - 1.91
+++ pf.423 May 2019 01:09:55 -
@@ -269,6 +269,7 @@ struct pf_status {
u_int64_t pcounters[2][2][3];
u_int64_t bcounters[2][2];
u_int64_t stateid;
+ u_int64_t syncookies_inflight[2]; /* unACKed SYNcookies */
time_t since;
u_int32_t running;
u_int32_t states;
@@ -277,6 +278,9 @@ struct pf_status {
u_int32_t debug;
u_int32_t hostid;
u_int32_t reass; /* reassembly */
+ u_int8_tsyncookies_active;
+ u_int8_tsyncookies_mode;/* never/always/adaptive */
+ u_int8_tpad[2];
charifname[IFNAMSIZ];
u_int8_tpf_chksum[PF_MD5_DIGEST_LENGTH];
};
@@ -411,7 +415,8 @@ struct pfioc_limit {
};
enum { PF_LIMIT_STATES, PF_LIMIT_SRC_NODES, PF_LIMIT_FRAGS,
- PF_LIMIT_TABLES, PF_LIMIT_TABLE_ENTRIES, PF_LIMIT_MAX };
+ PF_LIMIT_TABLES, PF_LIMIT_TABLE_ENTRIES, PF_LIMIT_PKTDELAY_PKTS,
+ PF_LIMIT_MAX };
.Ed
.It Dv DIOCGETLIMIT Fa "struct pfioc_limit *pl"
Get the hard
Re: use getpwuid_r in doas
On Tue, May 21, 2019 at 01:07:10PM -0400, Ted Unangst wrote:
> I have a coming change which will need to access both the calling user and
> target users' passwd entries. In order to accomplish this, we need to switch
> to the reentrant flavor of getpwuid. No behaviorial change, but I think this
> is clearer and less error prone as well, versus reusing a pointer to static
> storage.
ok lteo@
> Index: doas.c
> ===
> RCS file: /home/cvs/src/usr.bin/doas/doas.c,v
> retrieving revision 1.74
> diff -u -p -r1.74 doas.c
> --- doas.c17 Jan 2019 05:35:35 - 1.74
> +++ doas.c21 May 2019 17:04:04 -
> @@ -289,13 +289,15 @@ main(int argc, char **argv)
> const char *cmd;
> char cmdline[LINE_MAX];
> char myname[_PW_NAME_LEN + 1];
> - struct passwd *pw;
> + char mypwbuf[_PW_BUF_LEN], targpwbuf[_PW_BUF_LEN];
> + struct passwd mypwstore, targpwstore;
> + struct passwd *mypw, *targpw;
> const struct rule *rule;
> uid_t uid;
> uid_t target = 0;
> gid_t groups[NGROUPS_MAX + 1];
> int ngroups;
> - int i, ch;
> + int i, ch, rv;
> int sflag = 0;
> int nflag = 0;
> char cwdpath[PATH_MAX];
> @@ -346,10 +348,10 @@ main(int argc, char **argv)
> } else if ((!sflag && !argc) || (sflag && argc))
> usage();
>
> - pw = getpwuid(uid);
> - if (!pw)
> - err(1, "getpwuid failed");
> - if (strlcpy(myname, pw->pw_name, sizeof(myname)) >= sizeof(myname))
> + rv = getpwuid_r(uid, , mypwbuf, sizeof(mypwbuf), );
> + if (rv != 0 || mypw == NULL)
> + err(1, "getpwuid_r failed");
> + if (strlcpy(myname, mypw->pw_name, sizeof(myname)) >= sizeof(myname))
> errx(1, "pw_name too long");
> ngroups = getgroups(NGROUPS_MAX, groups);
> if (ngroups == -1)
> @@ -359,7 +361,7 @@ main(int argc, char **argv)
> if (sflag) {
> sh = getenv("SHELL");
> if (sh == NULL || *sh == '\0') {
> - shargv[0] = strdup(pw->pw_shell);
> + shargv[0] = strdup(mypw->pw_shell);
> if (shargv[0] == NULL)
> err(1, NULL);
> } else
> @@ -415,11 +417,11 @@ main(int argc, char **argv)
> if (pledge("stdio rpath getpw exec id", NULL) == -1)
> err(1, "pledge");
>
> - pw = getpwuid(target);
> - if (!pw)
> + rv = getpwuid_r(target, , targpwbuf, sizeof(targpwbuf),
> );
> + if (rv != 0 || targpw == NULL)
> errx(1, "no passwd entry for target");
>
> - if (setusercontext(NULL, pw, target, LOGIN_SETGROUP |
> + if (setusercontext(NULL, targpw, target, LOGIN_SETGROUP |
> LOGIN_SETPRIORITY | LOGIN_SETRESOURCES | LOGIN_SETUMASK |
> LOGIN_SETUSER) != 0)
> errx(1, "failed to set user context for target");
> @@ -436,7 +438,7 @@ main(int argc, char **argv)
> err(1, "pledge");
>
> syslog(LOG_AUTHPRIV | LOG_INFO, "%s ran command %s as %s from %s",
> - myname, cmdline, pw->pw_name, cwd);
> + myname, cmdline, targpw->pw_name, cwd);
>
> envp = prepenv(rule);
>
>
Re: tetris: remove unused vars
On Sun, May 19, 2019 at 02:39:20PM -0400, Jake Champlin wrote:
> Revision 1.23 added unveil to tetris, yet left two unused variables in
> scores.c
> Removes unused vars in scores.c, that were added to tetris.c in v1.23
Fixed, thanks!
> Index: scores.c
> ===
> RCS file: /cvs/src/games/tetris/scores.c,v
> retrieving revision 1.23
> diff -u -p -r1.23 scores.c
> --- scores.c 18 May 2019 19:38:25 - 1.23
> +++ scores.c 19 May 2019 18:42:35 -
> @@ -91,8 +91,8 @@ static char *thisuser(void);
> static void
> getscores(FILE **fpp)
> {
> - int sd, mint, i, ret;
> - char *mstr, *human, *home;
> + int sd, mint, i;
> + char *mstr, *human;
> FILE *sf;
>
> if (fpp != NULL) {
>
acpi_loadtables(): do not assume rsdt exists
This makes acpi_loadtables() ensure that rsdp->rsdp_rsdt exists before
attempting to to use it.
ok?
Index: acpi.c
===
RCS file: /cvs/src/sys/dev/acpi/acpi.c,v
retrieving revision 1.365
diff -u -p -r1.365 acpi.c
--- acpi.c 11 May 2019 14:59:52 - 1.365
+++ acpi.c 12 May 2019 01:36:26 -
@@ -1363,7 +1363,7 @@ acpi_loadtables(struct acpi_softc *sc, s
NULL, 1);
free(sdt, M_DEVBUF, sizeof(*sdt) + len);
- } else {
+ } else if (rsdp->rsdp_rsdt) {
struct acpi_rsdt *rsdt;
sdt = acpi_maptable(sc, rsdp->rsdp_rsdt, NULL, NULL, NULL, 0);
@@ -1382,6 +1382,9 @@ acpi_loadtables(struct acpi_softc *sc, s
NULL, 1);
free(sdt, M_DEVBUF, sizeof(*sdt) + len);
+ } else {
+ printf("couldn't find xsdt or rsdt\n");
+ return (ENOMEM);
}
return (0);
divert(4): increment divs_errors on if_get failure
This diff modifies divert_packet() to increment the divs_errors counter
on if_get failure so that users will be aware of it via netstat(1).
The same thing is done for divert6_packet().
While here, it also modifies divert_output() to move m_freem(m) below
divstat_inc(divs_errors). This is purely for consistency since all the
other divert_* functions increment the divstat counters before m_freem.
divert6_output() already does this so there is no need to make the same
change there.
ok?
Index: netinet/ip_divert.c
===
RCS file: /cvs/src/sys/netinet/ip_divert.c,v
retrieving revision 1.61
diff -u -p -r1.61 ip_divert.c
--- netinet/ip_divert.c 4 Feb 2019 21:40:52 - 1.61
+++ netinet/ip_divert.c 24 Mar 2019 23:39:58 -
@@ -163,8 +163,8 @@ divert_output(struct inpcb *inp, struct
return (error);
fail:
- m_freem(m);
divstat_inc(divs_errors);
+ m_freem(m);
return (error ? error : EINVAL);
}
@@ -199,6 +199,7 @@ divert_packet(struct mbuf *m, int dir, u
ifp = if_get(m->m_pkthdr.ph_ifidx);
if (ifp == NULL) {
+ divstat_inc(divs_errors);
m_freem(m);
return (0);
}
Index: netinet6/ip6_divert.c
===
RCS file: /cvs/src/sys/netinet6/ip6_divert.c,v
retrieving revision 1.59
diff -u -p -r1.59 ip6_divert.c
--- netinet6/ip6_divert.c 4 Feb 2019 21:40:52 - 1.59
+++ netinet6/ip6_divert.c 24 Mar 2019 23:39:03 -
@@ -205,6 +205,7 @@ divert6_packet(struct mbuf *m, int dir,
ifp = if_get(m->m_pkthdr.ph_ifidx);
if (ifp == NULL) {
+ div6stat_inc(div6s_errors);
m_freem(m);
return (0);
}
Re: Zap PF_TRANS_ALTQ
On Fri, Jan 19, 2018 at 02:18:08PM -0700, Theo de Raadt wrote: > > > On Fri, Jan 19, 2018 at 08:24:23PM +, Stuart Henderson wrote: > > > > To be honest though, unless it's in the way of something, I'm not sure > > > > it's > > > > worth removing. > > > > > > If those constatns are in ports and require revision bumps, we can > > > leave it in the header. Rebuilding base is one thing, but doing > > > manual work with ports is not worth it. > > > > This seems overblown. > > > > The PF API is frozen. > > > > It is only an ABI between the kernel and pfctl -- that is the sticky > > and tricky upgrade position. Everything else can must be able to > > handle change. > > > I meant to say: the PF API is *NOT* frozen. We can change it at any time! > > It is just a tiny bit uncomfortable to incompatibly change the bsd:pfctl > interface. The timing just has to be right. > > ports? Who cares! Thank you all for the feedback, and thanks sthen@ for searching the ports tree. I will coordinate with Theo regarding a suitable time to commit this.
Zap PF_TRANS_ALTQ
Nothing uses PF_TRANS_ALTQ anymore, so zap it.
ok?
Index: pfvar.h
===
RCS file: /cvs/src/sys/net/pfvar.h,v
retrieving revision 1.470
diff -u -p -r1.470 pfvar.h
--- pfvar.h 29 Dec 2017 17:05:25 - 1.470
+++ pfvar.h 19 Jan 2018 03:42:56 -
@@ -68,7 +68,7 @@ enum { PF_INOUT, PF_IN, PF_OUT, PF_FWD }
enum { PF_PASS, PF_DROP, PF_SCRUB, PF_NOSCRUB, PF_NAT, PF_NONAT,
PF_BINAT, PF_NOBINAT, PF_RDR, PF_NORDR, PF_SYNPROXY_DROP, PF_DEFER,
PF_MATCH, PF_DIVERT, PF_RT, PF_AFRT };
-enum { PF_TRANS_RULESET, PF_TRANS_ALTQ, PF_TRANS_TABLE };
+enum { PF_TRANS_RULESET, PF_TRANS_TABLE };
enum { PF_OP_NONE, PF_OP_IRG, PF_OP_EQ, PF_OP_NE, PF_OP_LT,
PF_OP_LE, PF_OP_GT, PF_OP_GE, PF_OP_XRG, PF_OP_RRG };
enum { PF_CHANGE_NONE, PF_CHANGE_ADD_HEAD, PF_CHANGE_ADD_TAIL,
Explicitly check PF_TRANS_RULESET
The pf(4) DIOCX{BEGIN,COMMIT,ROLLBACK} calls support two ruleset types:
PF_TRANS_RULESET and PF_TRANS_TABLE.
However, their switch statements in pf_ioctl.c only check for
PF_TRANS_TABLE and do not check PF_TRANS_RULESET at all.
This diff adds explicit checks for PF_TRANS_RULESET to those switch
statements.
ok?
Index: pf_ioctl.c
===
RCS file: /cvs/src/sys/net/pf_ioctl.c,v
retrieving revision 1.326
diff -u -p -U6 -r1.326 pf_ioctl.c
--- pf_ioctl.c 28 Nov 2017 16:05:46 - 1.326
+++ pf_ioctl.c 19 Jan 2018 03:40:47 -
@@ -2244,21 +2244,27 @@ pfioctl(dev_t dev, u_long cmd, caddr_t a
free(table, M_TEMP, sizeof(*table));
free(ioe, M_TEMP, sizeof(*ioe));
PF_UNLOCK();
goto fail;
}
break;
- default:
+ case PF_TRANS_RULESET:
if ((error = pf_begin_rules(>ticket,
ioe->anchor))) {
free(table, M_TEMP, sizeof(*table));
free(ioe, M_TEMP, sizeof(*ioe));
PF_UNLOCK();
goto fail;
}
break;
+ default:
+ free(table, M_TEMP, sizeof(*table));
+ free(ioe, M_TEMP, sizeof(*ioe));
+ error = EINVAL;
+ PF_UNLOCK();
+ goto fail;
}
if (copyout(ioe, io->array+i, sizeof(io->array[i]))) {
free(table, M_TEMP, sizeof(*table));
free(ioe, M_TEMP, sizeof(*ioe));
error = EFAULT;
PF_UNLOCK();
@@ -2310,21 +2316,27 @@ pfioctl(dev_t dev, u_long cmd, caddr_t a
free(table, M_TEMP, sizeof(*table));
free(ioe, M_TEMP, sizeof(*ioe));
PF_UNLOCK();
goto fail; /* really bad */
}
break;
- default:
+ case PF_TRANS_RULESET:
if ((error = pf_rollback_rules(ioe->ticket,
ioe->anchor))) {
free(table, M_TEMP, sizeof(*table));
free(ioe, M_TEMP, sizeof(*ioe));
PF_UNLOCK();
goto fail; /* really bad */
}
break;
+ default:
+ free(table, M_TEMP, sizeof(*table));
+ free(ioe, M_TEMP, sizeof(*ioe));
+ error = EINVAL;
+ PF_UNLOCK();
+ goto fail; /* really bad */
}
}
free(table, M_TEMP, sizeof(*table));
free(ioe, M_TEMP, sizeof(*ioe));
PF_UNLOCK();
break;
@@ -2370,25 +2382,31 @@ pfioctl(dev_t dev, u_long cmd, caddr_t a
free(ioe, M_TEMP, sizeof(*ioe));
error = EBUSY;
PF_UNLOCK();
goto fail;
}
break;
- default:
+ case PF_TRANS_RULESET:
rs = pf_find_ruleset(ioe->anchor);
if (rs == NULL ||
!rs->rules.inactive.open ||
rs->rules.inactive.ticket !=
ioe->ticket) {
free(table, M_TEMP, sizeof(*table));
free(ioe, M_TEMP, sizeof(*ioe));
error = EBUSY;
PF_UNLOCK();
goto fail;
}
break;
+ default:
+ free(table, M_TEMP, sizeof(*table));
+ free(ioe, M_TEMP, sizeof(*ioe));
+ error = EINVAL;
+
pf.4: sync structs with net/pfvar.h
This syncs the struct declarations in pf.4 with the latest net/pfvar.h
(r1.465 at the time of writing).
ok?
Index: pf.4
===
RCS file: /cvs/src/share/man/man4/pf.4,v
retrieving revision 1.87
diff -u -p -r1.87 pf.4
--- pf.430 May 2017 19:38:47 - 1.87
+++ pf.428 Aug 2017 01:38:22 -
@@ -94,8 +94,8 @@ struct pfioc_rule {
u_int32_t action;
u_int32_t ticket;
u_int32_t nr;
- charanchor[MAXPATHLEN];
- charanchor_call[MAXPATHLEN];
+ charanchor[PATH_MAX];
+ charanchor_call[PATH_MAX];
struct pf_rule rule;
};
.Ed
@@ -186,7 +186,7 @@ for the queue specified by
.Bd -literal
struct pfioc_ruleset {
u_int32_tnr;
- char path[MAXPATHLEN];
+ char path[PATH_MAX];
char name[PF_ANCHOR_NAME_SIZE];
};
.Ed
@@ -272,12 +272,13 @@ struct pf_status {
time_t since;
u_int32_t running;
u_int32_t states;
+ u_int32_t states_halfopen;
u_int32_t src_nodes;
u_int32_t debug;
u_int32_t hostid;
u_int32_t reass; /* reassembly */
charifname[IFNAMSIZ];
- u_int8_tpf_chksum[MD5_DIGEST_LENGTH];
+ u_int8_tpf_chksum[PF_MD5_DIGEST_LENGTH];
};
.Ed
.It Dv DIOCCLRSTATUS
@@ -462,7 +463,7 @@ On exit,
contains the number of tables effectively created.
.Bd -literal
struct pfr_table {
- charpfrt_anchor[MAXPATHLEN];
+ charpfrt_anchor[PATH_MAX];
charpfrt_name[PF_TABLE_NAME_SIZE];
u_int32_t pfrt_flags;
u_int8_tpfrt_fback;
@@ -746,7 +747,7 @@ struct pfioc_trans {
int esize; /* size of each element in bytes */
struct pfioc_trans_e {
int type;
- charanchor[MAXPATHLEN];
+ charanchor[PATH_MAX];
u_int32_t ticket;
} *array;
};
n_time in trpt(8)
In 2014, mpi@ substituted n_time, n_long, and n_short with their equivalent u_int_* types throughout the network stack to remove the dependency on : http://marc.info/?l=openbsd-tech=140523875001860=2 As mentioned in his mail, trpt(8) is the only program in userland that uses n_time. The following diff does the final cleanup for trpt(8). ok? Index: trpt.c === RCS file: /cvs/src/usr.sbin/trpt/trpt.c,v retrieving revision 1.33 diff -u -p -r1.33 trpt.c --- trpt.c 27 Aug 2016 01:50:07 - 1.33 +++ trpt.c 1 Dec 2016 19:17:38 - @@ -73,7 +73,6 @@ #include #include -#include #include #include #include @@ -112,7 +111,7 @@ int tcp_debx; struct tcp_debug tcp_debug[TCP_NDEBUG]; static caddr_t tcp_pcbs[TCP_NDEBUG]; -static n_time ntime; +static u_int32_t ntime; static int aflag, follow, sflag, tflag; extern char *__progname;
libpcap: pcap_set_immediate_mode
This imports pcap_set_immediate_mode() from mainline libpcap, which allows
a libpcap-based program to process packets as soon as they arrive.
ok?
Index: pcap-bpf.c
===
RCS file: /cvs/src/lib/libpcap/pcap-bpf.c,v
retrieving revision 1.34
diff -u -p -r1.34 pcap-bpf.c
--- pcap-bpf.c 8 May 2016 08:20:50 - 1.34
+++ pcap-bpf.c 5 Oct 2016 02:33:50 -
@@ -565,6 +565,16 @@ pcap_activate(pcap_t *p)
}
}
+ if (p->opt.immediate) {
+ v = 1;
+ if (ioctl(p->fd, BIOCIMMEDIATE, ) < 0) {
+ snprintf(p->errbuf, PCAP_ERRBUF_SIZE,
+ "BIOCIMMEDIATE: %s", pcap_strerror(errno));
+ status = PCAP_ERROR;
+ goto bad;
+ }
+ }
+
if (p->opt.promisc) {
/* set promiscuous mode, just warn if it fails */
if (ioctl(p->fd, BIOCPROMISC, NULL) < 0) {
@@ -846,6 +856,7 @@ pcap_create(const char *device, char *er
pcap_set_snaplen(p, 65535); /* max packet size */
p->opt.promisc = 0;
p->opt.buffer_size = 0;
+ p->opt.immediate = 0;
return (p);
}
Index: pcap-int.h
===
RCS file: /cvs/src/lib/libpcap/pcap-int.h,v
retrieving revision 1.13
diff -u -p -r1.13 pcap-int.h
--- pcap-int.h 11 Apr 2014 04:08:58 - 1.13
+++ pcap-int.h 5 Oct 2016 02:34:37 -
@@ -48,6 +48,7 @@ struct pcap_opt {
char*source;
int promisc;
int rfmon;
+ int immediate; /* immediate mode - deliver packets as soon as
they arrive */
};
/*
Index: pcap.3
===
RCS file: /cvs/src/lib/libpcap/pcap.3,v
retrieving revision 1.45
diff -u -p -r1.45 pcap.3
--- pcap.3 6 Apr 2016 09:10:28 - 1.45
+++ pcap.3 5 Oct 2016 02:39:24 -
@@ -78,6 +78,7 @@
.Nm pcap_set_rfmon ,
.Nm pcap_set_timeout ,
.Nm pcap_set_buffer_size ,
+.Nm pcap_set_immediate_mode ,
.Nm pcap_activate ,
.Nm pcap_statustostr ,
.Nm pcap_offline_filter
@@ -195,6 +196,8 @@
.Ft int
.Fn pcap_set_buffer_size "pcap_t *p" "int buffer_size"
.Ft int
+.Fn pcap_set_immediate_mode "pcap_t *p" "int immediate"
+.Ft int
.Fn pcap_activate "pcap_t *p"
.Ft const char *
.Fn pcap_statustostr "int errnum"
@@ -613,6 +616,13 @@ sets the buffer size that will be used o
handle is activated to
.Fa buffer_size ,
which is in units of bytes.
+.Pp
+.Fn pcap_set_immediate_mode
+sets whether immediate mode should be set on a capture handle when
+the handle is activated.
+If
+.Fa immediate
+is non-zero, immediate mode will be set, otherwise it will not be set.
.Pp
.Fn pcap_activate
is used to activate a packet capture handle to look at
Index: pcap.c
===
RCS file: /cvs/src/lib/libpcap/pcap.c,v
retrieving revision 1.19
diff -u -p -r1.19 pcap.c
--- pcap.c 6 Apr 2016 08:02:56 - 1.19
+++ pcap.c 5 Oct 2016 02:24:57 -
@@ -223,6 +223,15 @@ pcap_set_timeout(pcap_t *p, int timeout_
}
int
+pcap_set_immediate_mode(pcap_t *p, int immediate)
+{
+ if (pcap_check_activated(p))
+ return PCAP_ERROR_ACTIVATED;
+ p->opt.immediate = immediate;
+ return 0;
+}
+
+int
pcap_set_buffer_size(pcap_t *p, int buffer_size)
{
if (pcap_check_activated(p))
Index: pcap.h
===
RCS file: /cvs/src/lib/libpcap/pcap.h,v
retrieving revision 1.18
diff -u -p -r1.18 pcap.h
--- pcap.h 6 Apr 2016 08:02:56 - 1.18
+++ pcap.h 5 Oct 2016 02:44:34 -
@@ -168,6 +168,7 @@ int pcap_set_promisc(pcap_t *, int);
intpcap_can_set_rfmon(pcap_t *);
intpcap_set_rfmon(pcap_t *, int);
intpcap_set_timeout(pcap_t *, int);
+intpcap_set_immediate_mode(pcap_t *, int);
intpcap_set_buffer_size(pcap_t *, int);
intpcap_activate(pcap_t *);
Index: shlib_version
===
RCS file: /cvs/src/lib/libpcap/shlib_version,v
retrieving revision 1.15
diff -u -p -r1.15 shlib_version
--- shlib_version 6 Apr 2016 08:02:56 - 1.15
+++ shlib_version 5 Oct 2016 02:41:15 -
@@ -1,2 +1,2 @@
major=8
-minor=1
+minor=2
fix openssl(1) prime output
When the openssl(1) prime command is asked to check the primality of a decimal number, it changes it to hex in the output which is a little confusing: $ openssl prime 976110468996539 377C46DC41DBB is prime The following diff fixes this so that it will always show the original number in the answer: $ openssl prime 976110468996539 976110468996539 is prime Hex values explicitly specified with -hex remain unchanged: $ openssl prime -hex 377C46DC41DBB 377C46DC41DBB is prime ok? Index: prime.c === RCS file: /cvs/src/usr.bin/openssl/prime.c,v retrieving revision 1.7 diff -u -p -u -p -r1.7 prime.c --- prime.c 22 Aug 2015 16:36:05 - 1.7 +++ prime.c 8 Sep 2015 01:17:49 - @@ -177,8 +177,7 @@ prime_main(int argc, char **argv) } } - BN_print(bio_out, bn); - BIO_printf(bio_out, " is %sprime\n", + BIO_printf(bio_out, "%s is %sprime\n", prime, BN_is_prime_ex(bn, prime_config.checks, NULL, NULL) ? "" : "not "); }
openssl(1) remove redundant defines
This diff removes redundant defines in two files.
In s_socket.c, SOCKET_PROTOCOL is defined as IPPROTO_TCP, but it's only
used once.
In s_time.c, NO_SHUTDOWN is always defined, so there is no need for a
bunch of NO_SHUTDOWN #ifdef blocks.
No binary change.
ok?
Index: s_socket.c
===
RCS file: /cvs/src/usr.bin/openssl/s_socket.c,v
retrieving revision 1.7
diff -u -p -u -p -r1.7 s_socket.c
--- s_socket.c 20 Jul 2015 03:22:25 - 1.7
+++ s_socket.c 8 Sep 2015 02:43:13 -
@@ -77,8 +77,6 @@ static int init_server(int *sock, int po
static int init_server_long(int *sock, int port, char *ip, int type);
static int do_accept(int acc_sock, int *sock, char **host);
-#define SOCKET_PROTOCOLIPPROTO_TCP
-
int
init_client(int *sock, char *host, char *port, int type, int af)
{
@@ -187,7 +185,7 @@ init_server_long(int *sock, int port, ch
memcpy(_addr.s_addr, ip, 4);
if (type == SOCK_STREAM)
- s = socket(AF_INET, SOCK_STREAM, SOCKET_PROTOCOL);
+ s = socket(AF_INET, SOCK_STREAM, IPPROTO_TCP);
else/* type == SOCK_DGRAM */
s = socket(AF_INET, SOCK_DGRAM, IPPROTO_UDP);
Index: s_time.c
===
RCS file: /cvs/src/usr.bin/openssl/s_time.c,v
retrieving revision 1.9
diff -u -p -u -p -r1.9 s_time.c
--- s_time.c22 Aug 2015 16:36:05 - 1.9
+++ s_time.c8 Sep 2015 02:44:30 -
@@ -56,8 +56,6 @@
* [including the GNU Public Licence.]
*/
-#define NO_SHUTDOWN
-
/*-
s_time - SSL client connection timer program
Written and donated by Larry Streepy
@@ -341,11 +339,7 @@ s_time_main(int argc, char **argv)
while ((i = SSL_read(scon, buf, sizeof(buf))) > 0)
bytes_read += i;
}
-#ifdef NO_SHUTDOWN
SSL_set_shutdown(scon, SSL_SENT_SHUTDOWN |
SSL_RECEIVED_SHUTDOWN);
-#else
- SSL_shutdown(scon);
-#endif
shutdown(SSL_get_fd(scon), SHUT_RDWR);
close(SSL_get_fd(scon));
@@ -400,11 +394,7 @@ next:
SSL_write(scon, buf, strlen(buf));
while (SSL_read(scon, buf, sizeof(buf)) > 0);
}
-#ifdef NO_SHUTDOWN
SSL_set_shutdown(scon, SSL_SENT_SHUTDOWN | SSL_RECEIVED_SHUTDOWN);
-#else
- SSL_shutdown(scon);
-#endif
shutdown(SSL_get_fd(scon), SHUT_RDWR);
close(SSL_get_fd(scon));
@@ -434,11 +424,7 @@ next:
while ((i = SSL_read(scon, buf, sizeof(buf))) > 0)
bytes_read += i;
}
-#ifdef NO_SHUTDOWN
SSL_set_shutdown(scon, SSL_SENT_SHUTDOWN |
SSL_RECEIVED_SHUTDOWN);
-#else
- SSL_shutdown(scon);
-#endif
shutdown(SSL_get_fd(scon), SHUT_RDWR);
close(SSL_get_fd(scon));
Re: openssl(1) remove redundant defines
On Wed, Sep 09, 2015 at 11:17:55AM -0500, Brent Cook wrote:
> On Wed, Sep 9, 2015 at 10:15 AM, Todd C. Miller
> <todd.mil...@courtesan.com> wrote:
> > On Wed, 09 Sep 2015 10:02:17 -0400, Lawrence Teo wrote:
> >> In s_time.c, NO_SHUTDOWN is always defined, so there is no need for a
> >> bunch of NO_SHUTDOWN #ifdef blocks.
> >
> > I'm less sure about this as without calling SSL_shutdown() the
> > client is not notified. I suppose that's intentional as s_time is
> > just for timing connections?
> >
> > - todd
> >
>
> OK, who has a camera looking over my shoulder? I was just looking at this :)
Oops! Please pay no attention to the drone buzzing behind you. :)
> TBH, I'd rather this were a flag rather than a define. Yes, a knob,
> but this is a benchmark that really should be able to benchmark a full
> TLS connection and shutdown to be accurate. The default behavior of
> faking out the shutdown state machine does make the this run about 25%
> faster, but I would have never known that if not for playing with the
> define.
Thank you all for the feedback. I agree that a flag would be preferred
over recompiling openssl(1).
Here's a diff that adds a flag called -no_shutdown (the underscore is
there to match the -no_* flags used by other openssl(1) commands).
The diff also changes the behavior of s_time so that it will perform
a proper full shutdown by default (i.e. if -no_shutdown is not
specified).
Thoughts?
Index: openssl.1
===
RCS file: /cvs/src/usr.bin/openssl/openssl.1,v
retrieving revision 1.19
diff -u -p -u -p -r1.19 openssl.1
--- openssl.1 11 Aug 2015 05:01:03 - 1.19
+++ openssl.1 10 Sep 2015 01:51:18 -
@@ -7074,6 +7074,7 @@ unknown cipher suites a client says it s
.Op Fl key Ar keyfile
.Op Fl nbio
.Op Fl new
+.Op Fl no_shutdown
.Op Fl reuse
.Op Fl time Ar seconds
.Op Fl verify Ar depth
@@ -7135,6 +7136,10 @@ nor
.Fl reuse
are specified,
they are both on by default and executed in sequence.
+.It Fl no_shutdown
+Shutdown the connection without sending a
+.Dq close notify
+shutdown alert to the server.
.It Fl reuse
Performs the timing test using the same session ID;
this can be used as a test that session caching is working.
Index: s_time.c
===
RCS file: /cvs/src/usr.bin/openssl/s_time.c,v
retrieving revision 1.9
diff -u -p -u -p -r1.9 s_time.c
--- s_time.c22 Aug 2015 16:36:05 - 1.9
+++ s_time.c10 Sep 2015 01:58:17 -
@@ -56,8 +56,6 @@
* [including the GNU Public Licence.]
*/
-#define NO_SHUTDOWN
-
/*-
s_time - SSL client connection timer program
Written and donated by Larry Streepy <stre...@healthcare.com>
@@ -114,6 +112,7 @@ struct {
char *keyfile;
int maxtime;
int nbio;
+ int no_shutdown;
int perform;
int verify;
int verify_depth;
@@ -184,6 +183,12 @@ struct option s_time_options[] = {
.value = 1,
},
{
+ .name = "no_shutdown",
+ .desc = "Shutdown the connection without notifying the server",
+ .type = OPTION_FLAG,
+ .opt.flag = _time_config.no_shutdown,
+ },
+ {
.name = "reuse",
.desc = "Reuse the same session ID for each connection",
.type = OPTION_VALUE,
@@ -221,7 +226,7 @@ s_time_usage(void)
"usage: s_time "
"[-bugs] [-CAfile file] [-CApath directory] [-cert file]\n"
"[-cipher cipherlist] [-connect host:port] [-key keyfile]\n"
- "[-nbio] [-new] [-reuse] [-time seconds]\n"
+ "[-nbio] [-new] [-no_shutdown] [-reuse] [-time seconds]\n"
"[-verify depth] [-www page]\n\n");
options_usage(s_time_options);
}
@@ -341,11 +346,11 @@ s_time_main(int argc, char **argv)
while ((i = SSL_read(scon, buf, sizeof(buf))) > 0)
bytes_read += i;
}
-#ifdef NO_SHUTDOWN
- SSL_set_shutdown(scon, SSL_SENT_SHUTDOWN |
SSL_RECEIVED_SHUTDOWN);
-#else
- SSL_shutdown(scon);
-#endif
+ if (s_time_config.no_shutdown)
+ SSL_set_shutdown(scon, SSL_SENT_SHUTDOWN |
+ SSL_RECEIVED_SHUTDOWN);
+ else
+ SSL_shutdown(scon);
shutdown(SSL_get_fd(scon), SHUT_RDWR);
close(SSL_get_fd(scon));
@@ -400,11 +405,11 @@ next:
SSL_write(scon, buf, strlen(buf));
while (SSL_read(scon, buf, sizeof(buf)) > 0);
}
-#ifdef NO_SHUTDOWN
openssl(1) remove unused defines
This removes several unused defines in openssl(1). No binary change.
ok?
Index: ca.c
===
RCS file: /cvs/src/usr.bin/openssl/ca.c,v
retrieving revision 1.9
diff -u -p -u -p -r1.9 ca.c
--- ca.c22 Aug 2015 16:36:05 - 1.9
+++ ca.c8 Sep 2015 02:13:29 -
@@ -88,15 +88,10 @@
#define STRING_MASK"string_mask"
#define UTF8_IN"utf8"
-#define ENV_DIR"dir"
-#define ENV_CERTS "certs"
-#define ENV_CRL_DIR"crl_dir"
-#define ENV_CA_DB "CA_DB"
#define ENV_NEW_CERTS_DIR "new_certs_dir"
#define ENV_CERTIFICATE"certificate"
#define ENV_SERIAL "serial"
#define ENV_CRLNUMBER "crlnumber"
-#define ENV_CRL"crl"
#define ENV_PRIVATE_KEY"private_key"
#define ENV_DEFAULT_DAYS "default_days"
#define ENV_DEFAULT_STARTDATE "default_startdate"
Index: gendsa.c
===
RCS file: /cvs/src/usr.bin/openssl/gendsa.c,v
retrieving revision 1.2
diff -u -p -u -p -r1.2 gendsa.c
--- gendsa.c22 Aug 2015 16:36:05 - 1.2
+++ gendsa.c8 Sep 2015 02:33:09 -
@@ -74,8 +74,6 @@
#include
#include
-#define DEFBITS512
-
int
gendsa_main(int argc, char **argv)
{
Re: 58.html
I have applied this along with the other amdcf(4) diff you sent, thanks! On Thu, Sep 03, 2015 at 08:41:25PM -0400, Rob Pierce wrote: > href tar and cpio? > > Index: 58.html > === > RCS file: /cvs/www/58.html,v > retrieving revision 1.51 > diff -u -p -r1.51 58.html > --- 58.html 3 Sep 2015 23:19:55 - 1.51 > +++ 58.html 4 Sep 2015 00:39:18 - > @@ -143,7 +143,7 @@ to 5.8. > > sudo in base has been replaced with href="http://www.openbsd.org/cgi-bin/man.cgi?query=doassektion=1;>doas(1), > sudo is available as a package. > href="http://www.openbsd.org/cgi-bin/man.cgi?query=filesektion=1;>file(1) > has been replaced with a new modern implementation, including sandbox and > privilege separation. > - href="http://www.openbsd.org/cgi-bin/man.cgi?query=paxsektion=1;>pax(1) > (and tar(1) and cpio(1)) now prevent archive extraction from escaping the > current directory via symlinks; href="http://www.openbsd.org/cgi-bin/man.cgi?query=tarsektion=1;>tar(1) > without -P option now strips up through any ".." path > components. > + href="http://www.openbsd.org/cgi-bin/man.cgi?query=paxsektion=1;>pax(1) > (and href="http://www.openbsd.org/cgi-bin/man.cgi?query=tarsektion=1;>tar(1) > and href="http://www.openbsd.org/cgi-bin/man.cgi?query=cpiosektion=1;>cpio(1)) > now prevent archive extraction from escaping the current directory via > symlinks; href="http://www.openbsd.org/cgi-bin/man.cgi?query=tarsektion=1;>tar(1) > without -P option now strips up through any ".." path > components. > Improved kernel checks of ELF headers. > ... > >
Re: [PATCH] pcap manpages
Committed, thanks. On Mon, Apr 06, 2015 at 07:30:46PM +0200, Jan Stary wrote: Any comments? On Mar 29 22:24:41, h...@stare.cz wrote: The diff below fixes what seem to be errors in pcap.3, either in factuality or markup or grammar. Jan --- pcap.3.orig Sun Mar 29 22:06:49 2015 +++ pcap.3 Sun Mar 29 22:16:50 2015 @@ -169,7 +169,7 @@ at packets on the network. .Fa source is a string that specifies the network device to open. .Fa snaplen -specifies the maximum number of bytes to capture. +specifies the maximum number of bytes to capture from one packet. .Fa promisc specifies if the interface is to be put into promiscuous mode. (Note that even if this parameter is false, the interface @@ -209,7 +209,7 @@ for writing. The name .Ql - is a synonym for -.Dv stdin . +.Dv stdout . .Dv NULL is returned on failure. .Fa p @@ -229,7 +229,7 @@ can be used to get the error text. .Pp .Fn pcap_dump_fopen allows the use of savefile functions on the already-opened stream -.Dq f . +.Fa f . .Pp .Fn pcap_lookupdev returns a pointer to a network device suitable for use with @@ -276,7 +276,7 @@ pointer which is passed in from .Fn pcap_dispatch , a pointer to the .Fa pcap_pkthdr -struct (which precede the actual network headers and data), +struct (which precedes the actual network headers and data), and a .Fa u_char pointer to the packet data. @@ -351,7 +351,7 @@ return when live read timeouts occur. Rather, specifying a non-zero read timeout to .Fn pcap_open_live and then calling -.Fn pcap_dispatch +.Fn pcap_loop allows the reception and processing of any packets that arrive when the timeout occurs. A negative @@ -490,10 +490,11 @@ It is typically used when just using libpcap for compi .Pp .Fn pcap_fopen_offline may be used to read dumped data from an existing open stream -.Dq fp . +.Fa fp . .Pp .Fn pcap_lib_version returns a string describing the version of libpcap. +.Pp .Fn pcap_datalink_val_to_name and .Fn pcap_datalink_val_to_description
Re: Minor typo in fread.3
On Wed, Mar 04, 2015 at 09:08:21PM -0700, Ryan T May wrote: I found a minor typo in the manpage for fread(3). Thanks, Ryan May Fixed, thanks! Index: fread.3 === RCS file: /cvs/src/lib/libc/stdio/fread.3,v retrieving revision 1.8 diff -u -r1.8 fread.3 --- fread.3 1 May 2014 16:40:36 - 1.8 +++ fread.3 5 Mar 2015 03:11:41 - @@ -87,7 +87,7 @@ If the product of .Fa size and -.Fa nemb +.Fa nmemb results in integer overflow, 0 is returned and errno is set to .Er EOVERFLOW .
sys/mbuf.h: tedu m_act
m_act was the historical name of m_nextpkt, and was already deprecated
at the time DI of 4.4BSD was published. Nothing in our tree uses it,
so I would like to propose removing it. Both FreeBSD and NetBSD have
removed theirs.
I have tested this with make build+release. I also partially tested
this with an amd64 bulk build where dpb successfully built more than
7900 installable packages, but after fighting with frozen builds for a
week I think I'll leave bulk builds to the experts. :)
Thoughts/ok?
Index: mbuf.h
===
RCS file: /cvs/src/sys/sys/mbuf.h,v
retrieving revision 1.185
diff -u -p -r1.185 mbuf.h
--- mbuf.h 7 Feb 2015 02:30:00 - 1.185
+++ mbuf.h 8 Feb 2015 02:59:40 -
@@ -170,7 +170,6 @@ struct mbuf {
#definem_type m_hdr.mh_type
#definem_flags m_hdr.mh_flags
#definem_nextpkt m_hdr.mh_nextpkt
-#definem_act m_nextpkt
#definem_pkthdrM_dat.MH.MH_pkthdr
#definem_ext M_dat.MH.MH_dat.MH_ext
#definem_pktdatM_dat.MH.MH_dat.MH_databuf
libpcap use after free
libpcap has a use after free (found via LLVM).
pcap_close() currently looks like this:
void
pcap_close(pcap_t *p)
{
if (p-opt.source != NULL)
free(p-opt.source);
pcap_cleanup_bpf(p);
free(p);
}
The bug affects libpcap programs that enable monitor mode on 802.11
devices (i.e. if they call pcap_set_rfmon() followed by
pcap_activate()). If pcap_close() is called after that,
pcap_cleanup_bpf() will attempt to use p-opt.source while trying to
disable monitor mode, resulting in a use after free.
The fix is simple (diff below). I tested this with a small program
that calls pcap_create(), pcap_set_rfmon(), pcap_activate(), and
pcap_close() on an iwn(4) device with MALLOC_OPTIONS=AFGJPRX.
With the diff applied, the test program no longer segfaults.
ok?
Index: pcap-bpf.c
===
RCS file: /cvs/src/lib/libpcap/pcap-bpf.c,v
retrieving revision 1.24
diff -u -p -r1.24 pcap-bpf.c
--- pcap-bpf.c 16 Oct 2014 20:08:21 - 1.24
+++ pcap-bpf.c 14 Jan 2015 03:31:28 -
@@ -431,9 +431,8 @@ pcap_cleanup_bpf(pcap_t *p)
void
pcap_close(pcap_t *p)
{
- if (p-opt.source != NULL)
- free(p-opt.source);
pcap_cleanup_bpf(p);
+ free(p-opt.source);
free(p);
}
bpf(4) man page: update SEE ALSO reference
The bpf(4) man page's SEE ALSO section lists this as a reference:
McCanne, S. and Jacobson, V., An efficient, extensible, and
portable network monitor.
That paper is most likely an unpublished draft because it can't be found
online. The only reference I found to it is in a 1992 research report
(http://www.hpl.hp.com/techreports/Compaq-DEC/WRL-92-2.pdf) which lists it as a
Work in progress.
I think the authors' 1993 Winter USENIX paper is a more authoritative reference
for this man page:
McCanne, S. and Jacobson, V., The BSD Packet Filter: A New Architecture
for User-level Packet Capture, 1993 Winter USENIX Conference, January
1993.
The PDF is available here for anyone who's interested:
https://www.usenix.org/conference/usenix-winter-1993-conference/bsd-packet-filter-new-architecture-user-level-packet
ok?
Index: bpf.4
===
RCS file: /cvs/src/share/man/man4/bpf.4,v
retrieving revision 1.34
diff -u -p -r1.34 bpf.4
--- bpf.4 21 Jan 2014 11:03:32 - 1.34
+++ bpf.4 9 Jan 2015 03:46:59 -
@@ -1029,7 +1029,9 @@ struct bpf_insn insns[] = {
.Rs
.%A McCanne, S.
.%A Jacobson, V.
-.%J An efficient, extensible, and portable network monitor
+.%D January 1993
+.%J 1993 Winter USENIX Conference
+.%T The BSD Packet Filter: A New Architecture for User-level Packet Capture
.Re
.Sh HISTORY
The Enet packet filter was created in 1980 by Mike Accetta and Rick Rashid
tcpdrop usage()
This diff replaces goto fail in tcpdrop with a proper usage() to be
consistent with other programs in the tree.
ok?
Index: tcpdrop.c
===
RCS file: /cvs/src/usr.sbin/tcpdrop/tcpdrop.c,v
retrieving revision 1.15
diff -u -p -r1.15 tcpdrop.c
--- tcpdrop.c 1 Jan 2015 03:27:56 - 1.15
+++ tcpdrop.c 4 Jan 2015 04:49:48 -
@@ -33,7 +33,21 @@
#include stdlib.h
#include netdb.h
-extern char *__progname;
+__dead void usage(void);
+
+__dead void
+usage(void)
+{
+ extern char *__progname;
+
+ fprintf(stderr,
+ usage: %s local-addr local-port remote-addr remote-port\n,
+ __progname);
+ fprintf(stderr,
+ %s local-addr:local-port remote-addr:remote-port\n,
+ __progname);
+ exit(1);
+}
/*
* Drop a tcp connection.
@@ -61,7 +75,7 @@ main(int argc, char **argv)
if (port1)
*port1++ = '\0';
else
- goto fail;
+ usage();
faddr2 = addr2 = strdup(argv[2]);
if (!addr2)
@@ -70,22 +84,14 @@ main(int argc, char **argv)
if (port2)
*port2++ = '\0';
else
- goto fail;
+ usage();
} else if (argc == 5) {
laddr1 = addr1 = argv[1];
port1 = argv[2];
faddr2 = addr2 = argv[3];
port2 = argv[4];
- } else {
-fail:
- fprintf(stderr,
- usage: %s local-addr local-port remote-addr remote-port\n,
- __progname);
- fprintf(stderr,
- %s local-addr:local-port remote-addr:remote-port\n,
- __progname);
- exit(1);
- }
+ } else
+ usage();
if (addr1[0] == '[' addr1[strlen(addr1) - 1] == ']') {
laddr1 = strdup(addr1);
Re: tcpdrop freeaddrinfo
On Tue, Dec 30, 2014 at 07:44:51AM +0100, Claudio Jeker wrote:
On Tue, Dec 30, 2014 at 12:00:38AM -0500, Lawrence Teo wrote:
tcpdrop makes two getaddrinfo() calls:
if ((gaierr = getaddrinfo(laddr1, port1, hints, laddr)) != 0)
errx(1, %s port %s: %s, addr1, port1,
gai_strerror(gaierr));
if ((gaierr = getaddrinfo(faddr2, port2, hints, faddr)) != 0) {
freeaddrinfo(laddr);
errx(1, %s port %s: %s, addr2, port2,
gai_strerror(gaierr));
}
However, their corresponding freeaddrinfo() calls will not be called if
either of the two getnameinfo() calls in the subsequent for loop fails.
This diff ensures that both freeaddrinfo() calls are called in that
error path.
ok?
I see no need for this. errx() does not return and it frees all the memory
including the one allocated by getaddrinfo(). Why make this more
complicated when the program just calls exit as next?
I would even remove the freeaddrinfo(laddr); in the second case.
Claudio, thank you for the feedback; it makes the diff much simpler. :)
Index: tcpdrop.c
===
RCS file: /cvs/src/usr.sbin/tcpdrop/tcpdrop.c,v
retrieving revision 1.14
diff -u -p -u -p -r1.14 tcpdrop.c
--- tcpdrop.c 29 Jun 2014 00:58:45 - 1.14
+++ tcpdrop.c 31 Dec 2014 03:03:50 -
@@ -106,11 +106,9 @@ fail:
errx(1, %s port %s: %s, addr1, port1,
gai_strerror(gaierr));
- if ((gaierr = getaddrinfo(faddr2, port2, hints, faddr)) != 0) {
- freeaddrinfo(laddr);
+ if ((gaierr = getaddrinfo(faddr2, port2, hints, faddr)) != 0)
errx(1, %s port %s: %s, addr2, port2,
gai_strerror(gaierr));
- }
rval = 1;
for (ail = laddr; ail; ail = ail-ai_next) {
tcpdrop freeaddrinfo
tcpdrop makes two getaddrinfo() calls:
if ((gaierr = getaddrinfo(laddr1, port1, hints, laddr)) != 0)
errx(1, %s port %s: %s, addr1, port1,
gai_strerror(gaierr));
if ((gaierr = getaddrinfo(faddr2, port2, hints, faddr)) != 0) {
freeaddrinfo(laddr);
errx(1, %s port %s: %s, addr2, port2,
gai_strerror(gaierr));
}
However, their corresponding freeaddrinfo() calls will not be called if
either of the two getnameinfo() calls in the subsequent for loop fails.
This diff ensures that both freeaddrinfo() calls are called in that
error path.
ok?
Index: tcpdrop.c
===
RCS file: /cvs/src/usr.sbin/tcpdrop/tcpdrop.c,v
retrieving revision 1.14
diff -u -p -r1.14 tcpdrop.c
--- tcpdrop.c 29 Jun 2014 00:58:45 - 1.14
+++ tcpdrop.c 23 Dec 2014 03:58:56 -
@@ -47,7 +47,7 @@ main(int argc, char **argv)
char lhbuf[NI_MAXHOST], lsbuf[NI_MAXSERV];
char *laddr1, *addr1, *port1, *faddr2, *addr2, *port2;
struct tcp_ident_mapping tir;
- int gaierr, rval = 0;
+ int gaierr = 0, rval = 0;
memset(hints, 0, sizeof(hints));
hints.ai_family = AF_UNSPEC;
@@ -125,11 +125,11 @@ fail:
if ((gaierr = getnameinfo(aif-ai_addr, aif-ai_addrlen,
fhbuf, sizeof(fhbuf), fsbuf, sizeof(fsbuf),
NI_NUMERICHOST | NI_NUMERICSERV)) != 0)
- errx(1, getnameinfo: %s,
gai_strerror(gaierr));
+ goto done;
if ((gaierr = getnameinfo(ail-ai_addr, ail-ai_addrlen,
lhbuf, sizeof(lhbuf), lsbuf, sizeof(lsbuf),
NI_NUMERICHOST | NI_NUMERICSERV)) != 0)
- errx(1, getnameinfo: %s,
gai_strerror(gaierr));
+ goto done;
if (sysctl(mib, sizeof (mib) / sizeof (int), NULL,
NULL, tir, sizeof(tir)) == -1) {
@@ -145,7 +145,10 @@ fail:
}
}
}
+done:
freeaddrinfo(laddr);
freeaddrinfo(faddr);
+ if (gaierr)
+ errx(1, getnameinfo: %s, gai_strerror(gaierr));
exit(rval);
}
divert(4) m_pullup
Make divert_output() do an m_pullup only if truly needed.
ok?
Index: netinet/ip_divert.c
===
RCS file: /cvs/src/sys/netinet/ip_divert.c,v
retrieving revision 1.31
diff -u -p -r1.31 ip_divert.c
--- netinet/ip_divert.c 5 Dec 2014 15:50:04 - 1.31
+++ netinet/ip_divert.c 13 Dec 2014 04:32:23 -
@@ -101,7 +101,8 @@ divert_output(struct inpcb *inp, struct
/* Do basic sanity checks. */
if (m-m_pkthdr.len sizeof(struct ip))
goto fail;
- if ((m = m_pullup(m, sizeof(struct ip))) == NULL) {
+ if (m-m_len sizeof(struct ip)
+ (m = m_pullup(m, sizeof(struct ip))) == NULL) {
/* m_pullup() has freed the mbuf, so just return. */
divstat.divs_errors++;
return (ENOBUFS);
Index: netinet6/ip6_divert.c
===
RCS file: /cvs/src/sys/netinet6/ip6_divert.c,v
retrieving revision 1.31
diff -u -p -r1.31 ip6_divert.c
--- netinet6/ip6_divert.c 5 Dec 2014 15:50:04 - 1.31
+++ netinet6/ip6_divert.c 13 Dec 2014 04:32:24 -
@@ -104,7 +104,8 @@ divert6_output(struct inpcb *inp, struct
/* Do basic sanity checks. */
if (m-m_pkthdr.len sizeof(struct ip6_hdr))
goto fail;
- if ((m = m_pullup(m, sizeof(struct ip6_hdr))) == NULL) {
+ if (m-m_len sizeof(struct ip6_hdr)
+ (m = m_pullup(m, sizeof(struct ip6_hdr))) == NULL) {
/* m_pullup() has freed the mbuf, so just return. */
div6stat.divs_errors++;
return (ENOBUFS);
Re: pcap(3) manpage fixes
On Fri, Dec 12, 2014 at 03:32:31PM +0100, Ingo Schwarze wrote: Hi Kaspars, Kaspars Bankovskis wrote on Fri, Dec 12, 2014 at 03:22:16PM +0200: Function arguments in synopsis for pcap_inject and pcap_sendpacket are a bit messed up by comma. Types updated from actual code. And some .An and .In macro fixes while there. Committed, thanks. Some more argument names are missing, and i wouldn't be surprised if more argument types were wrong. If someone knowledgeable in this area could clean it up, that might be welcome. Here's my attempt to clean it up. This adds the missing argument names and ensures that the argument types and names are the same as those used in the code. ok? Index: pcap.3 === RCS file: /cvs/src/lib/libpcap/pcap.3,v retrieving revision 1.36 diff -u -p -u -p -r1.36 pcap.3 --- pcap.3 12 Dec 2014 14:23:17 - 1.36 +++ pcap.3 16 Dec 2014 05:07:38 - @@ -28,7 +28,7 @@ .Sh SYNOPSIS .In pcap.h .Ft pcap_t * -.Fn pcap_open_live char *device int snaplen int promisc int to_ms char *errbuf +.Fn pcap_open_live const char *source int snaplen int promisc int to_ms char *errbuf .Ft pcap_t * .Fn pcap_open_offline char *fname char *errbuf .Ft pcap_dumper_t * @@ -44,23 +44,23 @@ .Ft int .Fn pcap_loop pcap_t *p int cnt pcap_handler callback u_char *user .Ft void -.Fn pcap_dump u_char *user struct pcap_pkthdr *h u_char *sp +.Fn pcap_dump u_char *user const struct pcap_pkthdr *h const u_char *sp .Ft int .Fn pcap_inject pcap_t *p const void *buf size_t len .Ft int .Fn pcap_sendpacket pcap_t *p const u_char *buf int size .Ft int -.Fn pcap_compile pcap_t *p struct bpf_program *fp char *str int optimize bpf_u_int32 netmask +.Fn pcap_compile pcap_t *p struct bpf_program *program char *buf int optimize bpf_u_int32 netmask .Ft int .Fn pcap_setfilter pcap_t *p struct bpf_program *fp .Ft void -.Fn pcap_freecode struct bpf_program *fp +.Fn pcap_freecode struct bpf_program *program .Ft u_char * .Fn pcap_next pcap_t *p struct pcap_pkthdr *h .Ft int -.Fn pcap_next_ex pcap_t *p struct pcap_pkthdr **hp const u_char **pktp +.Fn pcap_next_ex pcap_t *p struct pcap_pkthdr **pkt_header const u_char **pkt_data .Ft int -.Fn pcap_setdirection pcap_t *p pcap_direction_t dir +.Fn pcap_setdirection pcap_t *p pcap_direction_t d .Ft int .Fn pcap_datalink pcap_t *p .Ft int @@ -78,13 +78,13 @@ .Ft int .Fn pcap_fileno pcap_t *p .Ft int -.Fn pcap_get_selectable_fd pcap_t * +.Fn pcap_get_selectable_fd pcap_t *p .Ft void .Fn pcap_perror pcap_t *p char *prefix .Ft char * .Fn pcap_geterr pcap_t *p .Ft char * -.Fn pcap_strerror int error +.Fn pcap_strerror int errnum .Ft void .Fn pcap_close pcap_t *p .Ft FILE * @@ -108,7 +108,7 @@ .Ft int .Fn pcap_set_datalink pcap_t * int dlt .Ft int -.Fn pcap_list_datalinks pcap_t *p int **dlts +.Fn pcap_list_datalinks pcap_t *p int **dlt_buffer .Ft pcap_t .Fn pcap_open_dead int linktype int snaplen .Ft pcap_t @@ -116,13 +116,13 @@ .Ft const char * .Fn pcap_lib_version void .Ft const char * -.Fn pcap_datalink_val_to_name int +.Fn pcap_datalink_val_to_name int dlt .Ft const char * -.Fn pcap_datalink_val_to_description int +.Fn pcap_datalink_val_to_description int dlt .Ft int .Fn pcap_datalink_name_to_val const char * .Ft pcap_t * -.Fn pcap_create const char *source char *errbuf +.Fn pcap_create const char *device char *errbuf .Ft int .Fn pcap_set_snaplen pcap_t *p int snaplen .Ft int @@ -132,13 +132,13 @@ .Ft int .Fn pcap_set_rfmon pcap_t *p int rfmon .Ft int -.Fn pcap_set_timeout pcap_t *p int timeout +.Fn pcap_set_timeout pcap_t *p int timeout_ms .Ft int .Fn pcap_set_buffer_size pcap_t *p int buffer_size .Ft int .Fn pcap_activate pcap_t *p .Ft const char * -.Fn pcap_statustostr int +.Fn pcap_statustostr int errnum .Sh DESCRIPTION .Nm provides a high level interface to packet capture systems. @@ -162,7 +162,7 @@ chars. .Fn pcap_open_live is used to obtain a packet capture descriptor to look at packets on the network. -.Fa device +.Fa source is a string that specifies the network device to open. .Fa snaplen specifies the maximum number of bytes to capture. @@ -305,9 +305,9 @@ It returns 0 on success or \-1 on failur .Pp .Fn pcap_compile is used to compile the string -.Fa str +.Fa buf into a filter program. -.Fa fp +.Fa program is a pointer to a .Fa bpf_program struct and is filled in by
divert(4) icmp length fix
divert_output() has a basic sanity check to ensure that the m_pkthdr.len
for reinjected packets is not shorter than the minimum length based on
the protocol:
if (p_hdrlen m-m_pkthdr.len off + p_hdrlen)
goto fail;
off is the length of the IP header, and p_hdrlen is the expected length
of the protocol header, e.g. sizeof(struct tcphdr) for TCP.
However, this check is currently wrong for ICMP in certain cases,
because sizeof(struct icmp) is 28 but an ICMP header can be as small as
8 bytes, e.g. an ICMP echo request packet with no payload. So this
causes divert(4) to incorrectly discard ICMP packets with a payload
length of 0-19 bytes.
To fix this, I would like to change the check for ICMP from
sizeof(struct icmp) to ICMP_MINLEN, which according to netinet/ip_icmp.h
is the absolute minimum length of an ICMP packet (8 bytes).
I would also like to rename the p_hdrlen variable to min_hdrlen to
better reflect its purpose.
Although ICMPv6 is not affected, this diff renames p_hdrlen in
divert6_output() to min_hdrlen as well to be consistent with
divert_output().
ok?
Index: netinet/ip_divert.c
===
RCS file: /cvs/src/sys/netinet/ip_divert.c,v
retrieving revision 1.26
diff -u -p -r1.26 ip_divert.c
--- netinet/ip_divert.c 12 Jul 2014 19:05:45 - 1.26
+++ netinet/ip_divert.c 13 Jul 2014 03:46:20 -
@@ -85,7 +85,7 @@ divert_output(struct inpcb *inp, struct
struct sockaddr_in *sin;
struct socket *so;
struct ifaddr *ifa;
- int s, error = 0, p_hdrlen = 0, dir;
+ int s, error = 0, min_hdrlen = 0, dir;
struct ip *ip;
u_int16_t off;
@@ -119,22 +119,22 @@ divert_output(struct inpcb *inp, struct
switch (ip-ip_p) {
case IPPROTO_TCP:
- p_hdrlen = sizeof(struct tcphdr);
+ min_hdrlen = sizeof(struct tcphdr);
m-m_pkthdr.csum_flags |= M_TCP_CSUM_OUT;
break;
case IPPROTO_UDP:
- p_hdrlen = sizeof(struct udphdr);
+ min_hdrlen = sizeof(struct udphdr);
m-m_pkthdr.csum_flags |= M_UDP_CSUM_OUT;
break;
case IPPROTO_ICMP:
- p_hdrlen = sizeof(struct icmp);
+ min_hdrlen = ICMP_MINLEN;
m-m_pkthdr.csum_flags |= M_ICMP_CSUM_OUT;
break;
default:
/* nothing */
break;
}
- if (p_hdrlen m-m_pkthdr.len off + p_hdrlen)
+ if (min_hdrlen m-m_pkthdr.len off + min_hdrlen)
goto fail;
m-m_pkthdr.pf.flags |= PF_TAG_DIVERTED_PACKET;
Index: netinet6/ip6_divert.c
===
RCS file: /cvs/src/sys/netinet6/ip6_divert.c,v
retrieving revision 1.27
diff -u -p -r1.27 ip6_divert.c
--- netinet6/ip6_divert.c 12 Jul 2014 19:05:45 - 1.27
+++ netinet6/ip6_divert.c 13 Jul 2014 03:46:20 -
@@ -89,7 +89,7 @@ divert6_output(struct inpcb *inp, struct
struct sockaddr_in6 *sin6;
struct socket *so;
struct ifaddr *ifa;
- int s, error = 0, p_hdrlen = 0, nxt = 0, off, dir;
+ int s, error = 0, min_hdrlen = 0, nxt = 0, off, dir;
struct ip6_hdr *ip6;
m-m_pkthdr.rcvif = NULL;
@@ -128,22 +128,22 @@ divert6_output(struct inpcb *inp, struct
switch (nxt) {
case IPPROTO_TCP:
- p_hdrlen = sizeof(struct tcphdr);
+ min_hdrlen = sizeof(struct tcphdr);
m-m_pkthdr.csum_flags |= M_TCP_CSUM_OUT;
break;
case IPPROTO_UDP:
- p_hdrlen = sizeof(struct udphdr);
+ min_hdrlen = sizeof(struct udphdr);
m-m_pkthdr.csum_flags |= M_UDP_CSUM_OUT;
break;
case IPPROTO_ICMPV6:
- p_hdrlen = sizeof(struct icmp6_hdr);
+ min_hdrlen = sizeof(struct icmp6_hdr);
m-m_pkthdr.csum_flags |= M_ICMP_CSUM_OUT;
break;
default:
/* nothing */
break;
}
- if (p_hdrlen m-m_pkthdr.len off + p_hdrlen)
+ if (min_hdrlen m-m_pkthdr.len off + min_hdrlen)
goto fail;
m-m_pkthdr.pf.flags |= PF_TAG_DIVERTED_PACKET;
Re: ftp(1) User-Agent
On Fri, Jul 11, 2014 at 12:20:00PM +0200, Alexander Hall wrote: On 07/10/14 06:30, Lawrence Teo wrote: About a month ago, I sent a diff that allows ftp(1) to set its User-Agent. Based on feedback from halex@ and deraadt@, I have changed it so that the User-Agent can be set via a -U command-line option instead of an environment variable. I have also fixed a conflict with guenther@'s recent fetch.c commit. Would anyone like to ok this latest version? I was reviewing this and I couldn't help finding it unnecessarily cumbersome. I propose this diff (ontop on the already proposed and committed diff). Apart from making the code simpler, this diff will change two things: Thanks for simplifying this. The original diff used an environment variable and for consistency with the existing code that deals with environment variables, I implemented it within auto_fetch(). When I changed it to use a command-line option, I continued implementing it within auto_fetch() because that was where my original code was. But as your diff shows, that's unnecessary, so I appreciate your work in making it less cumbersome. I agree with your diff except for this part: 1. You may specify -U as many times as you please, using only the last one. This is the behavious I'd expect. What is the use case for specifying multiple -U instances and only choosing the last one? To me that sounds like something I would accidentally do as opposed to something I would intentionally do, so that's why my code tried to prevent it. 2. If you compile with -DSMALL, using -U will produce an error. This does not follow the common, IMO questionable, practice of just ignoring the switches. However I find it a totally reasonable for most unavailable switches (possibly -C aside). OK? /Alexander Index: fetch.c === RCS file: /cvs/src/usr.bin/ftp/fetch.c,v retrieving revision 1.124 diff -u -p -r1.124 fetch.c --- fetch.c 11 Jul 2014 03:31:52 - 1.124 +++ fetch.c 11 Jul 2014 10:18:25 - @@ -1284,9 +1284,6 @@ auto_fetch(int argc, char *argv[], char char *cp, *url, *host, *dir, *file, *portnum; char *username, *pass, *pathstart; char *ftpproxy, *httpproxy; -#ifndef SMALL - char *uagent = NULL; -#endif /* !SMALL */ int rval, xargc; volatile int argpos; int dirhasglob, filehasglob, oautologin; @@ -1307,13 +1304,6 @@ auto_fetch(int argc, char *argv[], char if ((httpproxy = getenv(HTTP_PROXY)) != NULL *httpproxy == '\0') httpproxy = NULL; - if (httpuseragent == NULL) - httpuseragent = HTTP_USER_AGENT; -#ifndef SMALL - else - uagent = httpuseragent; -#endif /* !SMALL */ - /* * Loop through as long as there's files to fetch. */ @@ -1590,9 +1580,6 @@ bad_ftp_url: } if (connected rval != -1) disconnect(0, NULL); -#ifndef SMALL - free(uagent); -#endif /* !SMALL */ return (rval); } Index: main.c === RCS file: /cvs/src/usr.bin/ftp/main.c,v retrieving revision 1.88 diff -u -p -r1.88 main.c --- main.c11 Jul 2014 03:31:52 - 1.88 +++ main.c11 Jul 2014 10:18:25 - @@ -362,19 +362,17 @@ main(volatile int argc, char *argv[]) trace = 1; break; - case 'U': #ifndef SMALL - if (httpuseragent) - errx(1, User-Agent was already defined); - /* Ensure that User-Agent value is in a single line. */ + case 'U': + free (httpuseragent); if (strcspn(optarg, \r\n) != strlen(optarg)) errx(1, Invalid User-Agent: %s., optarg); if (asprintf(httpuseragent, User-Agent: %s, optarg) == -1) errx(1, Can't allocate memory for HTTP(S) User-Agent); -#endif /* !SMALL */ break; +#endif /* !SMALL */ case 'v': verbose = 1; @@ -394,6 +392,8 @@ main(volatile int argc, char *argv[]) #ifndef SMALL cookie_load(); #endif /* !SMALL */ + if (httpuseragent == NULL) + httpuseragent = HTTP_USER_AGENT; cpend = 0; /* no pending replies */ proxy = 0; /* proxy not active */
Re: ftp(1) User-Agent
On Fri, Jul 11, 2014 at 05:46:02PM +0200, Alexander Hall wrote:
On 07/11/14 17:35, Lawrence Teo wrote:
On Fri, Jul 11, 2014 at 12:20:00PM +0200, Alexander Hall wrote:
On 07/10/14 06:30, Lawrence Teo wrote:
About a month ago, I sent a diff that allows ftp(1) to set its
User-Agent.
Based on feedback from halex@ and deraadt@, I have changed it so that
the User-Agent can be set via a -U command-line option instead of an
environment variable.
I have also fixed a conflict with guenther@'s recent fetch.c commit.
Would anyone like to ok this latest version?
I was reviewing this and I couldn't help finding it unnecessarily
cumbersome.
I propose this diff (ontop on the already proposed and committed diff).
Apart from making the code simpler, this diff will change two things:
Thanks for simplifying this. The original diff used an environment
variable and for consistency with the existing code that deals with
environment variables, I implemented it within auto_fetch().
When I changed it to use a command-line option, I continued implementing
it within auto_fetch() because that was where my original code was. But
as your diff shows, that's unnecessary, so I appreciate your work in
making it less cumbersome.
I agree with your diff except for this part:
1. You may specify -U as many times as you please, using only the last
one. This is the behavious I'd expect.
What is the use case for specifying multiple -U instances and only
choosing the last one? To me that sounds like something I would
accidentally do as opposed to something I would intentionally do, so
that's why my code tried to prevent it.
Mainly because that's how I would expect any option to work. -o, just to
give one example.
hmmm.. use case:
getfile() {
ftp -U 'firefox' $@
}
getfile http://foo.bar/baz1
getfile http://foo.bar/baz2
getfile -U 'chrome' http://foo.bar/baz3
Ah, thanks. I most likely won't use it that way but I see the point.
OK lteo@
divert(4) checksum cleanup
This diff simplifies divert_output() further by removing the csum_flag
variable and setting the checksum flag in pkthdr directly (the variable
was originally there to help with zeroing the checksum, but we've now
determined that zeroing the checksum is unnecessary so that variable
is no longer needed).
I also noticed in divert_packet() that there is a in_proto_cksum_out()
call to force the protocol checksum to be calculated for outbound
packets before they are sent to userspace. This call was added in
ip_divert.c r1.9 before divert_output() gained the ability to
recalculate checksums in r1.13.
Since checksums for all packets are now recalculated on reinjection
anyway, this call is not needed any more. My divert(4) tests continue
to be successful without this call.
ok?
Index: netinet/ip_divert.c
===
RCS file: /cvs/src/sys/netinet/ip_divert.c,v
retrieving revision 1.24
diff -u -p -r1.24 ip_divert.c
--- netinet/ip_divert.c 12 Jul 2014 03:27:00 - 1.24
+++ netinet/ip_divert.c 12 Jul 2014 04:03:56 -
@@ -87,7 +87,7 @@ divert_output(struct inpcb *inp, struct
struct ifaddr *ifa;
int s, error = 0, p_hdrlen = 0, dir;
struct ip *ip;
- u_int16_t off, csum_flag = 0;
+ u_int16_t off;
m-m_pkthdr.rcvif = NULL;
m-m_nextpkt = NULL;
@@ -120,15 +120,15 @@ divert_output(struct inpcb *inp, struct
switch (ip-ip_p) {
case IPPROTO_TCP:
p_hdrlen = sizeof(struct tcphdr);
- csum_flag = M_TCP_CSUM_OUT;
+ m-m_pkthdr.csum_flags |= M_TCP_CSUM_OUT;
break;
case IPPROTO_UDP:
p_hdrlen = sizeof(struct udphdr);
- csum_flag = M_UDP_CSUM_OUT;
+ m-m_pkthdr.csum_flags |= M_UDP_CSUM_OUT;
break;
case IPPROTO_ICMP:
p_hdrlen = sizeof(struct icmp);
- csum_flag = M_ICMP_CSUM_OUT;
+ m-m_pkthdr.csum_flags |= M_ICMP_CSUM_OUT;
break;
default:
/* nothing */
@@ -137,9 +137,6 @@ divert_output(struct inpcb *inp, struct
if (p_hdrlen m-m_pkthdr.len off + p_hdrlen)
goto fail;
- if (csum_flag)
- m-m_pkthdr.csum_flags |= csum_flag;
-
m-m_pkthdr.pf.flags |= PF_TAG_DIVERTED_PACKET;
if (dir == PF_IN) {
@@ -227,9 +224,6 @@ divert_packet(struct mbuf *m, int dir, u
break;
}
}
- /* force checksum calculation */
- if (dir == PF_OUT)
- in_proto_cksum_out(m, NULL);
if (inp) {
sa = inp-inp_socket;
Index: netinet6/ip6_divert.c
===
RCS file: /cvs/src/sys/netinet6/ip6_divert.c,v
retrieving revision 1.25
diff -u -p -r1.25 ip6_divert.c
--- netinet6/ip6_divert.c 12 Jul 2014 03:27:00 - 1.25
+++ netinet6/ip6_divert.c 12 Jul 2014 04:03:56 -
@@ -91,7 +91,6 @@ divert6_output(struct inpcb *inp, struct
struct ifaddr *ifa;
int s, error = 0, p_hdrlen = 0, nxt = 0, off, dir;
struct ip6_hdr *ip6;
- u_int16_t csum_flag = 0;
m-m_pkthdr.rcvif = NULL;
m-m_nextpkt = NULL;
@@ -130,15 +129,15 @@ divert6_output(struct inpcb *inp, struct
switch (nxt) {
case IPPROTO_TCP:
p_hdrlen = sizeof(struct tcphdr);
- csum_flag = M_TCP_CSUM_OUT;
+ m-m_pkthdr.csum_flags |= M_TCP_CSUM_OUT;
break;
case IPPROTO_UDP:
p_hdrlen = sizeof(struct udphdr);
- csum_flag = M_UDP_CSUM_OUT;
+ m-m_pkthdr.csum_flags |= M_UDP_CSUM_OUT;
break;
case IPPROTO_ICMPV6:
p_hdrlen = sizeof(struct icmp6_hdr);
- csum_flag = M_ICMP_CSUM_OUT;
+ m-m_pkthdr.csum_flags |= M_ICMP_CSUM_OUT;
break;
default:
/* nothing */
@@ -147,9 +146,6 @@ divert6_output(struct inpcb *inp, struct
if (p_hdrlen m-m_pkthdr.len off + p_hdrlen)
goto fail;
- if (csum_flag)
- m-m_pkthdr.csum_flags |= csum_flag;
-
m-m_pkthdr.pf.flags |= PF_TAG_DIVERTED_PACKET;
if (dir == PF_IN) {
@@ -233,9 +229,6 @@ divert6_packet(struct mbuf *m, int dir,
break;
}
}
- /* force checksum calculation */
- if (dir == PF_OUT)
- in6_proto_cksum_out(m, NULL);
if (inp) {
sa = inp-inp_socket;
divert(4) checksum offload
Packets that are reinjected via a divert(4) socket will have their IP
and protocol checksums recalculated, since the userspace application
could have modified them.
Currently, these checksums are manually recalculated by divert_output().
But now that the new checksum offloading system is in place, we can use
that instead, at least for reinjected outbound packets.
This diff does the following for reinjected packets:
1. Zero the protocol checksum.
2. Set the checksum flag in pkthdr.
3a. For outbound packets, let the stack take care of the checksum.
3b. For inbound packets, calculate the checksum immediately with
in_proto_cksum_out(m, NULL).
I'm not sure if it's all right to use in_proto_cksum_out() for inbound
packets (its name ends with _out after all :)) but using it really
helps to simplify things and avoid redundant code.
Thoughts/ok?
Index: netinet/ip_divert.c
===
RCS file: /cvs/src/sys/netinet/ip_divert.c,v
retrieving revision 1.23
diff -U7 -p -r1.23 ip_divert.c
--- netinet/ip_divert.c 10 Jul 2014 03:17:59 - 1.23
+++ netinet/ip_divert.c 10 Jul 2014 03:35:07 -
@@ -81,18 +81,17 @@ int
divert_output(struct inpcb *inp, struct mbuf *m, struct mbuf *nam,
struct mbuf *control)
{
struct ifqueue *inq;
struct sockaddr_in *sin;
struct socket *so;
struct ifaddr *ifa;
- int s, error = 0, p_hdrlen = 0;
+ int s, error = 0, p_hdrlen = 0, dir;
struct ip *ip;
- u_int16_t off, csum = 0;
- u_int8_t nxt;
+ u_int16_t off, csum_flag = 0;
size_t p_off = 0;
m-m_pkthdr.rcvif = NULL;
m-m_nextpkt = NULL;
m-m_pkthdr.ph_rtableid = inp-inp_rtableid;
if (control)
@@ -113,64 +112,72 @@ divert_output(struct inpcb *inp, struct
if (ip-ip_v != IPVERSION)
goto fail;
off = ip-ip_hl 2;
if (off sizeof(struct ip) || ntohs(ip-ip_len) off ||
m-m_pkthdr.len ntohs(ip-ip_len))
goto fail;
- /*
-* Recalculate IP and protocol checksums since the userspace application
-* may have modified the packet prior to reinjection.
-*/
- ip-ip_sum = 0;
- ip-ip_sum = in_cksum(m, off);
- nxt = ip-ip_p;
+ dir = (sin-sin_addr.s_addr == INADDR_ANY ? PF_OUT : PF_IN);
+
switch (ip-ip_p) {
case IPPROTO_TCP:
p_hdrlen = sizeof(struct tcphdr);
- p_off = offsetof(struct tcphdr, th_sum);
+ p_off = off + offsetof(struct tcphdr, th_sum);
+ csum_flag = M_TCP_CSUM_OUT;
break;
case IPPROTO_UDP:
p_hdrlen = sizeof(struct udphdr);
- p_off = offsetof(struct udphdr, uh_sum);
+ p_off = off + offsetof(struct udphdr, uh_sum);
+ csum_flag = M_UDP_CSUM_OUT;
break;
case IPPROTO_ICMP:
p_hdrlen = sizeof(struct icmp);
- p_off = offsetof(struct icmp, icmp_cksum);
- nxt = 0;
+ p_off = off + offsetof(struct icmp, icmp_cksum);
+ csum_flag = M_ICMP_CSUM_OUT;
break;
default:
/* nothing */
break;
}
- if (p_hdrlen) {
- if (m-m_pkthdr.len off + p_hdrlen)
- goto fail;
+ if (p_hdrlen m-m_pkthdr.len off + p_hdrlen)
+ goto fail;
- if ((error = m_copyback(m, off + p_off, sizeof(csum), csum,
M_NOWAIT)))
- goto fail;
- csum = in4_cksum(m, nxt, off, m-m_pkthdr.len - off);
- if (ip-ip_p == IPPROTO_UDP csum == 0)
- csum = 0x;
- if ((error = m_copyback(m, off + p_off, sizeof(csum), csum,
M_NOWAIT)))
- goto fail;
+ if (csum_flag) {
+ u_int16_t csum = 0;
+
+ if ((p_off + sizeof(u_int16_t)) m-m_len) {
+ if ((error = m_copyback(m, p_off, sizeof(csum), csum,
+ M_NOWAIT)))
+ goto fail;
+ } else
+ *(u_int16_t *)(mtod(m, caddr_t) + p_off) = 0;
+ m-m_pkthdr.csum_flags |= csum_flag;
}
m-m_pkthdr.pf.flags |= PF_TAG_DIVERTED_PACKET;
- if (sin-sin_addr.s_addr != INADDR_ANY) {
+ if (dir == PF_IN) {
ipaddr.sin_addr = sin-sin_addr;
ifa = ifa_ifwithaddr(sintosa(ipaddr), m-m_pkthdr.ph_rtableid);
if (ifa == NULL) {
error = EADDRNOTAVAIL;
goto fail;
}
m-m_pkthdr.rcvif = ifa-ifa_ifp;
inq = ipintrq;
+
+ /*
+* Recalculate IP and protocol checksums for the inbound packet
+* since the userspace application may have
Re: ftp(1) User-Agent
About a month ago, I sent a diff that allows ftp(1) to set its
User-Agent.
Based on feedback from halex@ and deraadt@, I have changed it so that
the User-Agent can be set via a -U command-line option instead of an
environment variable.
I have also fixed a conflict with guenther@'s recent fetch.c commit.
Would anyone like to ok this latest version?
Index: fetch.c
===
RCS file: /cvs/src/usr.bin/ftp/fetch.c,v
retrieving revision 1.123
diff -u -p -r1.123 fetch.c
--- fetch.c 5 Jul 2014 09:20:54 - 1.123
+++ fetch.c 9 Jul 2014 03:41:16 -
@@ -884,10 +884,10 @@ again:
ftp_printf(fin, ssl, GET %s HTTP/1.0\r\n
Proxy-Authorization: Basic %s%s\r\n%s\r\n\r\n,
epath, credentials, buf ? buf : ,
- HTTP_USER_AGENT);
+ httpuseragent);
else
ftp_printf(fin, ssl, GET %s HTTP/1.0\r\n%s%s\r\n\r\n,
- epath, buf ? buf : , HTTP_USER_AGENT);
+ epath, buf ? buf : , httpuseragent);
} else {
#ifndef SMALL
@@ -945,7 +945,7 @@ again:
ftp_printf(fin, ssl, :%s, port);
#endif /* !SMALL */
ftp_printf(fin, ssl, \r\n%s%s\r\n\r\n,
- buf ? buf : , HTTP_USER_AGENT);
+ buf ? buf : , httpuseragent);
if (verbose)
fprintf(ttyout, \n);
}
@@ -1284,6 +1284,9 @@ auto_fetch(int argc, char *argv[], char
char *cp, *url, *host, *dir, *file, *portnum;
char *username, *pass, *pathstart;
char *ftpproxy, *httpproxy;
+#ifndef SMALL
+ char *uagent = NULL;
+#endif /* !SMALL */
int rval, xargc;
volatile int argpos;
int dirhasglob, filehasglob, oautologin;
@@ -1304,6 +1307,13 @@ auto_fetch(int argc, char *argv[], char
if ((httpproxy = getenv(HTTP_PROXY)) != NULL *httpproxy == '\0')
httpproxy = NULL;
+ if (httpuseragent == NULL)
+ httpuseragent = HTTP_USER_AGENT;
+#ifndef SMALL
+ else
+ uagent = httpuseragent;
+#endif /* !SMALL */
+
/*
* Loop through as long as there's files to fetch.
*/
@@ -1580,6 +1590,9 @@ bad_ftp_url:
}
if (connected rval != -1)
disconnect(0, NULL);
+#ifndef SMALL
+ free(uagent);
+#endif /* !SMALL */
return (rval);
}
Index: ftp.1
===
RCS file: /cvs/src/usr.bin/ftp/ftp.1,v
retrieving revision 1.92
diff -u -p -r1.92 ftp.1
--- ftp.1 25 Jun 2014 06:57:42 - 1.92
+++ ftp.1 8 Jul 2014 22:06:04 -
@@ -62,6 +62,7 @@
.Op Fl o Ar output
.Op Fl S Ar ssl_options
.Op Fl s Ar srcaddr
+.Op Fl U Ar useragent
.Sm off
.No http[s]:// Oo Ar user : password No @
.Oc Ar host Oo : Ar port
@@ -268,6 +269,11 @@ of the connection.
Only useful on systems with more than one address.
.It Fl t
Enables packet tracing.
+.It Fl U Ar useragent
+Set
+.Ar useragent
+as the User-Agent for HTTP(S) URL requests.
+If not specified, the default User-Agent is ``OpenBSD ftp''.
.It Fl V
Disable verbose mode, overriding the default of enabled when input
is from a terminal.
Index: ftp_var.h
===
RCS file: /cvs/src/usr.bin/ftp/ftp_var.h,v
retrieving revision 1.33
diff -u -p -r1.33 ftp_var.h
--- ftp_var.h 24 Dec 2013 13:00:59 - 1.33
+++ ftp_var.h 12 Jun 2014 19:32:51 -
@@ -181,6 +181,7 @@ char *httpport; /* port number to use
#ifndef SMALL
char *httpsport; /* port number to use for https connections */
#endif /* !SMALL */
+char *httpuseragent; /* user agent for http(s) connections */
char *gateport;/* port number to use for gateftp
connections */
jmp_buftoplevel; /* non-local goto stuff for cmd scanner
*/
Index: main.c
===
RCS file: /cvs/src/usr.bin/ftp/main.c,v
retrieving revision 1.87
diff -u -p -r1.87 main.c
--- main.c 23 Jan 2014 00:39:15 - 1.87
+++ main.c 9 Jul 2014 03:45:03 -
@@ -198,9 +198,10 @@ main(volatile int argc, char *argv[])
#ifndef SMALL
cookiefile = getenv(http_cookies);
#endif /* !SMALL */
+ httpuseragent = NULL;
while ((ch = getopt(argc, argv,
- 46AaCc:dD:Eegik:mno:pP:r:S:s:tvV)) != -1) {
+ 46AaCc:dD:Eegik:mno:pP:r:S:s:tU:vV)) != -1) {
switch (ch) {
case '4':
family = PF_INET;
@@ -361,6 +362,20 @@ main(volatile int argc, char *argv[])
trace = 1;
break;
+ case 'U':
+#ifndef SMALL
+
divert(4) without mbuf tags
The current divert(4) implementation allocates an mbuf tag in pf_test()
to store the divert port specified by a divert-packet PF rule.
The divert_packet() function then looks up that mbuf tag to retrieve the
divert port number before sending the packet to userspace.
As far as I can tell, this approach of using an mbuf tag was borrowed
from divert-to's implementation. However, in the case of divert(4) I
think it's overkill because once the packet has reached userspace,
its mbuf and mbuf tag are no longer needed.
I would like to simplify divert(4)'s implementation by passing the
divert port to divert_packet() directly as an argument, which avoids the
allocation of an mbuf tag completely.
ok?
Index: net/pf.c
===
RCS file: /cvs/src/sys/net/pf.c,v
retrieving revision 1.878
diff -u -p -u -p -r1.878 pf.c
--- net/pf.c20 May 2014 11:03:13 - 1.878
+++ net/pf.c14 Jun 2014 18:12:06 -
@@ -6617,14 +6617,8 @@ done:
}
}
- if (action == PF_PASS r-divert_packet.port) {
- struct pf_divert *divert;
-
- if ((divert = pf_get_divert(pd.m)))
- divert-port = r-divert_packet.port;
-
+ if (action == PF_PASS r-divert_packet.port)
action = PF_DIVERT;
- }
if (pd.pflog) {
struct pf_rule_item *ri;
@@ -6651,12 +6645,12 @@ done:
case PF_DIVERT:
switch (pd.af) {
case AF_INET:
- if (divert_packet(pd.m, pd.dir) == 0)
+ if (!divert_packet(pd.m, pd.dir, r-divert_packet.port))
*m0 = NULL;
break;
#ifdef INET6
case AF_INET6:
- if (divert6_packet(pd.m, pd.dir) == 0)
+ if (!divert6_packet(pd.m, pd.dir,
r-divert_packet.port))
*m0 = NULL;
break;
#endif /* INET6 */
Index: netinet/ip_divert.c
===
RCS file: /cvs/src/sys/netinet/ip_divert.c,v
retrieving revision 1.22
diff -u -p -u -p -r1.22 ip_divert.c
--- netinet/ip_divert.c 23 Apr 2014 14:43:14 - 1.22
+++ netinet/ip_divert.c 14 Jun 2014 17:45:12 -
@@ -189,12 +189,11 @@ fail:
}
int
-divert_packet(struct mbuf *m, int dir)
+divert_packet(struct mbuf *m, int dir, u_int16_t divert_port)
{
struct inpcb *inp;
struct socket *sa = NULL;
struct sockaddr_in addr;
- struct pf_divert *divert;
inp = NULL;
divstat.divs_ipackets++;
@@ -205,15 +204,8 @@ divert_packet(struct mbuf *m, int dir)
return (0);
}
- divert = pf_find_divert(m);
- if (divert == NULL) {
- divstat.divs_errors++;
- m_freem(m);
- return (0);
- }
-
TAILQ_FOREACH(inp, divbtable.inpt_queue, inp_queue) {
- if (inp-inp_lport != divert-port)
+ if (inp-inp_lport != divert_port)
continue;
if (inp-inp_divertfl == 0)
break;
Index: netinet/ip_divert.h
===
RCS file: /cvs/src/sys/netinet/ip_divert.h,v
retrieving revision 1.5
diff -u -p -u -p -r1.5 ip_divert.h
--- netinet/ip_divert.h 23 Apr 2014 14:43:14 - 1.5
+++ netinet/ip_divert.h 14 Jun 2014 17:52:05 -
@@ -55,7 +55,7 @@ extern struct divstat divstat;
voiddivert_init(void);
voiddivert_input(struct mbuf *, ...);
-int divert_packet(struct mbuf *, int);
+int divert_packet(struct mbuf *, int, u_int16_t);
int divert_sysctl(int *, u_int, void *, size_t *, void *, size_t);
int divert_usrreq(struct socket *,
int, struct mbuf *, struct mbuf *, struct mbuf *, struct proc *);
Index: netinet6/ip6_divert.c
===
RCS file: /cvs/src/sys/netinet6/ip6_divert.c,v
retrieving revision 1.23
diff -u -p -u -p -r1.23 ip6_divert.c
--- netinet6/ip6_divert.c 28 Apr 2014 15:43:04 - 1.23
+++ netinet6/ip6_divert.c 14 Jun 2014 18:13:03 -
@@ -188,12 +188,11 @@ fail:
}
int
-divert6_packet(struct mbuf *m, int dir)
+divert6_packet(struct mbuf *m, int dir, u_int16_t divert_port)
{
struct inpcb *inp;
struct socket *sa = NULL;
struct sockaddr_in6 addr;
- struct pf_divert *divert;
inp = NULL;
div6stat.divs_ipackets++;
@@ -204,15 +203,8 @@ divert6_packet(struct mbuf *m, int dir)
return (0);
}
- divert = pf_find_divert(m);
- if (divert == NULL) {
- div6stat.divs_errors++;
- m_freem(m);
- return (0);
- }
-
TAILQ_FOREACH(inp, divb6table.inpt_queue, inp_queue) {
- if (inp-inp_lport !=
tcpdump: fix/improve bad checksum display
The way tcpdump displays bad checksums is annoyingly inconsistent.
Here's an example where tcpdump is showing two packets: one with a
bad IP checksum, and another with a bad TCP checksum (emphasis mine):
Bad IP checksum
14:38:42.489639 192.168.30.1.20 192.168.30.70.80: S [tcp sum ok]
0:0(0) win 8192 (ttl 64, id 1, len 40, bad cksum abcd! differs by 116a)
^^^
Bad TCP checksum
14:38:44.900411 192.168.30.1.20 192.168.30.70.80: S [bad tcp cksum 1926!]
0:0(0) win 8192 (ttl 64, id 1, len 40) ^^^
Here are the inconsistencies:
1. differs by is shown for the bad IP checksum but not for the bad TCP
checksum.
2. The bad cksum value 0xabcd in the first packet is the bad IP checksum
within the IP header itself (my Scapy-crafted packet intentionally used
a bad IP checksum of 0xabcd); based on this, one would think that the
bad tcp cksum of 0x1926 in the second packet is the bad TCP checksum
within the TCP header too. But it's not!
Despite being called bad tcp cksum, 0x1926 is actually the
difference between the bad checksum in the TCP header and what the
good checksum should be (in other words, it's the differs by value).
3. The actual bad TCP checksum in the TCP header is not shown at all.
To fix these inconsistencies, the following diff forces tcpdump to
always show the actual bad checksum within the header. As a bonus, it
also shows what the correct checksum should be (concept and code
borrowed from mainline tcpdump).
For example, here's what the fixed tcpdump shows for a variety of bad
checksums (all of these packets use a crafted bad checksum of 0xabcd):
Bad IP checksum
14:38:42.489639 192.168.30.1.20 192.168.30.70.80: S [tcp sum ok]
0:0(0) win 8192 (ttl 64, id 1, len 40, bad ip cksum abcd! - bd37)
Bad TCP checksum (over IPv4)
14:38:44.900411 192.168.30.1.20 192.168.30.70.80: S
[bad tcp cksum abcd! - d1e6] 0:0(0) win 8192 (ttl 64, id 1, len 40)
Bad UDP checksum (over IPv4)
14:38:47.286166 192.168.30.1.53 192.168.30.70.53:
[bad udp cksum abcd! - 41dc] 0 [0q] (0) (ttl 64, id 1, len 28)
Bad ICMP checksum
14:38:49.686247 192.168.30.1 192.168.30.70: icmp: echo request (id:
seq:0) [bad icmp cksum abcd! - f7ff] (ttl 64, id 1, len 28)
Bad TCP checksum (over IPv6)
14:38:52.127616 2001:470::1::1.20 2001:470::1::70.80: S
[bad tcp cksum abcd! - 7ecc] 0:0(0) win 8192 (len 20, hlim 64)
Bad UDP checksum (over IPv6)
14:38:54.531458 2001:470::1::1.53 2001:470::1::70.53:
[bad udp cksum abcd! - eec1][|domain] (len 8, hlim 64)
Bad ICMPv6 checksum
14:38:56.937431 2001:470::1::1 2001:470::1::70: icmp6: echo request
(id: seq:0) [bad icmp6 cksum abcd! - 6f0a] (len 8, hlim 64)
Tested on amd64, i386, and macppc.
ok?
Index: Makefile
===
RCS file: /cvs/src/usr.sbin/tcpdump/Makefile,v
retrieving revision 1.54
diff -u -p -r1.54 Makefile
--- Makefile19 Jun 2013 03:51:30 - 1.54
+++ Makefile16 Jun 2014 04:22:35 -
@@ -50,7 +50,7 @@ SRCS= tcpdump.c addrtoname.c privsep.c p
print-pfsync.c pf_print_state.c \
print-udpencap.c print-carp.c \
print-802_11.c print-iapp.c print-mpls.c print-slow.c \
- gmt2local.c savestr.c setsignal.c
+ gmt2local.c savestr.c setsignal.c in_cksum.c
# TCP OS Fingerprinting
.PATH: ${.CURDIR}/../../sys/net
Index: in_cksum.c
===
RCS file: in_cksum.c
diff -N in_cksum.c
--- /dev/null 1 Jan 1970 00:00:00 -
+++ in_cksum.c 16 Jun 2014 04:22:35 -
@@ -0,0 +1,88 @@
+/* $OpenBSD$ */
+
+/*
+ * Copyright (c) 1988, 1992, 1993
+ * The Regents of the University of California. All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ * 1. Redistributions of source code must retain the above copyright
+ *notice, this list of conditions and the following disclaimer.
+ * 2. Redistributions in binary form must reproduce the above copyright
+ *notice, this list of conditions and the following disclaimer in the
+ *documentation and/or other materials provided with the distribution.
+ * 3. Neither the name of the University nor the names of its contributors
+ *may be used to endorse or promote products derived from this software
+ *without specific prior written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE REGENTS AND CONTRIBUTORS ``AS IS'' AND
+ * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
+ * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
+ * ARE DISCLAIMED. IN NO EVENT SHALL THE REGENTS OR CONTRIBUTORS BE LIABLE
+ * FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
+ * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
Re: ftp(1) User-Agent
On Thu, Jun 12, 2014 at 06:32:00PM +0200, Alexander Hall wrote:
On June 11, 2014 6:18:19 AM CEST, Lawrence Teo l...@openbsd.org wrote:
This diff allows ftp(1) to change the User-Agent for HTTP(S) URL
requests via the FTPUSERAGENT environment variable (personally I prefer
HTTPUSERAGENT but FTPUSERAGENT is what's used by ftp(1) on other BSDs).
This is useful when fetching URLs that are sensitive to the User-Agent,
such as sites that send different content if you're using a mobile
browser/device.
I generally dislike passing options as environment variables. The use
case you describe does not really strike me as something I'd have in
my $ENV.
Does any of the other ftp's have a corresponding switch for it?
Thanks for your feedback. As Adam Thompson also mentioned in his reply,
the other ftp's don't have a corresponding switch for the User-Agent.
The closest switch is FreeBSD's fetch(1) which uses --user-agent, but I
don't think we want that. :)
I only chose the environment variable approach because it's what's used
by the other BSDs. But if a command-line option is preferred, I don't
mind it at all.
So here's an alternate implementation that uses -U to specify the
User-Agent. After testing it, I find that I prefer it over using the
environment variable... for starters, there's way less typing! :)
Index: fetch.c
===
RCS file: /cvs/src/usr.bin/ftp/fetch.c,v
retrieving revision 1.122
diff -u -p -r1.122 fetch.c
--- fetch.c 20 May 2014 01:25:23 - 1.122
+++ fetch.c 14 Jun 2014 02:45:41 -
@@ -893,10 +893,10 @@ again:
if (cookie)
ftp_printf(fin, ssl, GET %s HTTP/1.0\r\n
Proxy-Authorization: Basic %s%s\r\n%s\r\n\r\n,
- epath, cookie, buf ? buf : , HTTP_USER_AGENT);
+ epath, cookie, buf ? buf : , httpuseragent);
else
ftp_printf(fin, ssl, GET %s HTTP/1.0\r\n%s%s\r\n\r\n,
- epath, buf ? buf : , HTTP_USER_AGENT);
+ epath, buf ? buf : , httpuseragent);
} else {
#ifndef SMALL
@@ -954,7 +954,7 @@ again:
ftp_printf(fin, ssl, :%s, port);
#endif /* !SMALL */
ftp_printf(fin, ssl, \r\n%s%s\r\n\r\n,
- buf ? buf : , HTTP_USER_AGENT);
+ buf ? buf : , httpuseragent);
if (verbose)
fprintf(ttyout, \n);
}
@@ -1293,6 +1293,9 @@ auto_fetch(int argc, char *argv[], char
char *cp, *url, *host, *dir, *file, *portnum;
char *username, *pass, *pathstart;
char *ftpproxy, *httpproxy;
+#ifndef SMALL
+ char *uagent = NULL;
+#endif /* !SMALL */
int rval, xargc;
volatile int argpos;
int dirhasglob, filehasglob, oautologin;
@@ -1313,6 +1316,13 @@ auto_fetch(int argc, char *argv[], char
if ((httpproxy = getenv(HTTP_PROXY)) != NULL *httpproxy == '\0')
httpproxy = NULL;
+ if (httpuseragent == NULL)
+ httpuseragent = HTTP_USER_AGENT;
+#ifndef SMALL
+ else
+ uagent = httpuseragent;
+#endif /* !SMALL */
+
/*
* Loop through as long as there's files to fetch.
*/
@@ -1583,6 +1593,9 @@ bad_ftp_url:
}
if (connected rval != -1)
disconnect(0, NULL);
+#ifndef SMALL
+ free(uagent);
+#endif /* !SMALL */
return (rval);
}
Index: ftp.1
===
RCS file: /cvs/src/usr.bin/ftp/ftp.1,v
retrieving revision 1.91
diff -u -p -r1.91 ftp.1
--- ftp.1 23 Jan 2014 08:09:08 - 1.91
+++ ftp.1 14 Jun 2014 02:45:41 -
@@ -62,6 +62,7 @@
.Op Fl o Ar output
.Op Fl S Ar ssl_options
.Op Fl s Ar srcaddr
+.Op Fl U Ar useragent
.Sm off
.No http[s]:// Oo Ar user : password No @
.Oc Ar host Oo : Ar port
@@ -267,6 +268,11 @@ of the connection.
Only useful on systems with more than one address.
.It Fl t
Enables packet tracing.
+.It Fl U Ar useragent
+Set
+.Ar useragent
+as the User-Agent for HTTP(S) URL requests.
+If not specified, the default User-Agent is ``OpenBSD ftp''.
.It Fl V
Disable verbose mode, overriding the default of enabled when input
is from a terminal.
Index: ftp_var.h
===
RCS file: /cvs/src/usr.bin/ftp/ftp_var.h,v
retrieving revision 1.33
diff -u -p -r1.33 ftp_var.h
--- ftp_var.h 24 Dec 2013 13:00:59 - 1.33
+++ ftp_var.h 14 Jun 2014 02:45:41 -
@@ -181,6 +181,7 @@ char *httpport; /* port number to use
#ifndef SMALL
char *httpsport; /* port number to use for https connections */
#endif /* !SMALL */
+char *httpuseragent; /* user agent for http(s) connections */
char *gateport;/* port number to use
ftp(1) User-Agent
This diff allows ftp(1) to change the User-Agent for HTTP(S) URL
requests via the FTPUSERAGENT environment variable (personally I prefer
HTTPUSERAGENT but FTPUSERAGENT is what's used by ftp(1) on other BSDs).
This is useful when fetching URLs that are sensitive to the User-Agent,
such as sites that send different content if you're using a mobile
browser/device.
I have tested this on amd64 and i386, including a make build/release and
a bsd.rd http install with the resulting sets.
Comments? OK?
Index: fetch.c
===
RCS file: /cvs/src/usr.bin/ftp/fetch.c,v
retrieving revision 1.122
diff -u -p -u -p -r1.122 fetch.c
--- fetch.c 20 May 2014 01:25:23 - 1.122
+++ fetch.c 20 May 2014 02:47:40 -
@@ -893,10 +893,10 @@ again:
if (cookie)
ftp_printf(fin, ssl, GET %s HTTP/1.0\r\n
Proxy-Authorization: Basic %s%s\r\n%s\r\n\r\n,
- epath, cookie, buf ? buf : , HTTP_USER_AGENT);
+ epath, cookie, buf ? buf : , httpuseragent);
else
ftp_printf(fin, ssl, GET %s HTTP/1.0\r\n%s%s\r\n\r\n,
- epath, buf ? buf : , HTTP_USER_AGENT);
+ epath, buf ? buf : , httpuseragent);
} else {
#ifndef SMALL
@@ -954,7 +954,7 @@ again:
ftp_printf(fin, ssl, :%s, port);
#endif /* !SMALL */
ftp_printf(fin, ssl, \r\n%s%s\r\n\r\n,
- buf ? buf : , HTTP_USER_AGENT);
+ buf ? buf : , httpuseragent);
if (verbose)
fprintf(ttyout, \n);
}
@@ -1293,6 +1293,9 @@ auto_fetch(int argc, char *argv[], char
char *cp, *url, *host, *dir, *file, *portnum;
char *username, *pass, *pathstart;
char *ftpproxy, *httpproxy;
+#ifndef SMALL
+ char *uagent = NULL;
+#endif /* !SMALL */
int rval, xargc;
volatile int argpos;
int dirhasglob, filehasglob, oautologin;
@@ -1313,6 +1316,18 @@ auto_fetch(int argc, char *argv[], char
if ((httpproxy = getenv(HTTP_PROXY)) != NULL *httpproxy == '\0')
httpproxy = NULL;
+ httpuseragent = HTTP_USER_AGENT;
+#ifndef SMALL
+ if ((cp = getenv(FTPUSERAGENT)) != NULL *cp != '\0') {
+ /* Ensure that User-Agent value is in a single line. */
+ if (strcspn(cp, \r\n) != strlen(cp))
+ errx(1, Invalid FTPUSERAGENT: %s., cp);
+ if (asprintf(uagent, User-Agent: %s, cp) == -1)
+ errx(1, Can't allocate memory for HTTP(S) User-Agent);
+ httpuseragent = uagent;
+ }
+#endif /* !SMALL */
+
/*
* Loop through as long as there's files to fetch.
*/
@@ -1583,6 +1598,9 @@ bad_ftp_url:
}
if (connected rval != -1)
disconnect(0, NULL);
+#ifndef SMALL
+ free(uagent);
+#endif /* !SMALL */
return (rval);
}
@@ -1783,10 +1801,10 @@ proxy_connect(int socket, char *host, ch
if (cookie) {
l = asprintf(connstr, CONNECT %s:%s HTTP/1.1\r\n
Proxy-Authorization: Basic %s\r\n%s\r\n\r\n,
- host, port, cookie, HTTP_USER_AGENT);
+ host, port, cookie, httpuseragent);
} else {
l = asprintf(connstr, CONNECT %s:%s HTTP/1.1\r\n%s\r\n\r\n,
- host, port, HTTP_USER_AGENT);
+ host, port, httpuseragent);
}
if (l == -1)
Index: ftp.1
===
RCS file: /cvs/src/usr.bin/ftp/ftp.1,v
retrieving revision 1.91
diff -u -p -u -p -r1.91 ftp.1
--- ftp.1 23 Jan 2014 08:09:08 - 1.91
+++ ftp.1 24 Jan 2014 15:50:32 -
@@ -1705,6 +1705,10 @@ Default is port returned by a
.Fn getservbyname
lookup of
.Dq ftpgate/tcp .
+.It Ev FTPUSERAGENT
+User-Agent header to use for HTTP(S) URL requests.
+The default value is
+.Dq Ox ftp .
.It Ev HOME
For default location of a
.Pa .netrc
Index: ftp_var.h
===
RCS file: /cvs/src/usr.bin/ftp/ftp_var.h,v
retrieving revision 1.33
diff -u -p -u -p -r1.33 ftp_var.h
--- ftp_var.h 24 Dec 2013 13:00:59 - 1.33
+++ ftp_var.h 28 Dec 2013 04:24:15 -
@@ -181,6 +181,7 @@ char *httpport; /* port number to use
#ifndef SMALL
char *httpsport; /* port number to use for https connections */
#endif /* !SMALL */
+char *httpuseragent; /* user agent for http(s) connections */
char *gateport;/* port number to use for gateftp
connections */
jmp_buftoplevel; /* non-local goto stuff for cmd scanner
*/
improve divert(4) example program
This diff improves the example program in the divert(4) man page:
- Remove unnecessary includes
- bzero - memset
- Better sanity checks and return value checks
- Use the tcphdr struct instead of tcpiphdr so that the program will
work even if there are IP options
- Use more conventional variable names and buffer sizes
Also, I contributed this example section and program in 2012 and have
made other changes to the man page since then, and would like to add
myself to the copyright.
Comments/OK?
Index: divert.4
===
RCS file: /cvs/src/share/man/man4/divert.4,v
retrieving revision 1.13
diff -U5 -p -r1.13 divert.4
--- divert.42 Jun 2013 01:07:47 - 1.13
+++ divert.44 Jun 2014 03:59:01 -
@@ -1,8 +1,9 @@
.\ $OpenBSD: divert.4,v 1.13 2013/06/02 01:07:47 benno Exp $
.\
.\ Copyright (c) 2009 Michele Marchetto mich...@openbsd.org
+.\ Copyright (c) 2012-2014 Lawrence Teo l...@openbsd.org
.\
.\ Permission to use, copy, modify, and distribute this software for any
.\ purpose with or without fee is hereby granted, provided that the above
.\ copyright notice and this permission notice appear in all copies.
.\
@@ -116,16 +117,13 @@ apart from discarding invalid IP packets
#include sys/types.h
#include sys/socket.h
#include netinet/in.h
#include netinet/in_systm.h
#include netinet/ip.h
-#include netinet/ip_var.h
#include netinet/tcp.h
-#include netinet/tcpip.h
#include arpa/inet.h
#include stdio.h
-#include stdlib.h
#include string.h
#include err.h
#define DIVERT_PORT 700
@@ -138,11 +136,11 @@ main(int argc, char *argv[])
fd = socket(AF_INET, SOCK_RAW, IPPROTO_DIVERT);
if (fd == -1)
err(1, socket);
- bzero(sin, sizeof(sin));
+ memset(sin, 0, sizeof(sin));
sin.sin_family = AF_INET;
sin.sin_port = htons(DIVERT_PORT);
sin.sin_addr.s_addr = 0;
sin_len = sizeof(struct sockaddr_in);
@@ -151,46 +149,57 @@ main(int argc, char *argv[])
if (s == -1)
err(1, bind);
for (;;) {
ssize_t n;
- char packet[1];
- struct ip *ip_hdr;
- struct tcpiphdr *tcpip_hdr;
- char srcip[40], dstip[40];
+ char packet[IP_MAXPACKET];
+ struct ip *ip;
+ struct tcphdr *th;
+ int hlen;
+ char src[48], dst[48];
- bzero(packet, sizeof(packet));
+ memset(packet, 0, sizeof(packet));
n = recvfrom(fd, packet, sizeof(packet), 0,
(struct sockaddr *) sin, sin_len);
-
- tcpip_hdr = (struct tcpiphdr *) packet;
- ip_hdr = (struct ip *) packet;
-
- bzero(srcip, sizeof(srcip));
- bzero(dstip, sizeof(dstip));
-
- if (inet_ntop(AF_INET, ip_hdr-ip_src, srcip,
- sizeof(srcip)) == NULL) {
- fprintf(stderr, Invalid IPv4 source packet\en);
+ if (n == -1) {
+ warn(recvfrom);
+ continue;
+ }
+ if (n sizeof(struct ip)) {
+ warnx(packet is too short);
continue;
}
- if (inet_ntop(AF_INET, ip_hdr-ip_dst, dstip,
- sizeof(dstip)) == NULL) {
- fprintf(stderr, Invalid IPv4 destination
- packet\en);
+
+ ip = (struct ip *) packet;
+ hlen = ip-ip_hl 2;
+ if (hlen sizeof(struct ip) || ntohs(ip-ip_len) hlen ||
+ n ntohs(ip-ip_len)) {
+ warnx(invalid IPv4 packet);
continue;
}
+ th = (struct tcphdr *) (packet + hlen);
+
+ if (inet_ntop(AF_INET, ip-ip_src, src,
+ sizeof(src)) == NULL)
+ (void)strlcpy(src, ?, sizeof(src));
+
+ if (inet_ntop(AF_INET, ip-ip_dst, dst,
+ sizeof(dst)) == NULL)
+ (void)strlcpy(dst, ?, sizeof(dst));
+
printf(%s:%u - %s:%u\en,
- srcip,
- ntohs(tcpip_hdr-ti_sport),
- dstip,
- ntohs(tcpip_hdr-ti_dport)
+ src,
+ ntohs(th-th_sport),
+ dst,
+ ntohs(th-th_dport)
);
n = sendto(fd, packet, n, 0, (struct sockaddr *) sin,
sin_len);
+ if (n == -1)
+ warn(sendto);
}
return 0;
}
.Ed
Re: skip pflog interfaces in pcap_lookupdev()
On Thu, Aug 08, 2013 at 12:12:39AM -0500, joshua stein wrote: Running tcpdump with no -i arg finds a good interface to listen on by default on many machines, but on my laptop it finds pflog0 before urtwn0. Can we skip pflog interfaces like loopbacks? I got reminded of this diff while working on libpcap recently. The behavior you described annoys me as well, and I agree with your change. :) OK lteo@ I had a version that looked up the interface in the egress group but this is much simpler since pflog0 is usually the only other up interface that isn't important. Index: lib/libpcap/inet.c === RCS file: /cvs/src/lib/libpcap/inet.c,v retrieving revision 1.19 diff -u -p -u -p -r1.19 inet.c --- lib/libpcap/inet.c26 Mar 2006 20:58:50 - 1.19 +++ lib/libpcap/inet.c8 Aug 2013 05:10:19 - @@ -141,6 +141,8 @@ pcap_lookupdev(errbuf) continue; if (ISLOOPBACK(ifa-ifa_name, ifa-ifa_flags)) continue; + if (!strncmp(ifa-ifa_name, pflog, 5)) + continue; for (cp = ifa-ifa_name; !isdigit(*cp); ++cp) continue; n = atoi(cp);
libpcap: malloc+memset - calloc
This changes a few malloc()+memset() calls to calloc().
OK?
Index: gencode.c
===
RCS file: /cvs/src/lib/libpcap/gencode.c,v
retrieving revision 1.36
diff -u -p -r1.36 gencode.c
--- gencode.c 9 Oct 2010 08:14:36 - 1.36
+++ gencode.c 3 Feb 2014 21:05:40 -
@@ -191,11 +191,10 @@ newchunk(n)
if (k = NCHUNKS)
bpf_error(out of memory);
size = CHUNK0SIZE k;
- cp-m = (void *)malloc(size);
+ cp-m = (void *)calloc(1, size);
if (cp-m == NULL)
bpf_error(out of memory);
-
- memset((char *)cp-m, 0, size);
+
cp-n_left = size;
if (n size)
bpf_error(out of memory);
Index: pcap-bpf.c
===
RCS file: /cvs/src/lib/libpcap/pcap-bpf.c,v
retrieving revision 1.22
diff -u -p -r1.22 pcap-bpf.c
--- pcap-bpf.c 3 Dec 2013 00:25:34 - 1.22
+++ pcap-bpf.c 3 Feb 2014 21:03:10 -
@@ -895,13 +895,12 @@ pcap_create(const char *device, char *eb
{
pcap_t *p;
- p = malloc(sizeof(*p));
+ p = calloc(1, sizeof(*p));
if (p == NULL) {
snprintf(ebuf, PCAP_ERRBUF_SIZE, malloc: %s,
pcap_strerror(errno));
return (NULL);
}
- memset(p, 0, sizeof(*p));
p-fd = -1; /* not opened yet */
p-opt.source = strdup(device);
Index: pcap.c
===
RCS file: /cvs/src/lib/libpcap/pcap.c,v
retrieving revision 1.13
diff -u -p -r1.13 pcap.c
--- pcap.c 25 May 2012 01:58:08 - 1.13
+++ pcap.c 3 Feb 2014 21:07:35 -
@@ -612,10 +612,9 @@ pcap_open_dead(int linktype, int snaplen
{
pcap_t *p;
- p = malloc(sizeof(*p));
+ p = calloc(1, sizeof(*p));
if (p == NULL)
return NULL;
- memset (p, 0, sizeof(*p));
p-snapshot = snaplen;
p-linktype = linktype;
p-fd = -1;
Index: savefile.c
===
RCS file: /cvs/src/lib/libpcap/savefile.c,v
retrieving revision 1.10
diff -u -p -r1.10 savefile.c
--- savefile.c 25 May 2012 01:58:08 - 1.10
+++ savefile.c 3 Feb 2014 21:07:59 -
@@ -129,13 +129,12 @@ pcap_fopen_offline(FILE *fp, char *errbu
struct pcap_file_header hdr;
int linklen;
- p = (pcap_t *)malloc(sizeof(*p));
+ p = (pcap_t *)calloc(1, sizeof(*p));
if (p == NULL) {
strlcpy(errbuf, out of swap, PCAP_ERRBUF_SIZE);
return (NULL);
}
- memset((char *)p, 0, sizeof(*p));
/*
* Set this field so we don't double-close in pcap_close!
*/
Re: Trivial patch for ipv6
Committed, thanks!
On Sun, Mar 09, 2014 at 06:39:21PM +0100, Alexander Bluhm wrote:
OK bluhm@
On Sun, Mar 02, 2014 at 05:45:15AM -0800, Loganaden Velvindron wrote:
On Wed, Feb 12, 2014 at 09:11:41PM +0100, Alexander Bluhm wrote:
On Wed, Feb 12, 2014 at 10:10:36AM -0800, Loganaden Velvindron wrote:
Hi All,
based on a similar change from FreeBSD:
Change the return error from EACCES to EPERM as it is not a file.
According to errno(2) EACCES is for file access permissions, so
EPERM seems more apporiate.
A grep for EACCES and EPERM in netinet and netinet6 shows that both
are used often. Do we want to move towards EPERM for networking?
I think ip6_mrouter_set() and ip6_mrouter_get() should stay in sync,
so please make the diff for both functions.
bluhm
Please find attached the diff (tested on my OpenBSD laptop running -current
and a tunnel to HE):
Index: sys/netinet6/ip6_mroute.c
===
RCS file: /cvs/src/sys/netinet6/ip6_mroute.c,v
retrieving revision 1.67
diff -u -p -u -p -r1.67 ip6_mroute.c
--- sys/netinet6/ip6_mroute.c 11 Nov 2013 09:15:35 - 1.67
+++ sys/netinet6/ip6_mroute.c 2 Mar 2014 13:41:18 -
@@ -247,7 +247,7 @@ int
ip6_mrouter_set(int cmd, struct socket *so, struct mbuf *m)
{
if (cmd != MRT6_INIT so != ip6_mrouter)
- return (EACCES);
+ return (EPERM);
switch (cmd) {
case MRT6_INIT:
@@ -287,7 +287,8 @@ ip6_mrouter_set(int cmd, struct socket *
int
ip6_mrouter_get(int cmd, struct socket *so, struct mbuf **m)
{
- if (so != ip6_mrouter) return EACCES;
+ if (so != ip6_mrouter)
+ return (EPERM);
*m = m_get(M_WAIT, MT_SOOPTS);
Index: src/sys/netinet6/ip6_mroute.c
===
RCS file: /cvs/src/sys/netinet6/ip6_mroute.c,v
retrieving revision 1.67
diff -u -p -u -p -r1.67 ip6_mroute.c
--- src/sys/netinet6/ip6_mroute.c 11 Nov 2013 09:15:35 -
1.67
+++ src/sys/netinet6/ip6_mroute.c 12 Feb 2014 18:04:44 -
@@ -247,7 +247,7 @@ int
ip6_mrouter_set(int cmd, struct socket *so, struct mbuf *m)
{
if (cmd != MRT6_INIT so != ip6_mrouter)
- return (EACCES);
+ return (EPERM);
switch (cmd) {
case MRT6_INIT:
Re: remove pf_check_congestion()
On Fri, Mar 07, 2014 at 10:22:59AM -0700, Theo de Raadt wrote: * Ted Unangst t...@tedunangst.com [2014-03-07 07:40]: On Thu, Mar 06, 2014 at 23:56, Lawrence Teo wrote: pf_check_congestion() simply checks if ifq-ifq_congestion is non-zero, and returns 1 or 0 accordingly. It is only called by pf_test_rule(). Since what pf_check_congestion() does is very trivial and pf_test_rule() is its only user, would it make sense to remove it and let pf_test_rule() check ifq-ifq_congestion directly to save a function call? I think function calls are not so very expensive, and it makes it easier to read imo. exactly. making in static inline would be the max I'd find acceptable - but I'm certain you won't be able to demonstrate any performance benefit (previous profiling is pretty clear on that). I have checked my old mail. The original reason for making this a function is that we thought there might be other conditions, rather than just a variable. That might still happen in the future. Ah, makes sense now. Thanks for clarifying.
remove pf_check_congestion()
pf_check_congestion() simply checks if ifq-ifq_congestion is non-zero,
and returns 1 or 0 accordingly. It is only called by pf_test_rule().
Since what pf_check_congestion() does is very trivial and pf_test_rule()
is its only user, would it make sense to remove it and let
pf_test_rule() check ifq-ifq_congestion directly to save a function
call?
Index: pf.c
===
RCS file: /cvs/src/sys/net/pf.c,v
retrieving revision 1.868
diff -u -p -r1.868 pf.c
--- pf.c25 Jan 2014 03:39:00 - 1.868
+++ pf.c18 Feb 2014 17:57:42 -
@@ -230,7 +230,6 @@ int pf_compare_state_keys(struct pf_s
struct pf_state*pf_find_state(struct pfi_kif *,
struct pf_state_key_cmp *, u_int, struct mbuf *);
int pf_src_connlimit(struct pf_state **);
-int pf_check_congestion(struct ifqueue *);
int pf_match_rcvif(struct mbuf *, struct pf_rule *);
voidpf_step_into_anchor(int *, struct pf_ruleset **,
struct pf_rule **, struct pf_rule **);
@@ -3159,7 +3158,7 @@ pf_test_rule(struct pf_pdesc *pd, struct
ifq = ip6intrq;
#endif
- if (pd-dir == PF_IN pf_check_congestion(ifq)) {
+ if (pd-dir == PF_IN ifq-ifq_congestion) {
REASON_SET(reason, PFRES_CONGEST);
return (PF_DROP);
}
@@ -6745,15 +6744,6 @@ done:
}
return (action);
-}
-
-int
-pf_check_congestion(struct ifqueue *ifq)
-{
- if (ifq-ifq_congestion)
- return (1);
- else
- return (0);
}
void
pf_check_proto_cksum(): simplify ICMP checksum verification
This diff simplifies the verification of ICMP checksums in
pf_check_proto_cksum() by letting it use the same in4_cksum() call that
is used for TCP and UDP checksums.
As a bonus, since in4_cksum() doesn't need that m_data/m_len dance the
code becomes much shorter as well.
OK?
Index: pf.c
===
RCS file: /cvs/src/sys/net/pf.c,v
retrieving revision 1.865
diff -u -p -U10 -r1.865 pf.c
--- pf.c22 Jan 2014 04:33:34 - 1.865
+++ pf.c22 Jan 2014 22:33:23 -
@@ -5875,37 +5875,25 @@ pf_check_proto_cksum(struct pf_pdesc *pd
return (0);
}
if (pd-m-m_pkthdr.csum_flags flag_bad || off sizeof(struct ip) ||
pd-m-m_pkthdr.len off + len) {
pd-csum_status = PF_CSUM_BAD;
return (1);
}
switch (af) {
#ifdef INET
case AF_INET:
- if (p == IPPROTO_ICMP) {
- if (pd-m-m_len off) {
- pd-csum_status = PF_CSUM_BAD;
- return (1);
- }
- pd-m-m_data += off;
- pd-m-m_len -= off;
- sum = in_cksum(pd-m, len);
- pd-m-m_data -= off;
- pd-m-m_len += off;
- } else {
- if (pd-m-m_len sizeof(struct ip)) {
- pd-csum_status = PF_CSUM_BAD;
- return (1);
- }
- sum = in4_cksum(pd-m, p, off, len);
+ if (pd-m-m_len sizeof(struct ip)) {
+ pd-csum_status = PF_CSUM_BAD;
+ return (1);
}
+ sum = in4_cksum(pd-m, (p == IPPROTO_ICMP ? 0 : p), off, len);
break;
#endif /* INET */
#ifdef INET6
case AF_INET6:
if (pd-m-m_len sizeof(struct ip6_hdr)) {
pd-csum_status = PF_CSUM_BAD;
return (1);
}
sum = in6_cksum(pd-m, p, off, len);
break;
tcpdrop(8): check strdup() return values
This checks the return values of strdup() calls in tcpdrop(8).
OK?
Index: tcpdrop.c
===
RCS file: /cvs/src/usr.sbin/tcpdrop/tcpdrop.c,v
retrieving revision 1.12
diff -u -p -r1.12 tcpdrop.c
--- tcpdrop.c 24 Oct 2013 02:55:50 - 1.12
+++ tcpdrop.c 9 Jan 2014 22:50:43 -
@@ -54,6 +54,8 @@ main(int argc, char **argv)
if (argc == 3) {
laddr1 = addr1 = strdup(argv[1]);
+ if (!addr1)
+ err(1, strdup);
port1 = strrchr(addr1, ':');
if (port1)
*port1++ = '\0';
@@ -61,6 +63,8 @@ main(int argc, char **argv)
goto fail;
faddr2 = addr2 = strdup(argv[2]);
+ if (!addr2)
+ err(1, strdup);
port2 = strrchr(addr2, ':');
if (port2)
*port2++ = '\0';
@@ -84,11 +88,15 @@ fail:
if (addr1[0] == '[' addr1[strlen(addr1) - 1] == ']') {
laddr1 = strdup(addr1);
+ if (!laddr1)
+ err(1, strdup);
laddr1[strlen(laddr1) - 1] = '\0';
laddr1++;
}
if (addr2[0] == '[' addr2[strlen(addr2) - 1] == ']') {
faddr2 = strdup(addr2);
+ if (!faddr2)
+ err(1, strdup);
faddr2[strlen(faddr2) - 1] = '\0';
faddr2++;
}
tcpdump: detect bad ICMP checksums (1/2)
Our tcpdump has the ability to detect bad TCP and UDP checksums with the
-v flag, but not ICMP and ICMPv6 checksums. Here are two diffs to let
tcpdump detect bad ICMP/ICMPv6 checksums, which should help when
debugging issues caused by such checksums. :)
The first diff (below) is simply a mechanical change to make
icmp_print() accept the length variable, which is the length of the
packet without the IP header. This value is needed by tcpdump's
in_cksum() (used by the second diff) to determine if an ICMP checksum is
correct.
Related functions like {tcp,udp,icmp6}_print() already accept this
length variable, so this change will make icmp_print() consistent with
them as well.
This diff introduces no functional change.
The second diff does the actual ICMP/ICMPv6 checksum verification; I'll
send it in a separate mail.
Index: interface.h
===
RCS file: /cvs/src/usr.sbin/tcpdump/interface.h,v
retrieving revision 1.61
diff -u -p -U5 -r1.61 interface.h
--- interface.h 6 Apr 2010 16:01:57 - 1.61
+++ interface.h 1 Jan 2014 20:36:46 -
@@ -208,11 +208,11 @@ extern void ether_if_print(u_char *, con
const u_char *);
extern void fddi_if_print(u_char *, const struct pcap_pkthdr *, const u_char
*);
extern void ppp_ether_if_print(u_char *, const struct pcap_pkthdr *,
const u_char *);
extern void gre_print(const u_char *, u_int);
-extern void icmp_print(const u_char *, const u_char *);
+extern void icmp_print(const u_char *, u_int, const u_char *);
extern void ieee802_11_if_print(u_char *, const struct pcap_pkthdr *,
const u_char *);
extern void ieee802_11_radio_if_print(u_char *, const struct pcap_pkthdr *,
const u_char *);
extern void iapp_print(const u_char *, u_int);
Index: print-icmp.c
===
RCS file: /cvs/src/usr.sbin/tcpdump/print-icmp.c,v
retrieving revision 1.20
diff -u -p -U5 -r1.20 print-icmp.c
--- print-icmp.c27 Oct 2009 23:59:55 - 1.20
+++ print-icmp.c2 Jan 2014 04:41:04 -
@@ -163,11 +163,11 @@ struct id_rdiscovery {
u_int32_t ird_addr;
u_int32_t ird_pref;
};
void
-icmp_print(const u_char *bp, const u_char *bp2)
+icmp_print(const u_char *bp, u_int length, const u_char *bp2)
{
const struct icmp *dp;
const struct ip *ip;
const char *str, *fmt;
const struct ip *oip;
Index: print-ip.c
===
RCS file: /cvs/src/usr.sbin/tcpdump/print-ip.c,v
retrieving revision 1.36
diff -u -p -U5 -r1.36 print-ip.c
--- print-ip.c 12 Jan 2010 06:10:33 - 1.36
+++ print-ip.c 1 Jan 2014 20:37:01 -
@@ -425,11 +425,11 @@ ip_print(register const u_char *bp, regi
case IPPROTO_UDP:
udp_print(cp, len, (const u_char *)ip);
break;
case IPPROTO_ICMP:
- icmp_print(cp, (const u_char *)ip);
+ icmp_print(cp, len, (const u_char *)ip);
break;
#ifndef IPPROTO_IGRP
#define IPPROTO_IGRP 9
#endif
Index: print-ipsec.c
===
RCS file: /cvs/src/usr.sbin/tcpdump/print-ipsec.c,v
retrieving revision 1.17
diff -u -p -U5 -r1.17 print-ipsec.c
--- print-ipsec.c 3 May 2012 10:17:23 - 1.17
+++ print-ipsec.c 1 Jan 2014 20:37:50 -
@@ -185,11 +185,11 @@ esp_decrypt (const u_char *bp, u_int len
break;
case IPPROTO_IPV4:
ip_print(data, len);
break;
case IPPROTO_ICMP:
- icmp_print(data, bp2);
+ icmp_print(data, len, bp2);
break;
case IPPROTO_ICMPV6:
icmp6_print(data, len, bp2);
break;
default:
@@ -296,11 +296,12 @@ ah_print (register const u_char *bp, reg
case IPPROTO_IPIP: /* Tunnel Mode, IP-in-IP */
ip_print(bp + pl_len, len - pl_len);
break;
case IPPROTO_ICMP: /* From here and down; Transport mode */
- icmp_print(bp + pl_len, (const u_char *) ip);
+ icmp_print(bp + pl_len, len - pl_len,
+ (const u_char *) ip);
break;
case IPPROTO_ICMPV6:
icmp6_print(bp + pl_len, len - pl_len,
(const u_char *) ip);
Re: in[6]_proto_cksum_out: ICMP checksum fix
On Fri, Oct 18, 2013 at 03:27:09PM -0400, Lawrence Teo wrote:
Back in August I sent a diff to fix ICMP checksum calculation in
in_proto_cksum_out() and in_delayed_cksum() in cases where the ICMP
checksum field is not in the first mbuf of an mbuf chain (original post
at http://marc.info/?l=openbsd-techm=137571298511653w=2 ).
Thanks to henning's and bluhm's work on checksums at b2k13, the diff to
fix this is now much more straightforward than my earlier versions.
ICMPv6 is no longer affected by the bug, so this diff is for v4 only.
OK?
Index: ip_output.c
===
RCS file: /cvs/src/sys/netinet/ip_output.c,v
retrieving revision 1.249
diff -u -p -U9 -r1.249 ip_output.c
--- ip_output.c 20 Oct 2013 13:44:23 - 1.249
+++ ip_output.c 24 Oct 2013 16:29:38 -
@@ -2064,18 +2064,22 @@ in_delayed_cksum(struct mbuf *m)
switch (ip-ip_p) {
case IPPROTO_TCP:
offset += offsetof(struct tcphdr, th_sum);
break;
case IPPROTO_UDP:
offset += offsetof(struct udphdr, uh_sum);
break;
+ case IPPROTO_ICMP:
+ offset += offsetof(struct icmp, icmp_cksum);
+ break;
+
default:
return;
}
if ((offset + sizeof(u_int16_t)) m-m_len)
m_copyback(m, offset, sizeof(csum), csum, M_NOWAIT);
else
*(u_int16_t *)(mtod(m, caddr_t) + offset) = csum;
}
@@ -2109,20 +2113,13 @@ in_proto_cksum_out(struct mbuf *m, struc
m-m_pkthdr.csum_flags = ~M_TCP_CSUM_OUT; /* Clear */
}
} else if (m-m_pkthdr.csum_flags M_UDP_CSUM_OUT) {
if (!ifp || !(ifp-if_capabilities IFCAP_CSUM_UDPv4) ||
ifp-if_bridgeport != NULL) {
in_delayed_cksum(m);
m-m_pkthdr.csum_flags = ~M_UDP_CSUM_OUT; /* Clear */
}
} else if (m-m_pkthdr.csum_flags M_ICMP_CSUM_OUT) {
- struct ip *ip = mtod(m, struct ip *);
- int hlen;
- struct icmp *icp;
-
- hlen = ip-ip_hl 2;
- icp = (struct icmp *)(mtod(m, caddr_t) + hlen);
- icp-icmp_cksum = in4_cksum(m, 0, hlen,
- ntohs(ip-ip_len) - hlen);
+ in_delayed_cksum(m);
m-m_pkthdr.csum_flags = ~M_ICMP_CSUM_OUT; /* Clear */
}
}
mbuf(9) man page: document ICMP checksum flags
Now that the M_ICMP_CSUM_* flags are actually used in the kernel by PF (specifically pf_check_proto_cksum() and pf_cksum()), document them in the mbuf(9) man page. OK? Index: mbuf.9 === RCS file: /cvs/src/share/man/man9/mbuf.9,v retrieving revision 1.65 diff -u -p -r1.65 mbuf.9 --- mbuf.9 21 Aug 2013 05:21:42 - 1.65 +++ mbuf.9 24 Oct 2013 16:51:21 - @@ -313,6 +313,8 @@ IPv4 checksum needed. TCP checksum needed. .It Dv M_UDP_CSUM_OUT UDP checksum needed. +.It Dv M_ICMP_CSUM_OUT +ICMP/ICMPv6 checksum needed. .It Dv M_IPV4_CSUM_IN_OK IPv4 checksum verified. .It Dv M_IPV4_CSUM_IN_BAD @@ -325,6 +327,10 @@ TCP checksum bad. UDP checksum verified. .It Dv M_UDP_CSUM_IN_BAD UDP checksum bad. +.It Dv M_ICMP_CSUM_IN_OK +ICMP/ICMPv6 checksum verified. +.It Dv M_ICMP_CSUM_IN_BAD +ICMP/ICMPv6 checksum bad. .El .Pp When only M_EXT flag is set, an external storage buffer is being used to
Re: in[6]_proto_cksum_out: ICMP checksum fix
On Sat, Oct 19, 2013 at 04:43:07PM +0200, Alexander Bluhm wrote:
On Fri, Oct 18, 2013 at 03:27:09PM -0400, Lawrence Teo wrote:
Back in August I sent a diff to fix ICMP checksum calculation in
in_proto_cksum_out() and in_delayed_cksum() in cases where the ICMP
checksum field is not in the first mbuf of an mbuf chain (original post
at http://marc.info/?l=openbsd-techm=137571298511653w=2 ).
bluhm@ replied on tech@ with the following feedback:
On Fri, Aug 09, 2013 at 02:21:29AM +0200, Alexander Bluhm wrote:
On Mon, Aug 05, 2013 at 10:28:57AM -0400, Lawrence Teo wrote:
Index: ip_output.c
===
RCS file: /cvs/src/sys/netinet/ip_output.c,v
retrieving revision 1.244
diff -U5 -p -r1.244 ip_output.c
--- ip_output.c 31 Jul 2013 15:41:52 - 1.244
+++ ip_output.c 5 Aug 2013 02:44:20 -
@@ -2058,25 +2058,35 @@ ip_mloopback(struct ifnet *ifp, struct m
*/
void
in_delayed_cksum(struct mbuf *m)
{
struct ip *ip;
- u_int16_t csum, offset;
+ u_int16_t csum = 0, offset;
ip = mtod(m, struct ip *);
offset = ip-ip_hl 2;
+
+ if (ip-ip_p == IPPROTO_ICMP)
+ if (m_copyback(m, offset + offsetof(struct icmp,
icmp_cksum),
+ sizeof(csum), csum, M_NOWAIT))
+ return;
The code at the end of this function tries to avoid the m_copyback()
in the common case unless (offset + sizeof(u_int16_t)) m-m_len).
Do we want this optimization here?
bluhm
Here's my revised diff that preserves that optimization so that existing
behavior won't change in the common case where the protocol checksum
field is in the first mbuf.
The new diff also implements similar logic in in6_proto_cksum_out().
Comments/feedback appreciated. :)
Is there any good reason why you pass u_int8_t proto, u_int16_t
p_off to the in_delayed_cksum() now? What was wrong with the switch
in your previous diff? I think splitting the offset calculation
between in_delayed_cksum() and in_proto_cksum_out() doesn't make
it better.
The optimisation is fine now.
bluhm
Thank you for the feedback.
Passing u_int8_t proto to in_delayed_cksum() was inspired by
in6_proto_cksum_out(), which passes the protocol (nxt) to
in6_delayed_cksum(). This allows in6_delayed_cksum() to check if the
actual protocol matches the expected protocol at the beginning of the
function before even doing the delayed checksum calculation. It seems
like a good optimization so I borrowed the logic from there.
Passing in u_int16_t p_off takes advantage of the fact that an mbuf with
the M_TCP_CSUM_OUT flag represents a TCP packet (and M_UDP_CSUM_OUT
represents UDP, etc.), so we can pass in the protocol-specific checksum
offset to in_delayed_cksum() without having to figure out the offset
from the protocol again in in_delayed_cksum().
Having said all this, now that Henning's commit is in, I think there's
an opportunity to do things differently by setting the ICMP checksum
field to 0 somewhere in Henning's new block of code at the top of
in_proto_cksum_out(). I'll try to come up with a diff for discussion.
Thanks,
Lawrence
Index: netinet/in.h
===
RCS file: /cvs/src/sys/netinet/in.h,v
retrieving revision 1.97
diff -u -p -u -p -r1.97 in.h
--- netinet/in.h9 Oct 2013 09:33:43 - 1.97
+++ netinet/in.h16 Oct 2013 15:14:39 -
@@ -835,7 +835,7 @@ intin_broadcast(struct in_addr, stru
int in_canforward(struct in_addr);
int in_cksum(struct mbuf *, int);
int in4_cksum(struct mbuf *, u_int8_t, int, int);
-void in_delayed_cksum(struct mbuf *);
+void in_delayed_cksum(struct mbuf *, u_int8_t, u_int16_t);
int in_localaddr(struct in_addr, u_int);
void in_socktrim(struct sockaddr_in *);
char *inet_ntoa(struct in_addr);
Index: netinet/ip_output.c
===
RCS file: /cvs/src/sys/netinet/ip_output.c,v
retrieving revision 1.247
diff -u -p -u -p -r1.247 ip_output.c
--- netinet/ip_output.c 18 Oct 2013 09:04:03 - 1.247
+++ netinet/ip_output.c 18 Oct 2013 15:14:04 -
@@ -2050,34 +2050,35 @@ ip_mloopback(struct ifnet *ifp, struct m
* Process a delayed payload checksum calculation.
*/
void
-in_delayed_cksum(struct mbuf *m)
+in_delayed_cksum(struct mbuf *m, u_int8_t proto, u_int16_t p_off)
{
struct ip *ip;
- u_int16_t csum, offset;
+ u_int16_t csum = 0, hlen, *p = NULL;
ip = mtod(m, struct ip *);
- offset = ip-ip_hl 2;
- csum = in4_cksum(m, 0, offset, m-m_pkthdr.len - offset);
- if (csum == 0 ip-ip_p == IPPROTO_UDP)
- csum = 0x;
-
- switch
in[6]_proto_cksum_out: ICMP checksum fix
Back in August I sent a diff to fix ICMP checksum calculation in
in_proto_cksum_out() and in_delayed_cksum() in cases where the ICMP
checksum field is not in the first mbuf of an mbuf chain (original post
at http://marc.info/?l=openbsd-techm=137571298511653w=2 ).
bluhm@ replied on tech@ with the following feedback:
On Fri, Aug 09, 2013 at 02:21:29AM +0200, Alexander Bluhm wrote:
On Mon, Aug 05, 2013 at 10:28:57AM -0400, Lawrence Teo wrote:
Index: ip_output.c
===
RCS file: /cvs/src/sys/netinet/ip_output.c,v
retrieving revision 1.244
diff -U5 -p -r1.244 ip_output.c
--- ip_output.c 31 Jul 2013 15:41:52 - 1.244
+++ ip_output.c 5 Aug 2013 02:44:20 -
@@ -2058,25 +2058,35 @@ ip_mloopback(struct ifnet *ifp, struct m
*/
void
in_delayed_cksum(struct mbuf *m)
{
struct ip *ip;
- u_int16_t csum, offset;
+ u_int16_t csum = 0, offset;
ip = mtod(m, struct ip *);
offset = ip-ip_hl 2;
+
+ if (ip-ip_p == IPPROTO_ICMP)
+ if (m_copyback(m, offset + offsetof(struct icmp, icmp_cksum),
+ sizeof(csum), csum, M_NOWAIT))
+ return;
The code at the end of this function tries to avoid the m_copyback()
in the common case unless (offset + sizeof(u_int16_t)) m-m_len).
Do we want this optimization here?
bluhm
Here's my revised diff that preserves that optimization so that existing
behavior won't change in the common case where the protocol checksum
field is in the first mbuf.
The new diff also implements similar logic in in6_proto_cksum_out().
Comments/feedback appreciated. :)
Thanks,
Lawrence
Index: netinet/in.h
===
RCS file: /cvs/src/sys/netinet/in.h,v
retrieving revision 1.97
diff -u -p -u -p -r1.97 in.h
--- netinet/in.h9 Oct 2013 09:33:43 - 1.97
+++ netinet/in.h16 Oct 2013 15:14:39 -
@@ -835,7 +835,7 @@ intin_broadcast(struct in_addr, stru
int in_canforward(struct in_addr);
int in_cksum(struct mbuf *, int);
int in4_cksum(struct mbuf *, u_int8_t, int, int);
-void in_delayed_cksum(struct mbuf *);
+void in_delayed_cksum(struct mbuf *, u_int8_t, u_int16_t);
int in_localaddr(struct in_addr, u_int);
void in_socktrim(struct sockaddr_in *);
char *inet_ntoa(struct in_addr);
Index: netinet/ip_output.c
===
RCS file: /cvs/src/sys/netinet/ip_output.c,v
retrieving revision 1.247
diff -u -p -u -p -r1.247 ip_output.c
--- netinet/ip_output.c 18 Oct 2013 09:04:03 - 1.247
+++ netinet/ip_output.c 18 Oct 2013 15:14:04 -
@@ -2050,34 +2050,35 @@ ip_mloopback(struct ifnet *ifp, struct m
* Process a delayed payload checksum calculation.
*/
void
-in_delayed_cksum(struct mbuf *m)
+in_delayed_cksum(struct mbuf *m, u_int8_t proto, u_int16_t p_off)
{
struct ip *ip;
- u_int16_t csum, offset;
+ u_int16_t csum = 0, hlen, *p = NULL;
ip = mtod(m, struct ip *);
- offset = ip-ip_hl 2;
- csum = in4_cksum(m, 0, offset, m-m_pkthdr.len - offset);
- if (csum == 0 ip-ip_p == IPPROTO_UDP)
- csum = 0x;
-
- switch (ip-ip_p) {
- case IPPROTO_TCP:
- offset += offsetof(struct tcphdr, th_sum);
- break;
-
- case IPPROTO_UDP:
- offset += offsetof(struct udphdr, uh_sum);
- break;
-
- default:
+ if (ip-ip_p != proto)
return;
+
+ hlen = ip-ip_hl 2;
+ p_off += hlen;
+ if ((p_off + sizeof(u_int16_t)) = m-m_len)
+ p = (u_int16_t *)(mtod(m, caddr_t) + p_off);
+
+ if (proto == IPPROTO_ICMP) {
+ if (p)
+ *p = 0;
+ else if (m_copyback(m, p_off, sizeof(csum), csum, M_NOWAIT))
+ return;
}
- if ((offset + sizeof(u_int16_t)) m-m_len)
- m_copyback(m, offset, sizeof(csum), csum, M_NOWAIT);
+ csum = in4_cksum(m, 0, hlen, m-m_pkthdr.len - hlen);
+ if (csum == 0 proto == IPPROTO_UDP)
+ csum = 0x;
+
+ if (p)
+ *p = csum;
else
- *(u_int16_t *)(mtod(m, caddr_t) + offset) = csum;
+ m_copyback(m, p_off, sizeof(csum), csum, M_NOWAIT);
}
void
@@ -2086,25 +2087,20 @@ in_proto_cksum_out(struct mbuf *m, struc
if (m-m_pkthdr.csum_flags M_TCP_CSUM_OUT) {
if (!ifp || !(ifp-if_capabilities IFCAP_CSUM_TCPv4) ||
ifp-if_bridgeport != NULL) {
- in_delayed_cksum(m);
+ in_delayed_cksum(m, IPPROTO_TCP,
+ offsetof(struct tcphdr, th_sum));
m-m_pkthdr.csum_flags = ~M_TCP_CSUM_OUT; /* Clear */
}
} else if (m-m_pkthdr.csum_flags
pfctl: two fixes when storing a state file
When storing a state file (pfctl -S statefile), pfctl_state_store() returns without freeing the inbuf pointer. And if the state table is empty, it doesn't close the file before returning. This diff fixes both bugs. OK? Index: pfctl.c === RCS file: /cvs/src/sbin/pfctl/pfctl.c,v retrieving revision 1.317 diff -u -p -U4 -r1.317 pfctl.c --- pfctl.c 12 Aug 2013 17:42:08 - 1.317 +++ pfctl.c 7 Oct 2013 21:16:10 - @@ -1857,20 +1857,22 @@ pfctl_state_store(int dev, const char *f if (ps.ps_len + sizeof(struct pfioc_states) len) break; if (len == 0 ps.ps_len == 0) - return; + goto done; if (len == 0 ps.ps_len != 0) len = ps.ps_len; if (ps.ps_len == 0) - return; /* no states */ + goto done; /* no states */ len *= 2; } n = ps.ps_len / sizeof(struct pfsync_state); if (fwrite(inbuf, sizeof(struct pfsync_state), n, f) n) err(1, fwrite); +done: + free(inbuf); fclose(f); } void
pfctl: only allow once rules in anchors
This diff ensures that PF one shot rules can only be used inside anchors
and not in the main ruleset.
OK?
Index: sbin/pfctl/parse.y
===
RCS file: /cvs/src/sbin/pfctl/parse.y,v
retrieving revision 1.624
diff -u -p -u -p -r1.624 parse.y
--- sbin/pfctl/parse.y 1 Aug 2013 19:03:11 - 1.624
+++ sbin/pfctl/parse.y 20 Aug 2013 01:54:10 -
@@ -1703,8 +1703,14 @@ pfrule : action dir logquick interface
r.set_prio[1] = $8.set_prio[1];
r.scrub_flags |= PFSTATE_SETPRIO;
}
- if ($8.marker FOM_ONCE)
+ if ($8.marker FOM_ONCE) {
+ if (pf-asd == 0) {
+ yyerror('once' can only be used
+ inside anchors\n);
+ YYERROR;
+ }
r.rule_flag |= PFRULE_ONCE;
+ }
if ($8.marker FOM_AFTO)
r.rule_flag |= PFRULE_AFTO;
r.af = $5;
Index: share/man/man5/pf.conf.5
===
RCS file: /cvs/src/share/man/man5/pf.conf.5,v
retrieving revision 1.527
diff -u -p -u -p -r1.527 pf.conf.5
--- share/man/man5/pf.conf.525 Apr 2013 16:53:11 - 1.527
+++ share/man/man5/pf.conf.518 Aug 2013 19:13:23 -
@@ -611,6 +611,7 @@ directive occurs only at configuration f
.It Ar once
Creates a one shot rule that will remove itself from an active ruleset after
the first match.
+This parameter can only be used in an anchor.
In case this is the only rule in the anchor, the anchor will be destroyed
automatically after the rule is matched.
.Pp
Re: include netinet/in_var.h in arch/dev
On Wed, Aug 07, 2013 at 03:39:59AM +0200, Alexander Bluhm wrote: Hi, I have just removed a bunch of useless include netinet/in_var.h from the machine independent drivers. I suspect that they are also not needed in the architecture specific network drivers. Unfortunately I don't have any of these machines. So if you have access to one of macppc mvme68k octeon sgi sparc vax could you please test if this diff compiles. Compiles on vax-simh (Aug 1 vax snapshot, gcc 2.95.3).
in_proto_cksum_out: fix ICMP checksum calculation
in_proto_cksum_out() currently calculates ICMP checksums like this:
hlen = ip-ip_hl 2;
icp = (struct icmp *)(mtod(m, caddr_t) + hlen);
icp-icmp_cksum = 0;
icp-icmp_cksum = in4_cksum(m, 0, hlen,
ntohs(ip-ip_len) - hlen);
However this won't work if the ICMP header or ICMP checksum field is not
in the first mbuf of an mbuf chain.
The diff below fixes this as follows:
- Instead of fixing this in in_proto_cksum_out() itself, call
in_delayed_cksum() which also uses in4_cksum() and includes a check to
see if the checksum field is in the first mbuf. Modify
in_delayed_cksum() to recognize ICMP accordingly.
- In in_delayed_cksum(), set the ICMP checksum to 0 to prepare for
calculation (this makes it match what in6_delayed_cksum() does).
- While here, move the UDP zero checksum check to the switch statement
to remove the duplicate IPPROTO_UDP check (also to match
in6_delayed_cksum()).
OK?
[On a related note, in6_delayed_cksum() also makes an incorrect
assumption about the ICMPv6 header/checksum field being in the first
mbuf; I'll send the fix in a separate mail.]
Index: ip_output.c
===
RCS file: /cvs/src/sys/netinet/ip_output.c,v
retrieving revision 1.244
diff -U5 -p -r1.244 ip_output.c
--- ip_output.c 31 Jul 2013 15:41:52 - 1.244
+++ ip_output.c 5 Aug 2013 02:44:20 -
@@ -2058,25 +2058,35 @@ ip_mloopback(struct ifnet *ifp, struct m
*/
void
in_delayed_cksum(struct mbuf *m)
{
struct ip *ip;
- u_int16_t csum, offset;
+ u_int16_t csum = 0, offset;
ip = mtod(m, struct ip *);
offset = ip-ip_hl 2;
+
+ if (ip-ip_p == IPPROTO_ICMP)
+ if (m_copyback(m, offset + offsetof(struct icmp, icmp_cksum),
+ sizeof(csum), csum, M_NOWAIT))
+ return;
+
csum = in4_cksum(m, 0, offset, m-m_pkthdr.len - offset);
- if (csum == 0 ip-ip_p == IPPROTO_UDP)
- csum = 0x;
switch (ip-ip_p) {
case IPPROTO_TCP:
offset += offsetof(struct tcphdr, th_sum);
break;
case IPPROTO_UDP:
offset += offsetof(struct udphdr, uh_sum);
+ if (csum == 0)
+ csum = 0x;
+ break;
+
+ case IPPROTO_ICMP:
+ offset += offsetof(struct icmp, icmp_cksum);
break;
default:
return;
}
@@ -2101,17 +2111,9 @@ in_proto_cksum_out(struct mbuf *m, struc
ifp-if_bridgeport != NULL) {
in_delayed_cksum(m);
m-m_pkthdr.csum_flags = ~M_UDP_CSUM_OUT; /* Clear */
}
} else if (m-m_pkthdr.csum_flags M_ICMP_CSUM_OUT) {
- struct ip *ip = mtod(m, struct ip *);
- int hlen;
- struct icmp *icp;
-
- hlen = ip-ip_hl 2;
- icp = (struct icmp *)(mtod(m, caddr_t) + hlen);
- icp-icmp_cksum = 0;
- icp-icmp_cksum = in4_cksum(m, 0, hlen,
- ntohs(ip-ip_len) - hlen);
+ in_delayed_cksum(m);
m-m_pkthdr.csum_flags = ~M_ICMP_CSUM_OUT; /* Clear */
}
}
in6_delayed_cksum: fix ICMPv6 checksum calculation
in6_delayed_cksum() incorrectly assumes that the ICMPv6 header or
checksum field is in the first mbuf of an mbuf chain before setting it
to 0. This diff fixes it.
OK?
Index: ip6_output.c
===
RCS file: /cvs/src/sys/netinet6/ip6_output.c,v
retrieving revision 1.143
diff -U5 -p -r1.143 ip6_output.c
--- ip6_output.c31 Jul 2013 15:41:52 - 1.143
+++ ip6_output.c5 Aug 2013 02:44:43 -
@@ -3174,22 +3174,21 @@ ip6_randomid_init(void)
*/
void
in6_delayed_cksum(struct mbuf *m, u_int8_t nxt)
{
int nxtp, offset;
- u_int16_t csum;
+ u_int16_t csum = 0;
offset = ip6_lasthdr(m, 0, IPPROTO_IPV6, nxtp);
if (offset = 0 || nxtp != nxt)
/* If the desired next protocol isn't found, punt. */
return;
- if (nxt == IPPROTO_ICMPV6) {
- struct icmp6_hdr *icmp6;
- icmp6 = (struct icmp6_hdr *)(mtod(m, caddr_t) + offset);
- icmp6-icmp6_cksum = 0;
- }
+ if (nxt == IPPROTO_ICMPV6)
+ if (m_copyback(m, offset + offsetof(struct icmp6_hdr,
icmp6_cksum),
+ sizeof(csum), csum, M_NOWAIT))
+ return;
csum = (u_int16_t)(in6_cksum(m, nxt, offset, m-m_pkthdr.len - offset));
switch (nxt) {
case IPPROTO_TCP:
pf(4) man page: fix two errors
This diff fixes two errors on the pf(4) man page:
1. DIOCSETSTATUSIF has not used struct pfioc_if since pf_ioctl.c rev
1.234; it now uses struct pfioc_iface. Since the definition of
pfioc_iface is already listed under DIOCIGETIFACES, I moved the
description of DIOCSETSTATUSIF below DIOCIGETIFACES.
2. DIOCKILLSRCNODES uses struct pfioc_src_node_kill, not pfioc_iface.
Also add the struct definition for pfioc_src_node_kill while here.
OK?
Index: pf.4
===
RCS file: /cvs/src/share/man/man4/pf.4,v
retrieving revision 1.77
diff -u -p -r1.77 pf.4
--- pf.42 Jul 2013 05:57:37 - 1.77
+++ pf.42 Jul 2013 15:30:55 -
@@ -265,13 +265,6 @@ but ignores all fields of the
.Vt pfioc_state_kill
structure, except
.Va psk_ifname .
-.It Dv DIOCSETSTATUSIF Fa struct pfioc_if *pi
-Specify the interface for which statistics are accumulated.
-.Bd -literal
-struct pfioc_if {
- char ifname[IFNAMSIZ];
-};
-.Ed
.It Dv DIOCGETSTATUS Fa struct pf_status *s
Get the internal packet filter statistics.
.Bd -literal
@@ -986,6 +979,8 @@ struct pfi_kif {
TAILQ_HEAD(, pfi_dynaddr)pfik_dynaddrs;
};
.Ed
+.It Dv DIOCSETSTATUSIF Fa struct pfioc_iface *pi
+Specify the interface for which statistics are accumulated.
.It Dv DIOCSETIFFLAG Fa struct pfioc_iface *io
Set the user settable flags (described above) of the
.Nm
@@ -999,8 +994,16 @@ The filtering process is the same as for
Works as
.Dv DIOCSETIFFLAG
above but clears the flags.
-.It Dv DIOCKILLSRCNODES Fa struct pfioc_iface *io
+.It Dv DIOCKILLSRCNODES Fa struct pfioc_src_node_kill *psnk
Explicitly remove source tracking nodes.
+.Bd -literal
+struct pfioc_src_node_kill {
+ sa_family_t psnk_af;
+ struct pf_rule_addr psnk_src;
+ struct pf_rule_addr psnk_dst;
+ u_intpsnk_killed;
+};
+.Ed
.El
.Sh FILES
.Bl -tag -width /dev/pf -compact
pf(4) man page: two small fixes
Here are two small fixes to the pf(4) man page to make pfioc_natlook
and pfr_addr match net/pfvar.h.
OK?
Index: pf.4
===
RCS file: /cvs/src/share/man/man4/pf.4,v
retrieving revision 1.74
diff -u -p -r1.74 pf.4
--- pf.410 Feb 2012 00:08:20 - 1.74
+++ pf.41 Jun 2013 13:37:52 -
@@ -305,6 +305,7 @@ struct pfioc_natlook {
struct pf_addr rsaddr;
struct pf_addr rdaddr;
u_int16_trdomain;
+ u_int16_trrdomain;
u_int16_tsport;
u_int16_tdport;
u_int16_trsport;
@@ -580,6 +581,8 @@ struct pfr_addr {
struct in6_addr _pfra_ip6addr;
}pfra_u;
char pfra_ifname[IFNAMSIZ];
+ u_int32_tpfra_states;
+ u_int16_tpfra_weight;
u_int8_t pfra_af;
u_int8_t pfra_net;
u_int8_t pfra_not;
calculating ICMP checksums with in4_cksum()
Currently, ICMP(v4) checksums are calculated using in_cksum(), which
requires the following m_data/m_len dance:
hlen = ip-ip_hl 2;
m-m_data += hlen;
m-m_len -= hlen;
icp = mtod(m, struct icmp *);
icp-icmp_cksum = 0;
icp-icmp_cksum = in_cksum(m, ntohs(ip-ip_len) - hlen);
m-m_data -= hlen;
m-m_len += hlen;
blambert@ and I found that the ICMP checksum can be calculated using
in4_cksum() instead, which avoids that dance and shortens the code:
hlen = ip-ip_hl 2;
icp = (struct icmp *)(mtod(m, caddr_t) + hlen);
icp-icmp_cksum = 0;
icp-icmp_cksum = in4_cksum(m, 0, hlen, ntohs(ip-ip_len) - hlen);
Note that the implementation of in_cksum() and in4_cksum() may be
architecture-specific on various architectures.
I have tested this diff on amd64, hppa, i386, loongson, macppc, sgi, and
sparc64. abieber@ has also tested it independently on macppc.
Comments? OK?
Index: ip_icmp.c
===
RCS file: /cvs/src/sys/netinet/ip_icmp.c,v
retrieving revision 1.99
diff -u -p -U7 -p -r1.99 ip_icmp.c
--- ip_icmp.c 3 May 2013 09:35:20 - 1.99
+++ ip_icmp.c 21 May 2013 21:52:59 -
@@ -343,24 +343,20 @@ icmp_input(struct mbuf *m, ...)
}
i = hlen + min(icmplen, ICMP_ADVLENMIN);
if (m-m_len i (m = m_pullup(m, i)) == NULL) {
icmpstat.icps_tooshort++;
return;
}
ip = mtod(m, struct ip *);
- m-m_len -= hlen;
- m-m_data += hlen;
- icp = mtod(m, struct icmp *);
- if (in_cksum(m, icmplen)) {
+ if (in4_cksum(m, 0, hlen, icmplen)) {
icmpstat.icps_checksum++;
goto freeit;
}
- m-m_len += hlen;
- m-m_data -= hlen;
+ icp = (struct icmp *)(mtod(m, caddr_t) + hlen);
#ifdef ICMPPRINTFS
/*
* Message type specific processing.
*/
if (icmpprintfs)
printf(icmp_input, type %d code %d\n, icp-icmp_type,
icp-icmp_code);
@@ -803,21 +799,17 @@ void
icmp_send(struct mbuf *m, struct mbuf *opts)
{
struct ip *ip = mtod(m, struct ip *);
int hlen;
struct icmp *icp;
hlen = ip-ip_hl 2;
- m-m_data += hlen;
- m-m_len -= hlen;
- icp = mtod(m, struct icmp *);
+ icp = (struct icmp *)(mtod(m, caddr_t) + hlen);
icp-icmp_cksum = 0;
- icp-icmp_cksum = in_cksum(m, ntohs(ip-ip_len) - hlen);
- m-m_data -= hlen;
- m-m_len += hlen;
+ icp-icmp_cksum = in4_cksum(m, 0, hlen, ntohs(ip-ip_len) - hlen);
#ifdef ICMPPRINTFS
if (icmpprintfs) {
char buf[4 * sizeof(123)];
strlcpy(buf, inet_ntoa(ip-ip_dst), sizeof buf);
printf(icmp_send dst %s src %s\n,
buf, inet_ntoa(ip-ip_src));
Index: ip_output.c
===
RCS file: /cvs/src/sys/netinet/ip_output.c,v
retrieving revision 1.239
diff -u -p -U7 -p -r1.239 ip_output.c
--- ip_output.c 24 Apr 2013 12:34:15 - 1.239
+++ ip_output.c 21 May 2013 21:52:59 -
@@ -2122,17 +2122,13 @@ in_proto_cksum_out(struct mbuf *m, struc
}
} else if (m-m_pkthdr.csum_flags M_ICMP_CSUM_OUT) {
struct ip *ip = mtod(m, struct ip *);
int hlen;
struct icmp *icp;
hlen = ip-ip_hl 2;
- m-m_data += hlen;
- m-m_len -= hlen;
- icp = mtod(m, struct icmp *);
+ icp = (struct icmp *)(mtod(m, caddr_t) + hlen);
icp-icmp_cksum = 0;
- icp-icmp_cksum = in_cksum(m, ntohs(ip-ip_len) - hlen);
- m-m_data -= hlen;
- m-m_len += hlen;
+ icp-icmp_cksum = in4_cksum(m, 0, hlen, ntohs(ip-ip_len) -
hlen);
m-m_pkthdr.csum_flags = ~M_ICMP_CSUM_OUT; /* Clear */
}
}
options(4) man page: bump BUFCACHEPERCENT
BUFCACHEPERCENT has been set to 20 by default for a while now, so bump it in the options(4) man page accordingly. OK? Index: options.4 === RCS file: /cvs/src/share/man/man4/options.4,v retrieving revision 1.231 diff -u -p -r1.231 options.4 --- options.4 22 Mar 2013 16:40:24 - 1.231 +++ options.4 17 May 2013 15:23:48 - @@ -278,7 +278,7 @@ for details. .Bl -ohang .It Cd option BUFCACHEPERCENT= Ns Ar integer Percentage of RAM to use as a file system buffer. -It defaults to 5. +It defaults to 20. .It Cd option EXT2FS_SYSTEM_FLAGS This option changes the behavior of the APPEND and IMMUTABLE flags for a file on an
tcpdump(8) man page: sync PF reason codes
This diff syncs the PF reason codes on the tcpdump(8) man page with PFRES_NAMES in net/pfvar.h. OK? Index: tcpdump.8 === RCS file: /cvs/src/usr.sbin/tcpdump/tcpdump.8,v retrieving revision 1.79 diff -u -p -r1.79 tcpdump.8 --- tcpdump.8 26 Sep 2012 16:19:45 - 1.79 +++ tcpdump.8 30 Apr 2013 02:22:01 - @@ -735,8 +735,9 @@ The known codes are: .Ar state-insert , .Ar state-limit , .Ar src-limit , +.Ar synproxy , and -.Ar synproxy +.Ar translate (applies only to packets logged by .Xr pf 4 ) . .It Cm rset Ar name @@ -1066,8 +1067,12 @@ and packet length are printed. .Pp On the packet filter logging interface .Xr pflog 4 , -logging reason -.Pq rule match, bad-offset, fragment, bad-timestamp, short, normalize, memory , +the logging reason +.Po +rule match, bad-offset, fragment, short, normalize, memory, bad-timestamp, +congestion, ip-option, proto-cksum, state-mismatch, state-insert, +state-limit, src-limit, synproxy, translate +.Pc action taken .Pq pass/block , direction
divert(4) man page: document recent checksum changes
This diff documents my recent checksum recalculation changes for divert(4): http://marc.info/?l=openbsd-cvsm=136543514928571w=2 I have also made the text more specific to clarify that divert sockets are only applicable to PF rules with the divert-packet parameter, with an explicit note that it does not relate to divert-to. Comments? OK? Index: divert.4 === RCS file: /cvs/src/share/man/man4/divert.4,v retrieving revision 1.11 diff -u -p -r1.11 divert.4 --- divert.424 Oct 2012 22:57:41 - 1.11 +++ divert.418 Apr 2013 14:22:41 - @@ -43,10 +43,18 @@ and .Xr udp 4 . When .Xr pf 4 -processes a packet that matches a divert rule (see +processes a packet that matches a rule with the +.Ar divert-packet +parameter +(see .Xr pf.conf 5 for details) it is immediately sent to the divert socket listening on the -port specified in the rule. +divert port specified in the rule. +Note that +.Ar divert-packet +should not be confused with +.Ar divert-to , +which does not use divert sockets. .Xr pf 4 reassembles TCP streams by default (if IP reassembly is not disabled) before sending them to the divert sockets. @@ -63,8 +71,12 @@ kernel. After being reinjected, inbound and outbound packets are treated differently. Inbound packets are added to the relevant input queue and a soft interrupt is scheduled to signal that a new packet is ready to be processed; outbound ones -are processed directly by the relevant IP/IPv6 output function. -The packets' checksums are recalculated upon reinjection. +are processed directly by the relevant IPv4/IPv6 output function. +Since the userspace application could have modified the packets, upon +reinjection basic sanity checks are done to ensure that the packets are still +valid. +The packets' IPv4 and protocol checksums (TCP, UDP, ICMP, and ICMPv6) are also +recalculated. .Pp Writing to a divert socket can be achieved using .Xr sendto 2
ftp(1) man page: remove paragraph in BUGS
The ftp(1) man page has a paragraph in BUGS that warns against using ftp(1) with 4.2BSD servers. I doubt anyone would connect to an 4.2BSD server today, so is there any reason not to remove it? Lawrence Index: ftp.1 === RCS file: /cvs/src/usr.bin/ftp/ftp.1,v retrieving revision 1.87 diff -U4 -p -r1.87 ftp.1 --- ftp.1 19 Feb 2013 03:02:34 - 1.87 +++ ftp.1 19 Mar 2013 03:10:02 - @@ -1730,19 +1730,8 @@ command appeared in .Sh BUGS Correct execution of many commands depends upon proper behavior by the remote server. .Pp -An error in the treatment of carriage returns -in the -.Bx 4.2 -ASCII-mode transfer code -has been corrected. -This correction may result in incorrect transfers of binary files -to and from -.Bx 4.2 -servers using the ASCII type. -Avoid this problem by using the binary image type. -.Pp In the recursive mode of .Ic mget , files and directories starting with whitespace are ignored because the list cannot be parsed any other way.
Re: md5: returns 0 when a file does not exist
On Thu, Mar 28, 2013 at 03:34:50PM +0100, Patrik Lundin wrote:
I noticed that md5(1) returns 0 when you target a file that does not
exist:
=
$ md5 foobar
md5: cannot open foobar: No such file or directory
$ echo $?
0
=
This seems wrong according to the man page, and I have looked at the
FreeBSD and Linux equivalent tool which returns 1 in this case.
I have attempted to write a patch that changes this behaviour. My
limited testing has been successful at least, see diff below.
This makes sense to me. I have reviewed and tested Patrik's diff and it
works for me.
OK?
Index: md5.c
===
RCS file: /cvs/src/bin/md5/md5.c,v
retrieving revision 1.54
diff -u -r1.54 md5.c
--- md5.c 4 Dec 2012 02:38:51 - 1.54
+++ md5.c 28 Mar 2013 14:09:43 -
@@ -194,7 +194,7 @@
TAILQ_HEAD(hash_list, hash_function);
void digest_end(const struct hash_function *, void *, char *, size_t, int);
-void digest_file(const char *, struct hash_list *, int);
+int digest_file(const char *, struct hash_list *, int);
int digest_filelist(const char *, struct hash_function *);
void digest_print(const struct hash_function *, const char *, const char *);
void digest_printstr(const struct hash_function *, const char *, const char *);
@@ -386,10 +386,10 @@
error += digest_filelist(*argv++,
TAILQ_FIRST(hl));
} else if (pflag || argc == 0)
- digest_file(-, hl, pflag);
+ error = digest_file(-, hl, pflag);
else
while (argc--)
- digest_file(*argv++, hl, 0);
+ error += digest_file(*argv++, hl, 0);
return(error ? EXIT_FAILURE : EXIT_SUCCESS);
}
@@ -476,7 +476,7 @@
}
}
-void
+int
digest_file(const char *file, struct hash_list *hl, int echo)
{
struct hash_function *hf;
@@ -489,7 +489,7 @@
fp = stdin;
else if ((fp = fopen(file, r)) == NULL) {
warn(cannot open %s, file);
- return;
+ return(1);
}
if (echo)
@@ -510,7 +510,7 @@
warn(%s: read error, file);
if (fp != stdin)
fclose(fp);
- return;
+ return(1);
}
if (fp != stdin)
fclose(fp);
@@ -523,6 +523,7 @@
else
digest_print(hf, file, digest);
}
+ return(0);
}
/*
Re: mbuf(9) man page: update checksum flags
On Thu, Mar 21, 2013 at 04:13:35PM +, Christian Weisgerber wrote:
Lawrence Teo l...@openbsd.org wrote:
The checksum flags listed in the mbuf(9) man page do not match the ones
in mbuf.h. In addition, the m_pkthdr.csum variable name should be
m_pkthdr.csum_flags.
The following diff fixes both issues.
OK?
Okay, but while you're there, could you remove the /IPv4 from the
M_{TCP,UDP}_CSUM_IN_* descriptions? We also use this for TCP or
UDP over IPv6, e.g. on newer bge(4) chips.
I'm not sure there's a point in documenting the M_ICMP_CSUM_* flags.
They aren't actually implemented in the stack, few chipsets support
them, and I doubt there would be much benefit.
Thank you for your feedback. I have revised the diff to remove /IPv4
from the relevant descriptions, and it no longer includes the
M_ICMP_CSUM_* flags that I added earlier.
OK?
Index: mbuf.9
===
RCS file: /cvs/src/share/man/man9/mbuf.9,v
retrieving revision 1.59
diff -u -p -r1.59 mbuf.9
--- mbuf.9 3 Jan 2013 07:53:22 - 1.59
+++ mbuf.9 22 Mar 2013 01:37:06 -
@@ -302,28 +302,28 @@ buffer holding the data (size MHLEN).
.El
.Pp
The
-.Fa m_pkthdr.csum
+.Fa m_pkthdr.csum_flags
variable can take the following values:
.Pp
.Bl -tag -compact -offset indent -width XX
.It Dv M_IPV4_CSUM_OUT
IPv4 checksum needed.
-.It Dv M_TCPV4_CSUM_OUT
+.It Dv M_TCP_CSUM_OUT
TCP checksum needed.
-.It Dv M_UDPV4_CSUM_OUT
+.It Dv M_UDP_CSUM_OUT
UDP checksum needed.
.It Dv M_IPV4_CSUM_IN_OK
IPv4 checksum verified.
.It Dv M_IPV4_CSUM_IN_BAD
IPv4 checksum bad.
.It Dv M_TCP_CSUM_IN_OK
-TCP/IPv4 checksum verified.
+TCP checksum verified.
.It Dv M_TCP_CSUM_IN_BAD
-TCP/IPv4 checksum bad.
+TCP checksum bad.
.It Dv M_UDP_CSUM_IN_OK
-UDP/IPv4 checksum verified.
+UDP checksum verified.
.It Dv M_UDP_CSUM_IN_BAD
-UDP/IPv4 checksum bad.
+UDP checksum bad.
.El
.Pp
When only M_EXT flag is set, an external storage buffer is being used to
Re: PF divert(4) bugfix: recalculate checksums on packet reinjection
On Mon, Mar 04, 2013 at 11:37:32PM -0500, Lawrence Teo wrote:
Brief background: divert(4) sockets can be used to send packets to a
userspace program. The program can inspect the packets and decide to
either reinject them back into the kernel or drop them.
According to the divert(4) man page, The packets' checksums are
recalculated upon reinjection. This makes sense, because the userspace
program could have modified the packet.
However, in my tests, I found that the checksums are not actually
recalculated upon reinjection. I ran into this bug when trying to use
divert-packet PF rules together with nat-to and rdr-to, e.g.:
match out on em5 inet nat-to (em5:0)
pass out on em5 proto tcp to port 80 divert-packet port 700
pass in on em5 proto tcp to port 22 divert-packet port 700 rdr-to
192.168.30.8
With the above rules, inbound packets would drop and netstat -p ip -s
shows that bad header checksums consistently increments by one for
every inbound packet.
The diff below fixes this bug by making divert(4) recalculate the IP(v4)
and TCP/UDP/ICMP/ICMPv6 checksums of reinjected packets on both IPv4 and
IPv6 (done with a ton of help and feedback from blambert@ who reviewed
many versions of this diff, thank you!).
If you are using divert(4), could you please test this diff to ensure
that it does not break your setup? Note that this diff only applies to
divert-packet PF rules, not divert-to/divert-reply.
I am also looking for feedback from developers to review the approach
and code that was used to fix this bug.
This revised diff uses ip6_lasthdr() to walk the IPv6 header chain,
which makes the diff a lot shorter and simpler.
The IPv4 part remains unchanged from the original diff that I sent last
week.
Thank you to those who have tested so far; more tests and feedback
welcome!
Lawrence
Index: netinet/ip_divert.c
===
RCS file: /cvs/src/sys/netinet/ip_divert.c,v
retrieving revision 1.10
diff -u -p -r1.10 ip_divert.c
--- netinet/ip_divert.c 21 Oct 2012 13:06:03 - 1.10
+++ netinet/ip_divert.c 12 Mar 2013 03:50:15 -
@@ -37,6 +37,9 @@
#include netinet/ip_var.h
#include netinet/in_pcb.h
#include netinet/ip_divert.h
+#include netinet/tcp.h
+#include netinet/udp.h
+#include netinet/ip_icmp.h
struct inpcbtable divbtable;
struct divstat divstat;
@@ -83,8 +86,12 @@ divert_output(struct mbuf *m, ...)
struct sockaddr_in *sin;
struct socket *so;
struct ifaddr *ifa;
- int s, error = 0;
+ int s, error = 0, p_hdrlen = 0;
va_list ap;
+ struct ip *ip;
+ u_int16_t off, csum = 0;
+ u_int8_t nxt;
+ size_t p_off = 0;
va_start(ap, m);
inp = va_arg(ap, struct inpcb *);
@@ -102,15 +109,68 @@ divert_output(struct mbuf *m, ...)
sin = mtod(nam, struct sockaddr_in *);
so = inp-inp_socket;
+ /* Do basic sanity checks. */
+ if (m-m_pkthdr.len sizeof(struct ip))
+ goto fail;
+ if ((m = m_pullup(m, sizeof(struct ip))) == NULL) {
+ /* m_pullup() has freed the mbuf, so just return. */
+ divstat.divs_errors++;
+ return (ENOBUFS);
+ }
+ ip = mtod(m, struct ip *);
+ if (ip-ip_v != IPVERSION)
+ goto fail;
+ off = ip-ip_hl 2;
+ if (off sizeof(struct ip) || ntohs(ip-ip_len) off ||
+ m-m_pkthdr.len ntohs(ip-ip_len))
+ goto fail;
+
+ /*
+* Recalculate IP and protocol checksums since the userspace application
+* may have modified the packet prior to reinjection.
+*/
+ ip-ip_sum = 0;
+ ip-ip_sum = in_cksum(m, off);
+ nxt = ip-ip_p;
+ switch (ip-ip_p) {
+ case IPPROTO_TCP:
+ p_hdrlen = sizeof(struct tcphdr);
+ p_off = offsetof(struct tcphdr, th_sum);
+ break;
+ case IPPROTO_UDP:
+ p_hdrlen = sizeof(struct udphdr);
+ p_off = offsetof(struct udphdr, uh_sum);
+ break;
+ case IPPROTO_ICMP:
+ p_hdrlen = sizeof(struct icmp);
+ p_off = offsetof(struct icmp, icmp_cksum);
+ nxt = 0;
+ break;
+ default:
+ /* nothing */
+ break;
+ }
+ if (p_hdrlen) {
+ if (m-m_pkthdr.len off + p_hdrlen)
+ goto fail;
+
+ if ((error = m_copyback(m, off + p_off, sizeof(csum), csum,
M_NOWAIT)))
+ goto fail;
+ csum = in4_cksum(m, nxt, off, m-m_pkthdr.len - off);
+ if (ip-ip_p == IPPROTO_UDP csum == 0)
+ csum = 0x;
+ if ((error = m_copyback(m, off + p_off, sizeof(csum), csum,
M_NOWAIT)))
+ goto fail;
+ }
+
m-m_pkthdr.pf.flags |= PF_TAG_DIVERTED_PACKET
PF divert(4) bugfix: recalculate checksums on packet reinjection
Brief background: divert(4) sockets can be used to send packets to a
userspace program. The program can inspect the packets and decide to
either reinject them back into the kernel or drop them.
According to the divert(4) man page, The packets' checksums are
recalculated upon reinjection. This makes sense, because the userspace
program could have modified the packet.
However, in my tests, I found that the checksums are not actually
recalculated upon reinjection. I ran into this bug when trying to use
divert-packet PF rules together with nat-to and rdr-to, e.g.:
match out on em5 inet nat-to (em5:0)
pass out on em5 proto tcp to port 80 divert-packet port 700
pass in on em5 proto tcp to port 22 divert-packet port 700 rdr-to
192.168.30.8
With the above rules, inbound packets would drop and netstat -p ip -s
shows that bad header checksums consistently increments by one for
every inbound packet.
The diff below fixes this bug by making divert(4) recalculate the IP(v4)
and TCP/UDP/ICMP/ICMPv6 checksums of reinjected packets on both IPv4 and
IPv6 (done with a ton of help and feedback from blambert@ who reviewed
many versions of this diff, thank you!).
If you are using divert(4), could you please test this diff to ensure
that it does not break your setup? Note that this diff only applies to
divert-packet PF rules, not divert-to/divert-reply.
I am also looking for feedback from developers to review the approach
and code that was used to fix this bug.
Thank you,
Lawrence
Index: netinet/ip_divert.c
===
RCS file: /cvs/src/sys/netinet/ip_divert.c,v
retrieving revision 1.10
diff -u -p -r1.10 ip_divert.c
--- netinet/ip_divert.c 21 Oct 2012 13:06:03 - 1.10
+++ netinet/ip_divert.c 4 Mar 2013 16:14:33 -
@@ -37,6 +37,9 @@
#include netinet/ip_var.h
#include netinet/in_pcb.h
#include netinet/ip_divert.h
+#include netinet/tcp.h
+#include netinet/udp.h
+#include netinet/ip_icmp.h
struct inpcbtable divbtable;
struct divstat divstat;
@@ -83,8 +86,12 @@ divert_output(struct mbuf *m, ...)
struct sockaddr_in *sin;
struct socket *so;
struct ifaddr *ifa;
- int s, error = 0;
+ int s, error = 0, p_hdrlen = 0;
va_list ap;
+ struct ip *ip;
+ u_int16_t off, csum = 0;
+ u_int8_t nxt;
+ size_t p_off = 0;
va_start(ap, m);
inp = va_arg(ap, struct inpcb *);
@@ -102,15 +109,68 @@ divert_output(struct mbuf *m, ...)
sin = mtod(nam, struct sockaddr_in *);
so = inp-inp_socket;
+ /* Do basic sanity checks. */
+ if (m-m_pkthdr.len sizeof(struct ip))
+ goto fail;
+ if ((m = m_pullup(m, sizeof(struct ip))) == NULL) {
+ /* m_pullup() has freed the mbuf, so just return. */
+ divstat.divs_errors++;
+ return (ENOBUFS);
+ }
+ ip = mtod(m, struct ip *);
+ if (ip-ip_v != IPVERSION)
+ goto fail;
+ off = ip-ip_hl 2;
+ if (off sizeof(struct ip) || ntohs(ip-ip_len) off ||
+ m-m_pkthdr.len ntohs(ip-ip_len))
+ goto fail;
+
+ /*
+* Recalculate IP and protocol checksums since the userspace application
+* may have modified the packet prior to reinjection.
+*/
+ ip-ip_sum = 0;
+ ip-ip_sum = in_cksum(m, off);
+ nxt = ip-ip_p;
+ switch (ip-ip_p) {
+ case IPPROTO_TCP:
+ p_hdrlen = sizeof(struct tcphdr);
+ p_off = offsetof(struct tcphdr, th_sum);
+ break;
+ case IPPROTO_UDP:
+ p_hdrlen = sizeof(struct udphdr);
+ p_off = offsetof(struct udphdr, uh_sum);
+ break;
+ case IPPROTO_ICMP:
+ p_hdrlen = sizeof(struct icmp);
+ p_off = offsetof(struct icmp, icmp_cksum);
+ nxt = 0;
+ break;
+ default:
+ /* nothing */
+ break;
+ }
+ if (p_hdrlen) {
+ if (m-m_pkthdr.len off + p_hdrlen)
+ goto fail;
+
+ if ((error = m_copyback(m, off + p_off, sizeof(csum), csum,
M_NOWAIT)))
+ goto fail;
+ csum = in4_cksum(m, nxt, off, m-m_pkthdr.len - off);
+ if (ip-ip_p == IPPROTO_UDP csum == 0)
+ csum = 0x;
+ if ((error = m_copyback(m, off + p_off, sizeof(csum), csum,
M_NOWAIT)))
+ goto fail;
+ }
+
m-m_pkthdr.pf.flags |= PF_TAG_DIVERTED_PACKET;
if (sin-sin_addr.s_addr != INADDR_ANY) {
ipaddr.sin_addr = sin-sin_addr;
ifa = ifa_ifwithaddr(sintosa(ipaddr), m-m_pkthdr.rdomain);
if (ifa == NULL) {
- divstat.divs_errors++;
- m_freem(m);
- return (EADDRNOTAVAIL);
+ error =
dhclient(8): check strdup() return values in bind_lease()
The bind_lease() function has several strdup() calls for the domainname
and nameservers variables, but their return values are not checked.
In my tests, dhclient won't crash even if these strdup() calls return
NULL; however, if one of those variables become NULL as a result, the
search or nameserver line will be missing in /etc/resolv.conf. If
both variables are NULL, /etc/resolv.conf won't be updated at all. In
either case, the user won't know why this happened.
This diff makes the following changes to address this:
- In new_resolv_conf(), confirm that nameservers is not NULL and
actually has a value before attempting to use it. While not strictly
necessary, this makes the code consistent with the domainname check
right above it.
- In bind_lease(), error out if the strdup() calls fail. Also
initialize domainname and nameservers to NULL at the beginning of
the function; since new_resolv_conf() ensures that these variables
are not NULL before using them, initializing them to NULL helps us
avoid the need to do strdup().
I have tested this diff for almost a week and also with (U)pgrade.
Comments? OK?
Lawrence
Index: dhclient.c
===
RCS file: /cvs/src/sbin/dhclient/dhclient.c,v
retrieving revision 1.202
diff -u -p -r1.202 dhclient.c
--- dhclient.c 6 Jan 2013 15:33:12 - 1.202
+++ dhclient.c 12 Jan 2013 20:02:13 -
@@ -701,7 +701,7 @@ bind_lease(void)
struct in_addr gateway, mask;
struct option_data *options;
struct client_lease *lease;
- char *domainname, *nameservers;
+ char *domainname = NULL, *nameservers = NULL;
delete_addresses(ifi-name, ifi-rdomain);
flush_routes_and_arp_cache(ifi-rdomain);
@@ -713,17 +713,19 @@ bind_lease(void)
memcpy(mask.s_addr, options[DHO_SUBNET_MASK].data,
options[DHO_SUBNET_MASK].len);
- if (options[DHO_DOMAIN_NAME].len)
+ if (options[DHO_DOMAIN_NAME].len) {
domainname = strdup(pretty_print_option(
DHO_DOMAIN_NAME, options[DHO_DOMAIN_NAME], 0));
- else
- domainname = strdup();
+ if (domainname == NULL)
+ error(no memory for domainname);
+ }
if (options[DHO_DOMAIN_NAME_SERVERS].len) {
nameservers = strdup(pretty_print_option(
DHO_DOMAIN_NAME_SERVERS,
options[DHO_DOMAIN_NAME_SERVERS], 0));
- } else
- nameservers = strdup();
+ if (nameservers == NULL)
+ error(no memory for nameservers);
+ }
new_resolv_conf(ifi-name, domainname, nameservers);
@@ -1929,13 +1931,15 @@ new_resolv_conf(char *ifname, char *doma
strlcat(imsg.contents, \n, MAXRESOLVCONFSIZE);
}
- for (p = strsep(nameservers, ); p != NULL;
- p = strsep(nameservers, )) {
- if (*p == '\0')
- continue;
- strlcat(imsg.contents, nameserver , MAXRESOLVCONFSIZE);
- strlcat(imsg.contents, p, MAXRESOLVCONFSIZE);
- strlcat(imsg.contents, \n, MAXRESOLVCONFSIZE);
+ if (nameservers strlen(nameservers)) {
+ for (p = strsep(nameservers, ); p != NULL;
+ p = strsep(nameservers, )) {
+ if (*p == '\0')
+ continue;
+ strlcat(imsg.contents, nameserver ,
MAXRESOLVCONFSIZE);
+ strlcat(imsg.contents, p, MAXRESOLVCONFSIZE);
+ strlcat(imsg.contents, \n, MAXRESOLVCONFSIZE);
+ }
}
rslt = imsg_compose(unpriv_ibuf, IMSG_NEW_RESOLV_CONF, 0, 0, -1, imsg,
Re: dhclient(8): fix segfault if calloc()/strdup() return NULL
On Thu, Dec 13, 2012 at 12:07:42PM +0100, Joerg Zinke wrote:
Am 11.12.2012 um 04:12 schrieb Lawrence Teo l...@openbsd.org:
There are a number of calloc() and strdup() calls in the
apply_defaults() and clone_lease() functions whose return values are not
checked. If they happen to return NULL, dhclient will segfault.
This diff checks their return values and does some cleanup if they
return NULL. The diff also ensures that dhclient will not attempt to
make any changes (e.g. delete addresses or flush routes) if it
encounters these errors, but instead will try again at the next retry
interval.
Thoughts/OK?
Instead of checking each and every result:
What about just introducing xstrdup() and xcalloc() similar to
xmalloc.c in SSH? Or is recovering from OOM case considered
important here?
After discussing my original diff with krw@, I have committed a revised
version that errors out if those strdup() and calloc() calls fail, since
that is what the rest of the code does in almost all OOM cases.
I'm not opposed to introducing xstrdup() and xcalloc() to do the same
thing, though I think one advantage of the current method is that we
know exactly where the memory allocation fails since the error messages
are very specific, e.g. no memory for unpriv_ibuf vs. xcalloc: out of
memory (allocating 512 bytes). Those specific errors could be helpful
during troubleshooting.
But since I'm very new to dhclient's code, I would defer to krw@ and
other dhclient hackers to see which method is preferred. :)
Thanks,
Lawrence
Index: dhclient.c
===
RCS file: /cvs/src/sbin/dhclient/dhclient.c,v
retrieving revision 1.191
diff -u -p -r1.191 dhclient.c
--- dhclient.c 9 Dec 2012 20:28:03 - 1.191
+++ dhclient.c 10 Dec 2012 04:09:12 -
@@ -677,10 +677,17 @@ bind_lease(void)
struct client_lease *lease;
char *domainname, *nameservers;
+ lease = apply_defaults(client-new);
+ if (lease == NULL) {
+ client-state = S_INIT;
+ set_timeout_interval(config-retry_interval, state_init);
+ go_daemon();
+ return;
+ }
+
delete_addresses(ifi-name, ifi-rdomain);
flush_routes_and_arp_cache(ifi-rdomain);
- lease = apply_defaults(client-new);
options = lease-options;
memset(mask, 0, sizeof(mask));
@@ -1939,6 +1946,10 @@ apply_defaults(struct client_lease *leas
int i, j;
newlease = clone_lease(lease);
+ if (newlease == NULL) {
+ warning(Unable to clone lease);
+ return (NULL);
+ }
for (i = 0; i 256; i++) {
for (j = 0; j config-ignored_option_count; j++) {
@@ -1959,6 +1970,8 @@ apply_defaults(struct client_lease *leas
newlease-options[i].len = config-defaults[i].len;
newlease-options[i].data = calloc(1,
config-defaults[i].len);
+ if (newlease-options[i].data == NULL)
+ goto cleanup;
memcpy(newlease-options[i].data,
config-defaults[i].data, config-defaults[i].len);
break;
@@ -1970,6 +1983,8 @@ apply_defaults(struct client_lease *leas
lease-options[i].len;
newlease-options[i].data = calloc(1,
newlease-options[i].len);
+ if (newlease-options[i].data == NULL)
+ goto cleanup;
memcpy(newlease-options[i].data,
config-defaults[i].data, config-defaults[i].len);
memcpy(newlease-options[i].data +
@@ -1984,6 +1999,8 @@ apply_defaults(struct client_lease *leas
lease-options[i].len;
newlease-options[i].data = calloc(1,
newlease-options[i].len);
+ if (newlease-options[i].data == NULL)
+ goto cleanup;
memcpy(newlease-options[i].data,
lease-options[i].data, lease-options[i].len);
memcpy(newlease-options[i].data +
@@ -1998,6 +2015,8 @@ apply_defaults(struct client_lease *leas
config-defaults[i].len;
newlease-options[i].data = calloc(1,
config-defaults[i].len);
+ if (newlease-options[i].data == NULL)
+ goto cleanup;
memcpy(newlease-options[i].data,
config-defaults[i].data,
config-defaults[i].len);
@@ -2010,6 +2029,14 @@ apply_defaults(struct client_lease *leas
}
return (newlease);
+
+cleanup
tcpdump: ensure priv_pcap_live() returns NULL if no device
In the unlikely event that a NULL device is passed to the
priv_pcap_live() function, it will just set the error buffer and
continue, causing a segfault.
The diff below fixes this bug by ensuring that the function returns
NULL in this situation.
Comments/OK?
Lawrence
Index: privsep_pcap.c
===
RCS file: /cvs/src/usr.sbin/tcpdump/privsep_pcap.c,v
retrieving revision 1.16
diff -u -p -r1.16 privsep_pcap.c
--- privsep_pcap.c 21 Sep 2010 04:08:12 - 1.16
+++ privsep_pcap.c 6 Nov 2012 04:20:55 -
@@ -234,8 +234,10 @@ priv_pcap_live(const char *dev, int slen
if (priv_fd 0)
errx(1, %s: called from privileged portion, __func__);
- if (dev == NULL)
+ if (dev == NULL) {
snprintf(ebuf, PCAP_ERRBUF_SIZE, No interface specified);
+ return (NULL);
+ }
p = (pcap_t *)malloc(sizeof(*p));
if (p == NULL) {
pflogd(8): two if_exists() fixes
Quick background: In pflogd(8), the if_exists() function tests if a
given pflogX interface exists. It returns 1 (if it exists) or 0 (if
not).
This diff fixes two issues with if_exists():
1. if_exists() opens a socket to test the pflogX interface exists. If
the interface does not exist, the function will return without
closing that socket.
2. There is an incorrect call to if_exists() in the code:
if (!if_exists(interface) == -1) {
Comments/OK?
Lawrence
Index: pflogd.c
===
RCS file: /cvs/src/sbin/pflogd/pflogd.c,v
retrieving revision 1.48
diff -U4 -p -r1.48 pflogd.c
--- pflogd.c5 Mar 2012 11:50:16 - 1.48
+++ pflogd.c3 Nov 2012 18:31:43 -
@@ -202,9 +202,9 @@ set_pcap_filter(void)
int
if_exists(char *ifname)
{
- int s;
+ int s, ret = 1;
struct ifreq ifr;
struct if_data ifrdat;
if ((s = socket(AF_INET, SOCK_DGRAM, 0)) == -1)
@@ -214,13 +214,13 @@ if_exists(char *ifname)
sizeof(ifr.ifr_name))
errx(1, main ifr_name: strlcpy);
ifr.ifr_data = (caddr_t)ifrdat;
if (ioctl(s, SIOCGIFDATA, (caddr_t)ifr) == -1)
- return (0);
+ ret = 0;
if (close(s))
err(1, close);
- return (1);
+ return (ret);
}
int
init_pcap(void)
@@ -689,9 +689,9 @@ main(int argc, char **argv)
while (1) {
np = pcap_dispatch(hpcap, PCAP_NUM_PKTS,
phandler, (u_char *)dpcap);
if (np 0) {
- if (!if_exists(interface) == -1) {
+ if (!if_exists(interface)) {
logmsg(LOG_NOTICE, interface %s went away,
interface);
ret = -1;
break;
Move pcap-filter(3) man page to section 7
I think the current pcap-filter(3) man page belongs in section 7, since it does not describe library functions like the other man pages in section 3. It is more similar to the man pages in section 7. The mainline libpcap's source distribution will try to install the pcap-filter man page in section 7 as well. This diff moves the pcap-filter(3) man page to section 7 by doing the following things: * Move lib/libpcap/pcap-filter.3 to pcap-filter.7 and update its section number in .TH accordingly. * Update lib/libpcap/Makefile to install pcap-filter.7. * Update SEE ALSO in the pcap(3) man page to point at pcap-filter(7). * Add the description of pcap-filter(7) to the intro(7) man page. * Add pcap-filter(7) to tcpdump(8)'s SEE ALSO. Thoughts? Ok? Lawrence Index: lib/libpcap/Makefile === RCS file: /cvs/src/lib/libpcap/Makefile,v retrieving revision 1.23 diff -u -p -r1.23 Makefile --- lib/libpcap/Makefile2 Aug 2012 13:38:39 - 1.23 +++ lib/libpcap/Makefile31 Aug 2012 01:49:56 - @@ -2,7 +2,7 @@ # $NetBSD: Makefile,v 1.3 1996/05/10 21:54:24 cgd Exp $ LIB= pcap -MAN= pcap.3 pcap-filter.3 +MAN= pcap.3 pcap-filter.7 MLINKS=pcap.3 pcap_open_live.3 pcap.3 pcap_open_offline.3 \ pcap.3 pcap_dump_open.3 pcap.3 pcap_lookupdev.3 pcap.3 \ pcap_lookupnet.3 pcap.3 pcap_dispatch.3 pcap.3 pcap_loop.3 \ Index: lib/libpcap/pcap-filter.3 === RCS file: lib/libpcap/pcap-filter.3 diff -N lib/libpcap/pcap-filter.3 --- lib/libpcap/pcap-filter.3 16 Jul 2012 08:55:48 - 1.1 +++ /dev/null 1 Jan 1970 00:00:00 - @@ -1,732 +0,0 @@ -.\ $OpenBSD: pcap-filter.3,v 1.1 2012/07/16 08:55:48 giovanni Exp $ -.\ -.\ Copyright (c) 1987, 1988, 1989, 1990, 1991, 1992, 1994, 1995, 1996, 1997 -.\The Regents of the University of California. All rights reserved. -.\ All rights reserved. -.\ -.\ Redistribution and use in source and binary forms, with or without -.\ modification, are permitted provided that: (1) source code distributions -.\ retain the above copyright notice and this paragraph in its entirety, (2) -.\ distributions including binary code include the above copyright notice and -.\ this paragraph in its entirety in the documentation or other materials -.\ provided with the distribution, and (3) all advertising materials mentioning -.\ features or use of this software display the following acknowledgement: -.\ ``This product includes software developed by the University of California, -.\ Lawrence Berkeley Laboratory and its contributors.'' Neither the name of -.\ the University nor the names of its contributors may be used to endorse -.\ or promote products derived from this software without specific prior -.\ written permission. -.\ THIS SOFTWARE IS PROVIDED ``AS IS'' AND WITHOUT ANY EXPRESS OR IMPLIED -.\ WARRANTIES, INCLUDING, WITHOUT LIMITATION, THE IMPLIED WARRANTIES OF -.\ MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE. -.\ -.TH PCAP-FILTER 3 2008-01-06 -.SH NAME -pcap-filter \- packet filter syntax -.br -.ad -.SH DESCRIPTION -.LP -.B pcap_compile() -is used to compile a string into a filter program. -The resulting filter program can then be applied to -some stream of packets to determine which packets will be supplied to -.BR pcap_loop() , -.BR pcap_dispatch() , -.BR pcap_next() , -or -.BR pcap_next_ex() . -.LP -The \fIfilter expression\fP consists of one or more -.IR primitives . -Primitives usually consist of an -.I id -(name or number) preceded by one or more qualifiers. -There are three -different kinds of qualifier: -.IP \fItype\fP -qualifiers say what kind of thing the id name or number refers to. -Possible types are -.BR host , -.B net , -.B and port . -E.g., `host foo', `net 128.3', `port 20'. -If there is no type -qualifier, -.B host -is assumed. -.IP \fIdir\fP -qualifiers specify a particular transfer direction to and/or from -.IR id . -Possible directions are -.BR src , -.BR dst , -.BR src or dst , -.BR src and dst , -.BR ra , -.BR ta , -.BR addr1 , -.BR addr2 , -.BR addr3 , -and -.BR addr4 . -E.g., `src foo', `dst net 128.3', `src or dst port ftp-data'. -If -there is no dir qualifier, -.B src or dst -is assumed. -The -.BR ra , -.BR ta , -.BR addr1 , -.BR addr2 , -.BR addr3 , -and -.B addr4 -qualifiers are only valid for IEEE 802.11 Wireless LAN link layers. -For some link layers, such as SLIP and the ``cooked'' Linux capture mode -used for the ``any'' device and for some other device types, the -.B inbound -and -.B outbound -qualifiers can be used to specify a desired direction. -.IP \fIproto\fP -qualifiers restrict the match to a particular protocol. -Possible -protos are: -.BR ether , -.BR fddi , -.BR tr , -.BR wlan , -.BR ip , -.BR ip6 , -.BR arp , -.BR rarp , -.BR decnet , -.B tcp -and -.BR udp . -E.g., `ether src foo', `arp net 128.3', `tcp port 21' -`wlan addr2 0:2:3:4:5:6'. -If
Re: ftp(1) usage/man page HTTP Basic authentication tweaks
Here's a revised version of the diff that was done with a lot of discussion with and help from jmc@. This is what the diff does: * In both usage and the man page synopsis, combine the http and https usage formats into the following so that it is less verbose: ftp [-C] [-c cookie] [-o output] [-s srcaddr] http[s]://[user:password@]host[:port]/file ... * In the AUTO-FETCHING FILES section of the ftp(1) man page, mention that specifying user and password with HTTP and HTTPS URLs will log in using Basic authentication if http_proxy is not defined. * When compiled with -DSMALL, fix ftp(1) usage so that [user:password@] is not shown for http[s]. Looking for more ok's. Lawrence Index: ftp.1 === RCS file: /cvs/src/usr.bin/ftp/ftp.1,v retrieving revision 1.83 diff -u -p -r1.83 ftp.1 --- ftp.1 14 Aug 2012 20:47:08 - 1.83 +++ ftp.1 19 Aug 2012 18:22:21 - @@ -61,18 +61,7 @@ .Op Fl o Ar output .Op Fl s Ar srcaddr .Sm off -.No http:// Oo Ar user : password No @ -.Oc Ar host Oo : Ar port -.Oc No / Ar file -.Sm on -.Ar ... -.Nm ftp -.Op Fl C -.Op Fl c Ar cookie -.Op Fl o Ar output -.Op Fl s Ar srcaddr -.Sm off -.No https:// Oo Ar user : password No @ +.No http[s]:// Oo Ar user : password No @ .Oc Ar host Oo : Ar port .Oc No / Ar file .Sm on @@ -102,7 +91,7 @@ standard File Transfer Protocol (FTP). The program allows a user to transfer files to and from a remote network site. .Pp -The latter five usage formats will fetch a file using either the +The latter four usage formats will fetch a file using either the FTP, HTTP, or HTTPS protocols into the current directory. This is ideal for scripts. Refer to @@ -1272,11 +1261,14 @@ An FTP URL, retrieved using the FTP prot isn't defined. Otherwise, transfer using HTTP via the proxy defined in .Ev ftp_proxy . -If -.Ar user : Ns Ar password Ns @ -is given and +If a +.Ar user +and +.Ar password +are given and .Ev ftp_proxy -isn't defined, log in as +isn't defined, +log in as .Ar user with a password of .Ar password . @@ -1285,12 +1277,36 @@ An HTTP URL, retrieved using the HTTP pr If .Ev http_proxy is defined, it is used as a URL to an HTTP proxy server. +If a +.Ar user +and +.Ar password +are given and +.Ev http_proxy +isn't defined, +log in as +.Ar user +with a password of +.Ar password +using Basic authentication. .It https://[user:password@]host[:port]/file An HTTPS URL, retrieved using the HTTPS protocol. If .Ev http_proxy is defined, this HTTPS proxy server will be used to fetch the file using the CONNECT method. +If a +.Ar user +and +.Ar password +are given and +.Ev http_proxy +isn't defined, +log in as +.Ar user +with a password of +.Ar password +using Basic authentication. .It file:file .Ar file is retrieved from a mounted file system. Index: main.c === RCS file: /cvs/src/usr.bin/ftp/main.c,v retrieving revision 1.84 diff -u -p -r1.84 main.c --- main.c 14 Aug 2012 20:47:08 - 1.84 +++ main.c 19 Aug 2012 01:47:56 - @@ -778,13 +778,15 @@ usage(void) [-s srcaddr]\n #endif /* !SMALL */ - http://[user:password@]host[:port]/file ...\n + http #ifndef SMALL - %s [-C] [-c cookie] [-o output] [-s srcaddr]\n - - https://[user:password@]host[:port]/file\n; - ...\n -#endif /* !SMALL */ + [s] +#endif + :// +#ifndef SMALL + [user:password@] +#endif + host[:port]/file ...\n %s #ifndef SMALL [-C] @@ -804,8 +806,7 @@ usage(void) #endif /* !SMALL */ host:/file[/] ...\n, #ifndef SMALL - __progname, __progname, __progname, __progname, __progname, - __progname); + __progname, __progname, __progname, __progname, __progname); #else /* !SMALL */ __progname, __progname, __progname, __progname); #endif /* !SMALL */
tcpdump(8) man page: don't send bug reports to mainline
We maintain our own tcpdump, so remove the sentence in the BUGS section that asks users to send bug reports to mainline tcpdump. While here, fix the following mandoc -Tlint warnings: tcpdump.8:603:2: WARNING: skipping paragraph macro tcpdump.8:610:2: WARNING: skipping paragraph macro ok? Lawrence Index: tcpdump.8 === RCS file: /cvs/src/usr.sbin/tcpdump/tcpdump.8,v retrieving revision 1.76 diff -u -p -r1.76 tcpdump.8 --- tcpdump.8 10 Jul 2012 18:07:37 - 1.76 +++ tcpdump.8 16 Aug 2012 02:35:56 - @@ -600,14 +600,12 @@ True if the packet has a length less tha This is equivalent to: .Pp .D1 Cm len = Ar length -.Pp .It Cm greater Ar length True if the packet has a length greater than or equal to .Ar length . This is equivalent to: .Pp .D1 Cm len = Ar length -.Pp .It Cm ip proto Ar proto True if the packet is an IP packet (see .Xr ip 4 ) @@ -1883,11 +1881,6 @@ and .An Steven McCanne Aq mcca...@ee.lbl.gov , all of the Lawrence Berkeley Laboratory, University of California, Berkeley, CA. .Sh BUGS -Please send bug reports to -.Aq tcpd...@ee.lbl.gov -or -.Aq libp...@ee.lbl.gov . -.Pp Some attempt should be made to reassemble IP fragments, or at least to compute the right length for the higher level protocol. .Pp
ftp(1) usage/man page HTTP Basic authentication tweaks
This is a small follow-up diff to haesbaert@'s recent commit that enables HTTP Basic authentication in ftp(1): * In the AUTO-FETCHING FILES section of the ftp(1) man page, describe what [user:password@] does when used with HTTP and HTTPS URLs. * Fix usage formatting to match man page SYNOPSIS exactly. * Fix usage to hide [user:password@] for http/https when compiled with -DSMALL. Thoughts? OK? Index: ftp.1 === RCS file: /cvs/src/usr.bin/ftp/ftp.1,v retrieving revision 1.83 diff -u -p -r1.83 ftp.1 --- ftp.1 14 Aug 2012 20:47:08 - 1.83 +++ ftp.1 15 Aug 2012 02:07:33 - @@ -1285,12 +1285,32 @@ An HTTP URL, retrieved using the HTTP pr If .Ev http_proxy is defined, it is used as a URL to an HTTP proxy server. +If +.Ar user : Ns Ar password Ns @ +is given and +.Ev http_proxy +isn't defined, authenticate as +.Ar user +with a password of +.Ar password +to retrieve the URL using Basic authentication as defined in +RFC 2617. .It https://[user:password@]host[:port]/file An HTTPS URL, retrieved using the HTTPS protocol. If .Ev http_proxy is defined, this HTTPS proxy server will be used to fetch the file using the CONNECT method. +If +.Ar user : Ns Ar password Ns @ +is given and +.Ev http_proxy +isn't defined, authenticate as +.Ar user +with a password of +.Ar password +to retrieve the URL using Basic authentication as defined in +RFC 2617. .It file:file .Ar file is retrieved from a mounted file system. Index: main.c === RCS file: /cvs/src/usr.bin/ftp/main.c,v retrieving revision 1.84 diff -u -p -r1.84 main.c --- main.c 14 Aug 2012 20:47:08 - 1.84 +++ main.c 15 Aug 2012 01:51:07 - @@ -778,12 +778,15 @@ usage(void) [-s srcaddr]\n #endif /* !SMALL */ - http://[user:password@]host[:port]/file ...\n + http://; +#ifndef SMALL + [user:password@] +#endif + host[:port]/file ...\n #ifndef SMALL %s [-C] [-c cookie] [-o output] [-s srcaddr]\n - https://[user:password@]host[:port]/file\n; - ...\n + https://[user:password@]host[:port]/file ...\n #endif /* !SMALL */ %s #ifndef SMALL
Re: vmt ref vmwh
On Mon, Aug 13, 2012 at 09:07:52AM +0200, David Coppa wrote: On Sun, Aug 12, 2012 at 11:34 PM, Ingo Schwarze schwa...@usta.de wrote: Hi Ted, Ted Unangst wrote on Tue, Jul 24, 2012 at 08:57:59PM -0400: The vmwh package is very handy, but somewhat hard to discover. Can we add a little hint to the man page for vmt? I know very little about VMWare, but from the mdoc(7) perspective, the are no issues, except that i think the information would better fit into the SEE ALSO section, for example like shown below. I'd say, feel free to commit either version in the SEE ALSO section. Yours, Ingo Index: vmt.4 === RCS file: /cvs/src/share/man/man4/vmt.4,v retrieving revision 1.5 diff -u -r1.5 vmt.4 --- vmt.4 23 Oct 2011 11:42:38 - 1.5 +++ vmt.4 12 Aug 2012 21:28:13 - @@ -44,6 +44,9 @@ .Xr init 8 , .Xr sensorsd 8 , .Xr sysctl 8 +.Pp +A sysutils/vmwh port is available via +.Xr packages 7 . .Sh HISTORY The .Nm Index: vmt.4 === RCS file: /cvs/src/share/man/man4/vmt.4,v retrieving revision 1.5 diff -u -p -r1.5 vmt.4 --- vmt.4 23 Oct 2011 11:42:38 - 1.5 +++ vmt.4 24 Jul 2012 12:45:27 - @@ -40,6 +40,10 @@ It also provides access to the host mach .Pp .Nm reports the guest's hostname and first non-loopback IP address to the host. +.Pp +The +.Nm vmwh +package may also be of interest. .Sh SEE ALSO .Xr init 8 , .Xr sensorsd 8 , ok for me. Ciao, David OK for me as well; I prefer the version that spells out the port name as sysutils/vmwh Lawrence
Re: vmt ref vmwh
On Mon, Aug 13, 2012 at 04:06:28PM +0100, Stuart Henderson wrote: On 2012/08/13 10:51, Lawrence Teo wrote: OK for me as well; I prefer the version that spells out the port name as sysutils/vmwh We direct users towards packages not ports. Good point.. thank you, I need to remember that. vmwh works for me then. :)
Re: Mention RFC 6106 in rtadvd.conf(5) SEE ALSO section
On Sun, Jul 22, 2012 at 09:42:03PM -0400, Brad Smith wrote: Mention RFC 6106 in the rtadvd.conf(5) man pages SEE ALSO section. OK Index: rtadvd.conf.5 === RCS file: /home/cvs/src/usr.sbin/rtadvd/rtadvd.conf.5,v retrieving revision 1.28 diff -u -p -r1.28 rtadvd.conf.5 --- rtadvd.conf.5 8 Jul 2012 10:46:00 - 1.28 +++ rtadvd.conf.5 23 Jul 2012 01:27:20 - @@ -345,6 +345,12 @@ Thomas Narten, Erik Nordmark and W. A. S Neighbor Discovery for IP version 6 (IPv6) .Dc , RFC 2461 +.Pp +J. Jeong, S. Park, L. Beloeil, and S. Madanapalli, +.Do +IPv6 Router Advertisement Options for DNS Configuration +.Dc , +RFC 6106 .Sh HISTORY The .Xr rtadvd 8 -- This message has been scanned for viruses and dangerous content by MailScanner, and is believed to be clean.
Re: pfctl: make -P work with -ss
On Thu, May 31, 2012 at 12:07:18AM -0400, Lawrence Teo wrote:
pfctl's -P flag (introduced in OpenBSD 5.1) makes pfctl print ports
using their names in /etc/services. It was originally intended to be
used with -sr.
The diff extends it to make it work with -ss.
Example:
# pfctl -P -ss
all tcp 192.168.6.7:ssh (172.16.88.25:6688) - 172.16.88.22:49622
ESTABLISHED:ESTABLISHED
all udp 172.16.88.25:37076 - 8.8.8.8:domain MULTIPLE:SINGLE
all udp 172.16.88.25:18253 - 8.8.8.8:domain MULTIPLE:SINGLE
all udp 172.16.88.25:36447 - 8.8.8.8:domain MULTIPLE:SINGLE
all udp 172.16.88.25:16927 - 8.8.8.8:domain MULTIPLE:SINGLE
all tcp 172.16.88.25:4461 - 142.244.12.42:www FIN_WAIT_2:FIN_WAIT_2
all udp 172.16.88.25:21053 - 65.49.70.244:ntp MULTIPLE:MULTIPLE
all tcp 2001:470:e3b6:1:20c:29ff:fe9b:22f7[28976] - 2001:4860:800a::93[www]
FIN_WAIT_2:FIN_WAIT_2
all ipv6-icmp 2001:470:e3b6:1:20c:29ff:fe9b:22f7[135] -
2001:470:e3b6:1::ff[30569] 0:0
Here's a revised diff. I have verified that this new version does not
break tcpdump and make build. :)
BTW tcpdump's pf_print_state.c has diverged significantly from pfctl's,
so the change to tcpdump's pf_print_state.c is not exactly the same as
pfctl's.
Comments? ok?
Lawrence
Index: sbin/pfctl/pf_print_state.c
===
RCS file: /cvs/src/sbin/pfctl/pf_print_state.c,v
retrieving revision 1.61
diff -u -p -r1.61 pf_print_state.c
--- sbin/pfctl/pf_print_state.c 1 Jun 2012 08:35:45 - 1.61
+++ sbin/pfctl/pf_print_state.c 7 Jul 2012 17:23:35 -
@@ -166,8 +166,11 @@ print_name(struct pf_addr *addr, sa_fami
void
print_host(struct pf_addr *addr, u_int16_t port, sa_family_t af, u_int16_t
rdom,
-int opts)
+const char *proto, int opts)
{
+ struct servent *s = NULL;
+ charps[6];
+
if (rdom)
printf((%u) , ntohs(rdom));
@@ -188,10 +191,13 @@ print_host(struct pf_addr *addr, u_int16
}
if (port) {
+ snprintf(ps, sizeof(ps), %u, ntohs(port));
+ if (opts PF_OPT_PORTNAMES)
+ s = getservbyport(port, proto);
if (af == AF_INET)
- printf(:%u, ntohs(port));
+ printf(:%s, s ? s-s_name : ps);
else
- printf([%u], ntohs(port));
+ printf([%s], s ? s-s_name : ps);
}
}
@@ -212,6 +218,7 @@ print_state(struct pfsync_state *s, int
struct pfsync_state_peer *src, *dst;
struct pfsync_state_key *sk, *nk;
struct protoent *p;
+ char *pn = NULL;
int min, sec;
int afto = (s-key[PF_SK_STACK].af != s-key[PF_SK_WIRE].af);
int idx;
@@ -232,33 +239,34 @@ print_state(struct pfsync_state *s, int
sk-port[1] = nk-port[1];
}
printf(%s , s-ifname);
- if ((p = getprotobynumber(s-proto)) != NULL)
- printf(%s , p-p_name);
- else
+ if ((p = getprotobynumber(s-proto)) != NULL) {
+ pn = p-p_name;
+ printf(%s , pn);
+ } else
printf(%u , s-proto);
- print_host(nk-addr[1], nk-port[1], nk-af, nk-rdomain, opts);
+ print_host(nk-addr[1], nk-port[1], nk-af, nk-rdomain, pn, opts);
if (nk-af != sk-af || PF_ANEQ(nk-addr[1], sk-addr[1], nk-af) ||
nk-port[1] != sk-port[1] ||
nk-rdomain != sk-rdomain) {
idx = afto ? 0 : 1;
printf( ();
print_host(sk-addr[idx], sk-port[idx], sk-af,
- sk-rdomain, opts);
+ sk-rdomain, pn, opts);
printf());
}
if (s-direction == PF_OUT || (afto s-direction == PF_IN))
printf( - );
else
printf( - );
- print_host(nk-addr[0], nk-port[0], nk-af, nk-rdomain, opts);
+ print_host(nk-addr[0], nk-port[0], nk-af, nk-rdomain, pn, opts);
if (nk-af != sk-af || PF_ANEQ(nk-addr[0], sk-addr[0], nk-af) ||
nk-port[0] != sk-port[0] ||
nk-rdomain != sk-rdomain) {
idx = afto ? 1 : 0;
printf( ();
print_host(sk-addr[idx], sk-port[idx], sk-af,
- sk-rdomain, opts);
+ sk-rdomain, pn, opts);
printf());
}
Index: sbin/pfctl/pfctl.h
===
RCS file: /cvs/src/sbin/pfctl/pfctl.h,v
retrieving revision 1.49
diff -u -p -r1.49 pfctl.h
--- sbin/pfctl/pfctl.h 1 Jun 2012 08:35:45 - 1.49
+++ sbin/pfctl/pfctl.h 7 Jul 2012 17:23:35 -
@@ -106,7 +106,7 @@ struct pf_altq *pfaltq_lookup(const char
char *rate2str(double);
voidprint_addr(struct pf_addr_wrap *, sa_family_t, int);
-voidprint_host(struct pf_addr *, u_int16_t p, sa_family_t
Re: [s...@cd80.net: Re: rtadvd(8) patch 2/2 : finalize server-side RFC 6106 support]
On Sat, Jul 07, 2012 at 03:17:30PM +0200, Matthieu Herrb wrote:
On Sat, Jul 07, 2012 at 12:47:32PM +0200, Peter Hessler wrote:
ressurecting an old patch.
OK from me, anyone else?
With my sysadmin-deplying-IPv6-at-my-dayjob hat, I'd love to see that
go in, but I can't test it before next week.
I only had a quick glance at the code (never looked at rtadvd source
code before), didn't spot anything dubious...
I can't test the diff, but noticed this while looking through it:
+ while ((tmpsl = strsep(addr, ,))) {
+ struct dnssldom *dnsd;
+ ssize_t len;
+
+ len = strlen(tmpsl);
Should len be declared size_t instead of ssize_t?
wbsio: Add support for Winbond W83627UHG
This diff adds support for the Winbond W83627UHG chip, as found on the Lanner FW-7539 appliance. This diff was ported from DragonFly BSD: http://gitweb.dragonflybsd.org/dragonfly.git/commitdiff/245ec76bc1613b22cf282526fa9931e4c16e4237 Here is the sysctl hw.sensors output on that Lanner appliance before the diff: $ sysctl -n kern.version OpenBSD 5.2-beta (GENERIC.MP) #338: Tue Jun 26 22:52:57 MDT 2012 dera...@amd64.openbsd.org:/usr/src/sys/arch/amd64/compile/GENERIC.MP $ sysctl hw.sensors hw.sensors.cpu0.temp0=13.00 degC hw.sensors.cpu1.temp0=13.00 degC And here's the output after the diff is applied: $ sysctl -n kern.version OpenBSD 5.2-beta (GENERIC.MP) #2: Thu Jun 28 16:14:19 EDT 2012 l...@nori.my.domain:/usr/src/sys/arch/amd64/compile/GENERIC.MP $ sysctl hw.sensors hw.sensors.cpu0.temp0=13.00 degC hw.sensors.cpu1.temp0=13.00 degC hw.sensors.lm1.temp0=36.00 degC hw.sensors.lm1.temp1=29.00 degC hw.sensors.lm1.temp2=36.00 degC hw.sensors.lm1.fan1=4017 RPM hw.sensors.lm1.volt0=1.16 VDC (VCore) hw.sensors.lm1.volt1=4.96 VDC (+12V) hw.sensors.lm1.volt2=2.45 VDC (+3.3V) hw.sensors.lm1.volt3=2.40 VDC (+3.3V) hw.sensors.lm1.volt4=-8.79 VDC (-12V) hw.sensors.lm1.volt5=0.74 VDC hw.sensors.lm1.volt6=0.05 VDC hw.sensors.lm1.volt7=2.45 VDC (3.3VSB) hw.sensors.lm1.volt8=0.79 VDC (VBAT) $ dmesg | grep W83627 wbsio0 at isa0 port 0x2e/2: W83627UHG rev 0x32 lm1 at wbsio0 port 0xa10/8: W83627DHG Full dmesg at http://lteo.net/stuff/fw7539-dmesg.txt I am very new to this part of the code, and would really appreciate a review of the diff to make sure I did not miss anything. Comments and feedback welcome too. Thank you! Lawrence Index: share/man/man4/lm.4 === RCS file: /cvs/src/share/man/man4/lm.4,v retrieving revision 1.23 diff -u -p -r1.23 lm.4 --- share/man/man4/lm.4 7 Dec 2011 14:47:19 - 1.23 +++ share/man/man4/lm.4 28 Jun 2012 21:02:04 - @@ -86,7 +86,7 @@ Nuvoton NCT6776F .It Winbond W83627HF, W83627THF, W83637HF and W83697HF .It -Winbond W83627DHG and W83627EHF +Winbond W83627DHG, W83627UHG and W83627EHF .It Winbond W83781D, W83782D and W83783S .It Index: sys/dev/isa/wbsio.c === RCS file: /cvs/src/sys/dev/isa/wbsio.c,v retrieving revision 1.7 diff -u -p -r1.7 wbsio.c --- sys/dev/isa/wbsio.c 6 Dec 2011 16:06:07 - 1.7 +++ sys/dev/isa/wbsio.c 28 Jun 2012 20:10:43 - @@ -108,6 +108,7 @@ wbsio_probe(struct device *parent, void case WBSIO_ID_W83627EHF: case WBSIO_ID_W83627DHG: case WBSIO_ID_W83627DHGP: + case WBSIO_ID_W83627UHG: case WBSIO_ID_W83637HF: case WBSIO_ID_W83697HF: case WBSIO_ID_NCT6776F: @@ -159,6 +160,9 @@ wbsio_attach(struct device *parent, stru break; case WBSIO_ID_W83627DHGP: printf(: W83627DHG-P); + break; + case WBSIO_ID_W83627UHG: + printf(: W83627UHG); break; case WBSIO_ID_W83637HF: printf(: W83637HF); Index: sys/dev/isa/wbsioreg.h === RCS file: /cvs/src/sys/dev/isa/wbsioreg.h,v retrieving revision 1.2 diff -u -p -r1.2 wbsioreg.h --- sys/dev/isa/wbsioreg.h 7 Dec 2011 12:24:17 - 1.2 +++ sys/dev/isa/wbsioreg.h 28 Jun 2012 20:11:01 - @@ -38,6 +38,7 @@ #define WBSIO_ID_W83627EHF 0x88 #define WBSIO_ID_W83627DHG 0xa0 #define WBSIO_ID_W83627DHGP0xb0 +#define WBSIO_ID_W83627UHG 0xa2 #define WBSIO_ID_W83627SF 0x59 #define WBSIO_ID_W83637HF 0x70 #define WBSIO_ID_W83697HF 0x60
ssl(8) man page: miscellaneous fixes
This diff fixes a few grammar/punctuation issues and missing words on the ssl(8) man page. I have also added some text to make it flow better. ok? Index: ssl.8 === RCS file: /cvs/src/share/man/man8/ssl.8,v retrieving revision 1.50 diff -u -p -r1.50 ssl.8 --- ssl.8 27 May 2012 07:04:52 - 1.50 +++ ssl.8 15 Jun 2012 04:05:34 - @@ -66,7 +66,7 @@ device does not exist or is not readable This is most commonly seen by users as the .Ar RSA routines failing in applications such as -.Xr ssh 1 , +.Xr ssh 1 and .Xr httpd 8 . .Pp @@ -109,12 +109,20 @@ transactions in you will need to generate an .Ar RSA certificate. +The first step is to generate a key using a command like the following: .Bd -literal -offset indent # openssl genrsa -out /etc/ssl/private/server.key 2048 .Ed .Pp -Or, if you wish the key to be encrypted with a passphrase that you will -have to type in when starting servers +This command would generate a +2048 bit +.Ar RSA +key and save it to the file +file +.Pa /etc/ssl/private/server.key . +.Pp +Alternatively, if you wish the key to be encrypted with a passphrase +that you will have to type in when starting servers, use the command: .Bd -literal -offset indent # openssl genrsa -aes256 -out /etc/ssl/private/server.key 2048 .Ed @@ -166,7 +174,7 @@ you can switch to using the new certific .Pa /etc/ssl/server.crt with the certificate signed by your Certificate Authority, and then restarting -.Xr httpd 8 +.Xr httpd 8 . .Sh GENERATING DSA SERVER CERTIFICATES Generating a .Ar DSA @@ -178,7 +186,7 @@ parameter set with a command like the fo # openssl dsaparam 1024 -out dsa1024.pem .Ed .Pp -Would generate +This command would generate .Ar DSA parameters for 1024 bit .Ar DSA @@ -189,7 +197,7 @@ file Once you have the .Ar DSA parameters generated, you can generate a certificate -and unencrypted private key using the command: +and an unencrypted private key using the command: .Bd -literal -offset indent # openssl req -x509 -nodes -newkey dsa:dsa1024.pem \e -out /etc/ssl/dsacert.pem -keyout /etc/ssl/private/dsakey.pem
nginx.conf SSL file locations
This diff modifies the default ssl_certificate and ssl_certificate_key
values in nginx.conf to match the ssl(8) man page.
ok?
Index: nginx.conf
===
RCS file: /cvs/src/usr.sbin/nginx/conf/nginx.conf,v
retrieving revision 1.3
diff -u -p -r1.3 nginx.conf
--- nginx.conf 15 Mar 2012 10:11:37 - 1.3
+++ nginx.conf 15 Jun 2012 04:15:10 -
@@ -100,8 +100,8 @@ http {
#server_name localhost;
#ssl on;
-#ssl_certificate cert.pem;
-#ssl_certificate_key cert.key;
+#ssl_certificate /etc/ssl/server.crt;
+#ssl_certificate_key /etc/ssl/private/server.key;
#ssl_session_timeout 5m;
pfctl: make -P work with -ss
pfctl's -P flag (introduced in OpenBSD 5.1) makes pfctl print ports
using their names in /etc/services. It was originally intended to be
used with -sr.
The diff extends it to make it work with -ss.
Example:
# pfctl -P -ss
all tcp 192.168.6.7:ssh (172.16.88.25:6688) - 172.16.88.22:49622
ESTABLISHED:ESTABLISHED
all udp 172.16.88.25:37076 - 8.8.8.8:domain MULTIPLE:SINGLE
all udp 172.16.88.25:18253 - 8.8.8.8:domain MULTIPLE:SINGLE
all udp 172.16.88.25:36447 - 8.8.8.8:domain MULTIPLE:SINGLE
all udp 172.16.88.25:16927 - 8.8.8.8:domain MULTIPLE:SINGLE
all tcp 172.16.88.25:4461 - 142.244.12.42:www FIN_WAIT_2:FIN_WAIT_2
all udp 172.16.88.25:21053 - 65.49.70.244:ntp MULTIPLE:MULTIPLE
all tcp 2001:470:e3b6:1:20c:29ff:fe9b:22f7[28976] - 2001:4860:800a::93[www]
FIN_WAIT_2:FIN_WAIT_2
all ipv6-icmp 2001:470:e3b6:1:20c:29ff:fe9b:22f7[135] -
2001:470:e3b6:1::ff[30569] 0:0
Comments? Ok?
Lawrence
Index: pf_print_state.c
===
RCS file: /cvs/src/sbin/pfctl/pf_print_state.c,v
retrieving revision 1.59
diff -u -p -r1.59 pf_print_state.c
--- pf_print_state.c13 Oct 2011 18:30:54 - 1.59
+++ pf_print_state.c31 May 2012 03:10:29 -
@@ -166,8 +166,11 @@ print_name(struct pf_addr *addr, sa_fami
void
print_host(struct pf_addr *addr, u_int16_t port, sa_family_t af, u_int16_t
rdom,
-int opts)
+const char *proto, int opts)
{
+ struct servent *s = NULL;
+ charps[6];
+
if (rdom)
printf((%u) , ntohs(rdom));
@@ -188,10 +191,13 @@ print_host(struct pf_addr *addr, u_int16
}
if (port) {
+ snprintf(ps, sizeof(ps), %u, ntohs(port));
+ if (opts PF_OPT_PORTNAMES)
+ s = getservbyport(port, proto);
if (af == AF_INET)
- printf(:%u, ntohs(port));
+ printf(:%s, s ? s-s_name : ps);
else
- printf([%u], ntohs(port));
+ printf([%s], s ? s-s_name : ps);
}
}
@@ -212,6 +218,7 @@ print_state(struct pfsync_state *s, int
struct pfsync_state_peer *src, *dst;
struct pfsync_state_key *sk, *nk;
struct protoent *p;
+ char *pn = NULL;
int min, sec;
int afto = (s-key[PF_SK_STACK].af != s-key[PF_SK_WIRE].af);
int idx;
@@ -232,33 +239,34 @@ print_state(struct pfsync_state *s, int
sk-port[1] = nk-port[1];
}
printf(%s , s-ifname);
- if ((p = getprotobynumber(s-proto)) != NULL)
- printf(%s , p-p_name);
- else
+ if ((p = getprotobynumber(s-proto)) != NULL) {
+ pn = p-p_name;
+ printf(%s , pn);
+ } else
printf(%u , s-proto);
- print_host(nk-addr[1], nk-port[1], nk-af, nk-rdomain, opts);
+ print_host(nk-addr[1], nk-port[1], nk-af, nk-rdomain, pn, opts);
if (nk-af != sk-af || PF_ANEQ(nk-addr[1], sk-addr[1], nk-af) ||
nk-port[1] != sk-port[1] ||
nk-rdomain != sk-rdomain) {
idx = afto ? 0 : 1;
printf( ();
print_host(sk-addr[idx], sk-port[idx], sk-af,
- sk-rdomain, opts);
+ sk-rdomain, pn, opts);
printf());
}
if (s-direction == PF_OUT || (afto s-direction == PF_IN))
printf( - );
else
printf( - );
- print_host(nk-addr[0], nk-port[0], nk-af, nk-rdomain, opts);
+ print_host(nk-addr[0], nk-port[0], nk-af, nk-rdomain, pn, opts);
if (nk-af != sk-af || PF_ANEQ(nk-addr[0], sk-addr[0], nk-af) ||
nk-port[0] != sk-port[0] ||
nk-rdomain != sk-rdomain) {
idx = afto ? 1 : 0;
printf( ();
print_host(sk-addr[idx], sk-port[idx], sk-af,
- sk-rdomain, opts);
+ sk-rdomain, pn, opts);
printf());
}
Index: pfctl.h
===
RCS file: /cvs/src/sbin/pfctl/pfctl.h,v
retrieving revision 1.47
diff -u -p -r1.47 pfctl.h
--- pfctl.h 27 Jul 2011 00:26:10 - 1.47
+++ pfctl.h 29 May 2012 04:38:38 -
@@ -106,7 +106,7 @@ struct pf_altq *pfaltq_lookup(const char
char *rate2str(double);
voidprint_addr(struct pf_addr_wrap *, sa_family_t, int);
-voidprint_host(struct pf_addr *, u_int16_t p, sa_family_t, u_int16_t, int);
+voidprint_host(struct pf_addr *, u_int16_t p, sa_family_t, u_int16_t,
const char *, int);
voidprint_seq(struct pfsync_state_peer *);
voidprint_state(struct pfsync_state *, int);
int unmask(struct pf_addr *, sa_family_t);
inet(3) man page: remove extra verbiage
The inet(3) man page has always felt messy to me, where the words function and routine are used interchangeably to describe the various functions in inconsistent ways. This extra verbiage makes it somewhat harder to look up the descriptions of functions. Since it is understood that this man page describes functions, I have created a diff that removes those words to make it easier and quicker for programmers to find the info they need. Comments? Ok? Lawrence Index: inet.3 === RCS file: /cvs/src/lib/libc/net/inet.3,v retrieving revision 1.23 diff -u -p -r1.23 inet.3 --- inet.3 20 Apr 2012 07:00:21 - 1.23 +++ inet.3 31 May 2012 03:39:50 - @@ -68,7 +68,6 @@ .Ft in_addr_t .Fn inet_lnaof struct in_addr in .Sh DESCRIPTION -The routines .Fn inet_aton , .Fn inet_addr , and @@ -78,27 +77,24 @@ numbers expressed in the Internet standa .Dq dot notation. .Pp -The .Fn inet_aton -routine interprets the specified character string as an Internet address, +interprets the specified character string as an Internet address, placing the address into the structure provided. It returns 1 if the string was successfully interpreted, or 0 if the string was invalid. .Pp -The .Fn inet_addr and .Fn inet_network -functions return numbers suitable for use +return numbers suitable for use as Internet addresses and Internet network numbers, respectively. Both functions return the constant .Dv INADDR_NONE if the specified character string is malformed. .Pp -The .Fn inet_pton -function converts a presentation format address (that is, printable form +converts a presentation format address (that is, printable form as held in a character string) to network format (usually a .Li struct in_addr or some other internal binary representation, in network byte order). @@ -112,7 +108,6 @@ This function is presently valid for and .Dv AF_INET6 . .Pp -The function .Fn inet_ntop converts an address from network format (usually a .Li struct in_addr @@ -125,18 +120,15 @@ error occurs (in which case, .Va errno will have been set), or it returns a pointer to the destination string. .Pp -The routine .Fn inet_ntoa takes an Internet address and returns an ASCII string representing the address in dot notation. .Pp -The routine .Fn inet_makeaddr takes an Internet network number and a local network address and constructs an Internet address from it. .Pp -The routines .Fn inet_netof and .Fn inet_lnaof
Re: pkg_add/pkg_delete: include PID in syslog messages
On Mon, Mar 19, 2012 at 12:50:12PM +0100, Marc Espie wrote:
On Wed, Mar 07, 2012 at 12:31:48AM -0500, Lawrence Teo wrote:
This simple diff makes pkg_add and pkg_delete include their PID when
logging to syslog. This is useful when trying to determine whether
several packages were added (or removed) by the same pkg_add (or
pkg_delete) process.
Here is some sample output:
Mar 3 22:15:17 obsd-amd64 pkg_add[3530]: Added nano-2.2.6
Mar 3 22:15:26 obsd-amd64 pkg_delete[21136]: Removed nano-2.2.6
Mar 3 22:16:51 obsd-amd64 pkg_add[30666]: Added redland-1.0.8p1
Mar 3 22:16:54 obsd-amd64 pkg_add[30666]: Added mozilla-dicts-en-GB-1.3
Mar 3 22:16:56 obsd-amd64 pkg_add[30666]: Added hunspell-1.2.12
Mar 3 22:23:22 obsd-amd64 pkg_add[30666]: Added libreoffice-3.4.5.2v0
Mar 3 23:33:23 obsd-amd64 pkg_add[5948]: Added kdiff3-0.9.96p1
Comments?
The more I think about it, the more I fail to see the value.
Consider that any pkg_add/pkg_delete that actually changes installed
packages *will* lock the database anyways, so by nature, all relevant
runs of pkg_add/pkg_delete will happen in sequence.
Hence, there's totally no ambiguity in the log lines. If I remove the pids
in there, I still have no trouble figuring out what happened.
When I look at those logs, I usually have to look at the time anyways,
so in the above case, figuring out that libreoffice and kdiff3 are
different runs of pkg_add is not that hard...
Thank you for your comments.
If I remove the PIDs from the above logs, part of it would look like
this:
Mar 3 22:16:51 obsd-amd64 pkg_add: Added redland-1.0.8p1
Mar 3 22:16:54 obsd-amd64 pkg_add: Added mozilla-dicts-en-GB-1.3
Mar 3 22:16:56 obsd-amd64 pkg_add: Added hunspell-1.2.12
Mar 3 22:23:22 obsd-amd64 pkg_add: Added libreoffice-3.4.5.2v0
Just relying on those timestamps alone without PIDs, it looks like
redland, mozilla-dicts, and hunspell are part of the same run, while
libreoffice is a different run.
However, with PIDs it is clear that they are part of the same run, where
it so happened that it took almost 6.5 minutes to install the
libreoffice package on this machine:
Mar 3 22:16:51 obsd-amd64 pkg_add[30666]: Added redland-1.0.8p1
Mar 3 22:16:54 obsd-amd64 pkg_add[30666]: Added mozilla-dicts-en-GB-1.3
Mar 3 22:16:56 obsd-amd64 pkg_add[30666]: Added hunspell-1.2.12
Mar 3 22:23:22 obsd-amd64 pkg_add[30666]: Added libreoffice-3.4.5.2v0
I won't say I can't be swayed, but you'll have to give me a concrete case
where the pid offers some actual advantage over no pid.
My goal is to help the bleary-eyed sysadmin figure out what in the world
s/he did after working on a project all night long. :) I think the
PIDs would help them answer questions like, Why did I install $PACKAGE
again? (where $PACKAGE could be hunspell in the above example).
Please let me know if you have any further thoughts and if you're ok
with the diff. Since a long time has passed since your post on this
thread and I only managed to reply now, I have included my diff again
below.
Thank you,
Lawrence
Index: AddDelete.pm
===
RCS file: /cvs/src/usr.sbin/pkg_add/OpenBSD/AddDelete.pm,v
retrieving revision 1.53
diff -u -p -r1.53 AddDelete.pm
--- AddDelete.pm9 Oct 2011 16:43:50 - 1.53
+++ AddDelete.pm4 Mar 2012 04:37:04 -
@@ -185,7 +185,7 @@ sub handle_options
$state-{loglevel} //= $state-config-value(loglevel) // 1;
if ($state-{loglevel}) {
require Sys::Syslog;
- Sys::Syslog::openlog($state-{cmd}, nofatal);
+ Sys::Syslog::openlog($state-{cmd}, nofatal,pid);
}
}
Latest libpcap 1.2.0 diff (2012-05-11)
I sent a diff to update libpcap in base in November 2011 [1]. Here's
the latest version that fixes a bug I found after running regression
tests on all pcap-based ports in the ports tree.
If anyone would like to test the diff, the programs in base that link to
libpcap and would also require testing are:
- /usr/libexec/spamlogd
- /usr/sbin/pppd
- /usr/sbin/tcpdump
- /sbin/tcpdump
There's also a test program in /usr/src/regress/lib/libpthread/pcap/
that uses libpcap.
I have set up a simple page at the following URL to keep track of my own
progress and to encourage more widespread testing.
http://lteo.net/libpcap/
If you do test it, I would really appreciate it if you could please let
me know your feedback, both positive and negative.
Thank you!
Lawrence
[1] http://marc.info/?l=openbsd-techm=132029368027597w=2
How to apply:
cd /usr/src/lib/libpcap
patch libpcap-1.2.0-20120511.diff
make obj
make includes
make
make install
cd /usr/src/usr.sbin/tcpdump
make obj make make install
cd /usr/src/libexec/spamlogd
make obj make make install
cd /usr/src/sbin/pflogd
make obj make make install
cd /usr/src/usr.sbin/pppd
make obj make make install
Index: Makefile
===
RCS file: /cvs/src/lib/libpcap/Makefile,v
retrieving revision 1.20
diff -u -p -r1.20 Makefile
--- Makefile13 Aug 2009 19:54:58 - 1.20
+++ Makefile11 May 2012 04:35:01 -
@@ -25,11 +25,15 @@ MLINKS= pcap.3 pcap_open_live.3 pcap.3 p
pcap.3 pcap_sendpacket.3 pcap.3 pcap_next_ex.3 \
pcap.3 pcap_setdirection.3 pcap.3 pcap_dump_file.3 \
pcap.3 pcap_dump_ftell.3 pcap.3 pcap_fopen_offline.3 \
- pcap.3 pcap_dump_flush.3
+ pcap.3 pcap_dump_flush.3 pcap.3 pcap_create.3 \
+ pcap.3 pcap_set_snaplen.3 pcap.3 pcap_set_promisc.3 \
+ pcap.3 pcap_can_set_rfmon.3 pcap.3 pcap_set_rfmon.3 \
+ pcap.3 pcap_set_timeout.3 pcap.3 pcap_set_buffer_size.3 \
+ pcap.3 pcap_activate.3 pcap.3 pcap_statustostr.3
DEFS= -DHAVE_SYS_IOCCOM_H -DHAVE_SYS_SOCKIO_H -DHAVE_ETHER_HOSTTON \
-DHAVE_STRERROR -DHAVE_SOCKADDR_SA_LEN -DLBL_ALIGN -DHAVE_IFADDRS_H \
- -DINET6
+ -DINET6 -DHAVE_BSD_IEEE80211
CFLAGS+=-I. -I${.CURDIR} -Dyylval=pcap_yylval ${DEFS}
Index: pcap-bpf.c
===
RCS file: /cvs/src/lib/libpcap/pcap-bpf.c,v
retrieving revision 1.20
diff -u -p -r1.20 pcap-bpf.c
--- pcap-bpf.c 26 Mar 2006 20:58:51 - 1.20
+++ pcap-bpf.c 11 May 2012 04:35:01 -
@@ -38,6 +38,8 @@
#include string.h
#include unistd.h
+#include net/if_media.h
+
#include pcap-int.h
#ifdef HAVE_OS_PROTO_H
@@ -46,6 +48,12 @@
#include gencode.h
+static int find_802_11(struct bpf_dltlist *);
+static int monitor_mode(pcap_t *, int);
+
+static int pcap_activate_bpf(pcap_t *p);
+static int pcap_setfilter_bpf(pcap_t *p, struct bpf_program *fp);
+
int
pcap_stats(pcap_t *p, struct pcap_stat *ps)
{
@@ -54,7 +62,7 @@ pcap_stats(pcap_t *p, struct pcap_stat *
if (ioctl(p-fd, BIOCGSTATS, (caddr_t)s) 0) {
snprintf(p-errbuf, PCAP_ERRBUF_SIZE, BIOCGSTATS: %s,
pcap_strerror(errno));
- return (-1);
+ return (PCAP_ERROR);
}
ps-ps_recv = s.bs_recv;
@@ -76,11 +84,11 @@ pcap_read(pcap_t *p, int cnt, pcap_handl
if (p-break_loop) {
/*
* Yes - clear the flag that indicates that it
-* has, and return -2 to indicate that we were
-* told to break out of the loop.
+* has, and return PCAP_ERROR_BREAK to indicate
+* that we were told to break out of the loop.
*/
p-break_loop = 0;
- return (-2);
+ return (PCAP_ERROR_BREAK);
}
cc = p-cc;
@@ -95,6 +103,21 @@ pcap_read(pcap_t *p, int cnt, pcap_handl
case EWOULDBLOCK:
return (0);
+
+ case ENXIO:
+ /*
+* The device on which we're capturing
+* went away.
+*
+* XXX - we should really return
+* PCAP_ERROR_IFACE_NOT_UP, but
+* pcap_dispatch() etc. aren't
+* defined to retur that.
+*/
+ snprintf(p-errbuf, PCAP_ERRBUF_SIZE,
+ The interface went down);
+ return (PCAP_ERROR);
+
#if defined(sun) !defined(BSD)
/*
* Due to a SunOS bug, after 2^31 bytes, the kernel
@@ -112,7 +135,7 @@ pcap_read(pcap_t *p, int cnt, pcap_handl
}
Re: Latest libpcap 1.2.0 diff (2012-05-11)
On Tue, May 15, 2012 at 08:06:43PM +0100, Stuart Henderson wrote: On 2012/05/15 14:19, Lawrence Teo wrote: If anyone would like to test the diff, the programs in base that link to libpcap and would also require testing are: - /usr/libexec/spamlogd - /usr/sbin/pppd - /usr/sbin/tcpdump - /sbin/tcpdump I think you meant pflogd ;) Oops, yes, thank you for catching that! Lawrence
sha1(1) man page: recommend sha256(1) instead
This diff changes the sha1(1) man page to recommend the use of sha256(1). It uses language that is similar to the md5(1) man page. Thoughts? Lawrence Index: sha1.1 === RCS file: /cvs/src/bin/md5/sha1.1,v retrieving revision 1.29 diff -u -p -r1.29 sha1.1 --- sha1.1 3 Sep 2010 09:53:20 - 1.29 +++ sha1.1 8 May 2012 03:46:24 - @@ -35,8 +35,14 @@ takes as input a message of arbitrary length and produces as output a 160-bit fingerprint or message digest of the input. It is conjectured that it is computationally infeasible to produce -two messages having the same message digest, or to produce any +two messages having the same message digest (a collision), or to produce any message having a given prespecified target message digest. +However, researchers have developed theoretical attacks that significantly +reduce the amount of time needed to find a collision in +.Em SHA-1 . +The use of other message digest functions, such as +.Xr sha256 1 , +is now preferred. .Pp The .Em SHA-1 @@ -116,3 +122,19 @@ sha256, sha384 and sha512. .%T US Secure Hash Algorithm 1 .%O RFC 3174 .Re +.Rs +.%A X. Wang +.%A Y. Yin +.%A H. Yu +.%T Finding Collisions in the Full SHA-1 +.%J Crypto +.%D 2005 +.Re +.Sh CAVEATS +Theoretical attacks that significantly reduce the amount of time needed +to find a collision in +.Em SHA-1 +have been developed. +The use of +.Xr sha256 1 +is recommended instead.
cron: fix incorrect error message
This diff fixes the error message for one of the log_it() calls in cron
(was probably a pasto). While here, also fix the style for two other
log_it() calls.
Lawrence
Index: cron.c
===
RCS file: /cvs/src/usr.sbin/cron/cron.c,v
retrieving revision 1.43
diff -u -p -r1.43 cron.c
--- cron.c 22 Aug 2011 19:32:42 - 1.43
+++ cron.c 8 May 2012 03:42:12 -
@@ -100,7 +100,7 @@ main(int argc, char *argv[]) {
set_cron_cwd();
if (putenv(PATH=_PATH_DEFPATH) 0) {
- log_it(CRON, getpid(), DEATH, can't malloc);
+ log_it(CRON, getpid(), DEATH, can't set PATH);
exit(EXIT_FAILURE);
}
@@ -113,7 +113,7 @@ main(int argc, char *argv[]) {
} else if (NoFork == 0) {
switch (fork()) {
case -1:
- log_it(CRON,getpid(),DEATH,can't fork);
+ log_it(CRON, getpid(), DEATH, can't fork);
exit(EXIT_FAILURE);
break;
case 0:
@@ -126,7 +126,7 @@ main(int argc, char *argv[]) {
if (fd != STDERR_FILENO)
(void) close(fd);
}
- log_it(CRON,getpid(),STARTUP,CRON_VERSION);
+ log_it(CRON, getpid(), STARTUP, CRON_VERSION);
break;
default:
/* parent process should just die */
Re: tcpbench: crash with -n and -b
On Fri, May 04, 2012 at 11:34:02PM +0200, Erik Lax wrote: Hi, I noticed that tcpbench sometimes crashes when using -n and -b combined, this is because of a double-free in the client initialization loop. This is consistently reproducible for me, if I run tcpbench -s in one session and tcpbench with the following -n and -b flags in another: $ tcpbench -v -n 50 -b 127.0.0.1 127.0.0.1 elapsed_ms bytes mbps bwidth Trying [127.0.0.1]:12345 Try to bind to [127.0.0.1]:0 Try to bind to [127.0.0.1]:0 Try to bind to [127.0.0.1]:0 Try to bind to [127.0.0.1]:0 Try to bind to [127.0.0.1]:0 Try to bind to [127.0.0.1]:0 Try to bind to [127.0.0.1]:0 Try to bind to [127.0.0.1]:0 Try to bind to [127.0.0.1]:0 tcpbench in free(): error: chunk is already free 0x209819300 Abort trap (core dumped) Erik's diff fixes it for me, and the code looks good to me too. :) Lawrence
Re: pfctl: fix printing of 'foo/*' anchors
On Fri, Apr 27, 2012 at 12:45:01PM +0100, Stuart Henderson wrote:
On 2012/04/27 00:51, Lawrence Teo wrote:
The diff below fixes pfctl so that it will show the 'authpf/*' anchor
as intended:
This is extremely useful for relayd/ftp-proxy too.
I agree. I bumped into this bug while writing a custom proxy that uses
'foo/*' anchors, so I think the fix will also help future proxies that
use these anchors.
Note that since this diff changes the behavior of
pfctl -a 'foo/*' -sr, it will also change the pfload* regression
tests since those tests execute this command:
pfctl -o none -a 'regress/*' -gvvsr
If this diff is correct, I would appreciate some guidance from the
developers on how to address the pfload* regression tests.
First problem: -a 'regress/*' used to print anchor regress,
and now it _only_ prints sub-anchors. Following diff changes it
to print the named anchor itself as well as sub-anchors, which
is how I would expect recursive printing to work.
Thank you for catching this! I agree that the behavior shown by your
revised diff is much closer to what users expect.
If '-s l' and maybe also '-s a' get the same treatment then I think
I'll be OK with this going in.
Here's a revised diff that makes it work with '-s l'.
Index: pfctl.c
===
RCS file: /cvs/src/sbin/pfctl/pfctl.c,v
retrieving revision 1.310
diff -u -p pfctl.c
--- pfctl.c 18 Apr 2012 14:42:17 - 1.310
+++ pfctl.c 28 Apr 2012 02:18:05 -
@@ -864,6 +864,7 @@ pfctl_show_rules(int dev, char *path, int opts, enum p
switch (format) {
case PFCTL_SHOW_LABELS:
if (pr.rule.label[0]) {
+ INDENT(depth, !(opts PF_OPT_VERBOSE));
printf(%s %llu %llu %llu %llu
%llu %llu %llu %llu\n,
pr.rule.label,
@@ -1937,6 +1938,7 @@ main(int argc, char *argv[])
int optimize = PF_OPTIMIZE_BASIC;
int level;
char anchorname[MAXPATHLEN];
+ int anchor_wildcard = 0;
char*path;
char*lfile = NULL, *sfile = NULL;
const char *errstr;
@@ -2097,9 +2099,10 @@ main(int argc, char *argv[])
int len = strlen(anchoropt);
if (anchoropt[len - 1] == '*') {
- if (len = 2 anchoropt[len - 2] == '/')
+ if (len = 2 anchoropt[len - 2] == '/') {
anchoropt[len - 2] = '\0';
- else
+ anchor_wildcard = 1;
+ } else
anchoropt[len - 1] = '\0';
opts |= PF_OPT_RECURSE;
}
@@ -2137,11 +2140,19 @@ main(int argc, char *argv[])
pfctl_load_fingerprints(dev, opts);
pfctl_show_rules(dev, path, opts, PFCTL_SHOW_RULES,
anchorname, 0, 0, shownr);
+ if (anchor_wildcard)
+ pfctl_show_rules(dev, path, opts,
+ PFCTL_SHOW_RULES, anchorname, 0,
+ anchor_wildcard, shownr);
break;
case 'l':
pfctl_load_fingerprints(dev, opts);
pfctl_show_rules(dev, path, opts, PFCTL_SHOW_LABELS,
anchorname, 0, 0, shownr);
+ if (anchor_wildcard)
+ pfctl_show_rules(dev, path, opts,
+ PFCTL_SHOW_LABELS, anchorname, 0,
+ anchor_wildcard, shownr);
break;
case 'q':
pfctl_show_altq(dev, ifaceopt, opts,
I'm not sure what's the best way to handle '-s a' yet, since I think
users typically run 'pfctl -s a' to find out everything about their
full ruleset instead of specific anchors. Applying a recursive
behavior on '-s a' also seems complex. Do you have any thoughts on
what a command like pfctl -a 'foo/*' -s a should show?
Next problem, and this is not directly relating to the above diff
to fix printing, but it is highlighted by it: the anchors added by
pfload regression tests are never emptied. Running 'make pfload-update'
results in the same crap being added to lots of the files it produces.
So it seems the -Fr / -Fa flush commands also need teaching how to work
recursively and the regress Makefile changing to use this.
If I bypass this problem by temporarily adjusting pfload${n}-update
to use individual anchor names per rule (rather than '-a regress'
I used '-a foo$n') I see that with the above diff an additional
summary is printed each time, each rule-printing gets these additional
lines:
[ Skip steps: i=end d
pfctl: fix printing of 'foo/*' anchors
The pfctl(8) man page says:
By default, recursive inline printing of anchors applies only to
unnamed anchors specified inline in the ruleset. If the anchor
name is terminated with a `*' character, the -s flag will
recursively print all anchors in a brace delimited block. For
example the following will print the ``authpf'' ruleset
recursively:
# pfctl -a 'authpf/*' -sr
However, that pfctl command will not show any output, whether for
'authpf/*' or any other 'foo/*' anchor, even if they are populated.
I tested this by setting up authpf and logged in as two users (bula and
charlie) so that my 'authpf/*' anchor is populated. My live PF
ruleset looks like this:
# pfctl -a '*' -sr
pass all flags S/SA
anchor authpf/* all {
anchor bula(1874) all {
pass in on em0 inet proto tcp from 172.16.0.15 to any port = 9876 flags S/SA
}
anchor charlie(5749) all {
pass in quick on em0 inet proto tcp from 172.16.0.22 to any port = 5678
flags S/SA
}
}
When I try to print just the 'authpf/*' anchor per the man page, pfctl
does not show any output:
# pfctl -a 'authpf/*' -sr
#
The diff below fixes pfctl so that it will show the 'authpf/*' anchor
as intended:
# pfctl -a 'authpf/*' -sr
anchor bula(1874) all {
pass in on em0 inet proto tcp from 172.16.0.15 to any port = 9876 flags S/SA
}
anchor charlie(5749) all {
pass in quick on em0 inet proto tcp from 172.16.0.22 to any port = 5678 flags
S/SA
}
Note that since this diff changes the behavior of
pfctl -a 'foo/*' -sr, it will also change the pfload* regression
tests since those tests execute this command:
pfctl -o none -a 'regress/*' -gvvsr
If this diff is correct, I would appreciate some guidance from the
developers on how to address the pfload* regression tests.
Comments are welcome.
Thank you,
Lawrence
Index: pfctl.c
===
RCS file: /cvs/src/sbin/pfctl/pfctl.c,v
retrieving revision 1.310
diff -u -p -r1.310 pfctl.c
--- pfctl.c 18 Apr 2012 14:42:17 - 1.310
+++ pfctl.c 26 Apr 2012 03:50:02 -
@@ -1937,6 +1937,7 @@ main(int argc, char *argv[])
int optimize = PF_OPTIMIZE_BASIC;
int level;
char anchorname[MAXPATHLEN];
+ int anchor_wildcard = 0;
char*path;
char*lfile = NULL, *sfile = NULL;
const char *errstr;
@@ -2097,9 +2098,10 @@ main(int argc, char *argv[])
int len = strlen(anchoropt);
if (anchoropt[len - 1] == '*') {
- if (len = 2 anchoropt[len - 2] == '/')
+ if (len = 2 anchoropt[len - 2] == '/') {
anchoropt[len - 2] = '\0';
- else
+ anchor_wildcard = 1;
+ } else
anchoropt[len - 1] = '\0';
opts |= PF_OPT_RECURSE;
}
@@ -2136,7 +2138,7 @@ main(int argc, char *argv[])
case 'r':
pfctl_load_fingerprints(dev, opts);
pfctl_show_rules(dev, path, opts, PFCTL_SHOW_RULES,
- anchorname, 0, 0, shownr);
+ anchorname, 0, anchor_wildcard, shownr);
break;
case 'l':
pfctl_load_fingerprints(dev, opts);
Re: ftp(1): new -s flag to specify source IP address
On Wed, Apr 18, 2012 at 11:58:26PM -0400, Lawrence Teo wrote:
This diff adds a -s flag to ftp(1) to let the user specify the
source IP address of the connection. This is useful when using
ftp(1) over VPN tunnels or when an alternate source IP is required
to fetch a file from a FTP/HTTP/HTTPS server due to access control
policies.
I have revised this diff based on feedback from haesbaert@. The primary
change is to use AI_NUMERICHOST in the hints addrinfo struct that's
passed to getaddrinfo().
The resulting ftp(1) binary still passes all 48 tests from my test
script at http://lteo.net/stuff/ftp-test
Comments and more testing are welcome!
Thanks,
Lawrence
Index: fetch.c
===
RCS file: /cvs/src/usr.bin/ftp/fetch.c,v
retrieving revision 1.104
diff -u -p -r1.104 fetch.c
--- fetch.c 23 Apr 2012 21:22:02 - 1.104
+++ fetch.c 25 Apr 2012 02:12:13 -
@@ -179,7 +179,7 @@ url_get(const char *origline, const char
char *hosttail, *cause = unknown, *newline, *host, *port, *buf = NULL;
char *epath, *redirurl, *loctail;
int error, i, isftpurl = 0, isfileurl = 0, isredirect = 0, rval = -1;
- struct addrinfo hints, *res0, *res;
+ struct addrinfo hints, *res0, *res, *ares = NULL;
const char * volatile savefile;
char * volatile proxyurl = NULL;
char *cookie = NULL;
@@ -198,6 +198,7 @@ url_get(const char *origline, const char
#endif /* !SMALL */
SSL *ssl = NULL;
int status;
+ int save_errno;
direction = received;
@@ -490,6 +491,17 @@ noslash:
goto cleanup_url_get;
}
+#ifndef SMALL
+ if (srcaddr) {
+ hints.ai_flags |= AI_NUMERICHOST;
+ error = getaddrinfo(srcaddr, NULL, hints, ares);
+ if (error) {
+ warnx(%s: %s, gai_strerror(error), srcaddr);
+ goto cleanup_url_get;
+ }
+ }
+#endif /* !SMALL */
+
s = -1;
for (res = res0; res; res = res-ai_next) {
if (getnameinfo(res-ai_addr, res-ai_addrlen, hbuf,
@@ -504,10 +516,28 @@ noslash:
continue;
}
+#ifndef SMALL
+ if (srcaddr) {
+ if (ares-ai_family != res-ai_family) {
+ close(s);
+ s = -1;
+ errno = EINVAL;
+ cause = bind;
+ continue;
+ }
+ if (bind(s, ares-ai_addr, ares-ai_addrlen) 0) {
+ save_errno = errno;
+ close(s);
+ errno = save_errno;
+ s = -1;
+ cause = bind;
+ continue;
+ }
+ }
+#endif /* !SMALL */
+
again:
if (connect(s, res-ai_addr, res-ai_addrlen) 0) {
- int save_errno;
-
if (errno == EINTR)
goto again;
save_errno = errno;
@@ -532,6 +562,10 @@ again:
break;
}
freeaddrinfo(res0);
+#ifndef SMALL
+ if (srcaddr)
+ freeaddrinfo(ares);
+#endif /* !SMALL */
if (s 0) {
warn(%s, cause);
goto cleanup_url_get;
Index: ftp.1
===
RCS file: /cvs/src/usr.bin/ftp/ftp.1,v
retrieving revision 1.81
diff -u -p -r1.81 ftp.1
--- ftp.1 26 Jul 2010 21:31:34 - 1.81
+++ ftp.1 25 Apr 2012 02:12:13 -
@@ -42,10 +42,12 @@
.Op Fl k Ar seconds
.Op Fl P Ar port
.Op Fl r Ar seconds
+.Op Fl s Ar srcaddr
.Op Ar host Op Ar port
.Nm ftp
.Op Fl C
.Op Fl o Ar output
+.Op Fl s Ar srcaddr
.Sm off
.No ftp:// Oo Ar user : password No @
.Oc Ar host Oo : Ar port
@@ -57,6 +59,7 @@
.Op Fl C
.Op Fl c Ar cookie
.Op Fl o Ar output
+.Op Fl s Ar srcaddr
.Sm off
.No http:// Ar host Oo : Ar port
.Oc No / Ar file
@@ -66,6 +69,7 @@
.Op Fl C
.Op Fl c Ar cookie
.Op Fl o Ar output
+.Op Fl s Ar srcaddr
.Sm off
.No https:// Ar host Oo : Ar port
.Oc No / Ar file
@@ -74,6 +78,7 @@
.Nm ftp
.Op Fl C
.Op Fl o Ar output
+.Op Fl s Ar srcaddr
.Sm off
.No file: Ar file
.Sm on
@@ -81,6 +86,7 @@
.Nm ftp
.Op Fl C
.Op Fl o Ar output
+.Op Fl s Ar srcaddr
.Sm off
.Ar host : No / Ar file Oo /
.Oc
@@ -220,6 +226,12 @@ if the server does not support passive c
.It Fl r Ar seconds
Retry to connect if failed, pausing for number of
.Ar seconds .
+.It Fl s Ar srcaddr
+Use
+.Ar srcaddr
+on the local machine as the source address
+of the connection.
+Only useful on systems with more than one address.
.It Fl t
Enables packet tracing.
.It Fl V
Index: ftp.c
Re: ftp-proxy(8): ensure nat_range_high is defined in add_nat()
On Wed, Apr 18, 2012 at 11:36:49PM -0400, Lawrence Teo wrote:
This simple diff adds a check to the add_nat() function in
ftp-proxy(8) to ensure that nat_range_high is defined before
proceeding to create the PF NAT rule. I think the original author
may have intended to do this since there is an existing check for
nat_range_low.
Technically, all calls to add_nat() already use non-zero values for
nat_range_low and nat_range_high, but I think it is still important
to add the check as an additional safeguard in case those calls do
change in the future.
I received a reply mentioning that my original diff overran 80 columns
columns but was otherwise ok. Here is a revised diff that keeps the
lines within 80 columns.
Lawrence
Index: filter.c
===
RCS file: /cvs/src/usr.sbin/ftp-proxy/filter.c,v
retrieving revision 1.17
diff -u -p -r1.17 filter.c
--- filter.c6 Mar 2012 12:50:20 - 1.17
+++ filter.c20 Apr 2012 00:55:18 -
@@ -71,7 +71,7 @@ add_nat(u_int32_t id, struct sockaddr *s
u_int16_t nat_range_high)
{
if (!src || !dst || !d_port || !nat || !nat_range_low ||
- (src-sa_family != nat-sa_family)) {
+ !nat_range_high || (src-sa_family != nat-sa_family)) {
errno = EINVAL;
return (-1);
}
ftp-proxy(8): ensure nat_range_high is defined in add_nat()
This simple diff adds a check to the add_nat() function in
ftp-proxy(8) to ensure that nat_range_high is defined before
proceeding to create the PF NAT rule. I think the original author
may have intended to do this since there is an existing check for
nat_range_low.
Technically, all calls to add_nat() already use non-zero values for
nat_range_low and nat_range_high, but I think it is still important
to add the check as an additional safeguard in case those calls do
change in the future.
Comments?
Thanks,
Lawrence
Index: filter.c
===
RCS file: /cvs/src/usr.sbin/ftp-proxy/filter.c,v
retrieving revision 1.17
diff -u -p -r1.17 filter.c
--- filter.c6 Mar 2012 12:50:20 - 1.17
+++ filter.c16 Apr 2012 03:17:47 -
@@ -70,7 +70,7 @@ add_nat(u_int32_t id, struct sockaddr *s
u_int16_t d_port, struct sockaddr *nat, u_int16_t nat_range_low,
u_int16_t nat_range_high)
{
- if (!src || !dst || !d_port || !nat || !nat_range_low ||
+ if (!src || !dst || !d_port || !nat || !nat_range_low ||
!nat_range_high ||
(src-sa_family != nat-sa_family)) {
errno = EINVAL;
return (-1);
inet(3) man page: reorganize for better readability
The inet(3) man page has always been rather difficult to read for me, primarily because the order of the inet_* functions in the DESCRIPTION section is not the same as their order in SYNOPSIS. In addition, the descriptions of various functions are grouped together in large paragraphs, making it hard to look up a specific function. For example, the description of inet_pton() is sandwiched deep within the first large paragraph that also describes three other functions. To address these issues, this diff: - reorganizes the SYNOPSIS and DESCRIPTION sections so that their order is in sync; - separates the description of each function into its own paragraph for readability (where possible); - moves the contents of the DIAGNOSTICS section to the description of inet_addr() and inet_network(), because that information is more useful there; and - lists the functions according to their logical meaning, e.g. inet_aton(), inet_addr(), inet_network(), and inet_pton() are listed together as a group because these functions are related to converting a character string to its Internet address / network number. For your convenience, you can see the original and revised versions of the man page here: http://lteo.net/stuff/inet.3.diff.html (original on left, revised on right) There are other things I want to change in this man page but I would like to get feedback on this initial diff first. Any thoughts? Thanks, Lawrence Index: inet.3 === RCS file: /cvs/src/lib/libc/net/inet.3,v retrieving revision 1.22 diff -u -p -r1.22 inet.3 --- inet.3 9 Dec 2008 19:38:38 - 1.22 +++ inet.3 16 Apr 2012 02:04:07 - @@ -34,39 +34,39 @@ .Dt INET 3 .Os .Sh NAME -.Nm inet_addr , .Nm inet_aton , -.Nm inet_lnaof , -.Nm inet_makeaddr , -.Nm inet_netof , +.Nm inet_addr , .Nm inet_network , -.Nm inet_ntoa , +.Nm inet_pton , .Nm inet_ntop , -.Nm inet_pton +.Nm inet_ntoa , +.Nm inet_makeaddr , +.Nm inet_netof , +.Nm inet_lnaof .Nd Internet address manipulation routines .Sh SYNOPSIS .Fd #include sys/types.h .Fd #include sys/socket.h .Fd #include netinet/in.h .Fd #include arpa/inet.h -.Ft in_addr_t -.Fn inet_addr const char *cp .Ft int .Fn inet_aton const char *cp struct in_addr *addr .Ft in_addr_t -.Fn inet_lnaof struct in_addr in +.Fn inet_addr const char *cp +.Ft in_addr_t +.Fn inet_network const char *cp +.Ft int +.Fn inet_pton int af const char *src void *dst +.Ft const char * +.Fn inet_ntop int af const void *src char *dst socklen_t size +.Ft char * +.Fn inet_ntoa struct in_addr in .Ft struct in_addr .Fn inet_makeaddr in_addr_t net in_addr_t lna .Ft in_addr_t .Fn inet_netof struct in_addr in .Ft in_addr_t -.Fn inet_network const char *cp -.Ft char * -.Fn inet_ntoa struct in_addr in -.Ft const char * -.Fn inet_ntop int af const void *src char *dst socklen_t size -.Ft int -.Fn inet_pton int af const char *src void *dst +.Fn inet_lnaof struct in_addr in .Sh DESCRIPTION The routines .Fn inet_aton , @@ -77,6 +77,25 @@ interpret character strings representing numbers expressed in the Internet standard .Dq dot notation. +.Pp +The +.Fn inet_aton +routine interprets the specified character string as an Internet address, +placing the address into the structure provided. +It returns 1 if the string was successfully interpreted, +or 0 if the string was invalid. +.Pp +The +.Fn inet_addr +and +.Fn inet_network +functions return numbers suitable for use +as Internet addresses and Internet network +numbers, respectively. +Both functions return the constant +.Dv INADDR_NONE +if the specified character string is malformed. +.Pp The .Fn inet_pton function converts a presentation format address (that is, printable form @@ -92,19 +111,6 @@ This function is presently valid for .Dv AF_INET and .Dv AF_INET6 . -The -.Fn inet_aton -routine interprets the specified character string as an Internet address, -placing the address into the structure provided. -It returns 1 if the string was successfully interpreted, -or 0 if the string was invalid. -The -.Fn inet_addr -and -.Fn inet_network -functions return numbers suitable for use -as Internet addresses and Internet network -numbers, respectively. .Pp The function .Fn inet_ntop @@ -118,15 +124,18 @@ if a system error occurs (in which case, .Va errno will have been set), or it returns a pointer to the destination string. +.Pp The routine .Fn inet_ntoa takes an Internet address and returns an ASCII string representing the address in dot notation. +.Pp The routine .Fn inet_makeaddr takes an Internet network number and a local network address and constructs an Internet address from it. +.Pp The routines .Fn inet_netof and @@ -261,14 +270,6 @@ or in compressed form: :::129.144.52.38 .Ed .El -.Sh DIAGNOSTICS -The constant -.Dv INADDR_NONE -is returned by -.Fn inet_addr -and -.Fn inet_network -for malformed requests. .Sh SEE ALSO .Xr byteorder 3 , .Xr
ftp(1): new -s flag to specify source IP address
This diff adds a -s flag to ftp(1) to let the user specify the
source IP address of the connection. This is useful when using
ftp(1) over VPN tunnels or when an alternate source IP is required
to fetch a file from a FTP/HTTP/HTTPS server due to access control
policies.
The -s flag is present in ftp(1) on FreeBSD, NetBSD, DragonFly BSD,
and even OS X so it is very portable in shell scripts. :)
I have tested -s with both IPv4 and IPv6 with a variety of different
flags. The following is the test script that I used during
development to confirm that this diff does not break existing
behavior and to ensure that -s works with related flags:
http://lteo.net/stuff/ftp-test
These are the test results:
http://lteo.net/stuff/ftp-test-results.txt
(with my IPv6 address masked out)
Note: If you would like to replicate the test with my test script,
please assign two IPv4 addresses and two IPv6 addresses to your
egress interface. If you don't have IPv6 connectivity, you can run
the script in IPv4-only mode with ftp-test -4
Comments and feedback would be appreciated.
Thanks,
Lawrence
Index: fetch.c
===
RCS file: /cvs/src/usr.bin/ftp/fetch.c,v
retrieving revision 1.103
diff -u -p -r1.103 fetch.c
--- fetch.c 25 Aug 2010 20:32:37 - 1.103
+++ fetch.c 15 Apr 2012 02:08:43 -
@@ -179,7 +179,7 @@ url_get(const char *origline, const char
char *hosttail, *cause = unknown, *newline, *host, *port, *buf = NULL;
char *epath, *redirurl, *loctail;
int error, i, isftpurl = 0, isfileurl = 0, isredirect = 0, rval = -1;
- struct addrinfo hints, *res0, *res;
+ struct addrinfo hints, *res0, *res, *ares = NULL;
const char * volatile savefile;
char * volatile proxyurl = NULL;
char *cookie = NULL;
@@ -198,6 +198,7 @@ url_get(const char *origline, const char
#endif /* !SMALL */
SSL *ssl = NULL;
int status;
+ int save_errno;
direction = received;
@@ -490,6 +491,16 @@ noslash:
goto cleanup_url_get;
}
+#ifndef SMALL
+ if (srcaddr) {
+ error = getaddrinfo(srcaddr, NULL, hints, ares);
+ if (error) {
+ warnx(%s: %s, gai_strerror(error), srcaddr);
+ goto cleanup_url_get;
+ }
+ }
+#endif /* !SMALL */
+
s = -1;
for (res = res0; res; res = res-ai_next) {
if (getnameinfo(res-ai_addr, res-ai_addrlen, hbuf,
@@ -504,6 +515,26 @@ noslash:
continue;
}
+#ifndef SMALL
+ if (srcaddr) {
+ if (ares-ai_family != res-ai_family) {
+ close(s);
+ s = -1;
+ errno = EINVAL;
+ cause = bind;
+ continue;
+ }
+ if (bind(s, ares-ai_addr, ares-ai_addrlen) 0) {
+ save_errno = errno;
+ close(s);
+ errno = save_errno;
+ s = -1;
+ cause = bind;
+ continue;
+ }
+ }
+#endif /* !SMALL */
+
again:
if (connect(s, res-ai_addr, res-ai_addrlen) 0) {
int save_errno;
@@ -532,6 +563,10 @@ again:
break;
}
freeaddrinfo(res0);
+#ifndef SMALL
+ if (srcaddr)
+ freeaddrinfo(ares);
+#endif /* !SMALL */
if (s 0) {
warn(%s, cause);
goto cleanup_url_get;
Index: ftp.1
===
RCS file: /cvs/src/usr.bin/ftp/ftp.1,v
retrieving revision 1.81
diff -u -p -r1.81 ftp.1
--- ftp.1 26 Jul 2010 21:31:34 - 1.81
+++ ftp.1 15 Apr 2012 02:08:43 -
@@ -42,10 +42,12 @@
.Op Fl k Ar seconds
.Op Fl P Ar port
.Op Fl r Ar seconds
+.Op Fl s Ar srcaddr
.Op Ar host Op Ar port
.Nm ftp
.Op Fl C
.Op Fl o Ar output
+.Op Fl s Ar srcaddr
.Sm off
.No ftp:// Oo Ar user : password No @
.Oc Ar host Oo : Ar port
@@ -57,6 +59,7 @@
.Op Fl C
.Op Fl c Ar cookie
.Op Fl o Ar output
+.Op Fl s Ar srcaddr
.Sm off
.No http:// Ar host Oo : Ar port
.Oc No / Ar file
@@ -66,6 +69,7 @@
.Op Fl C
.Op Fl c Ar cookie
.Op Fl o Ar output
+.Op Fl s Ar srcaddr
.Sm off
.No https:// Ar host Oo : Ar port
.Oc No / Ar file
@@ -74,6 +78,7 @@
.Nm ftp
.Op Fl C
.Op Fl o Ar output
+.Op Fl s Ar srcaddr
.Sm off
.No file: Ar file
.Sm on
@@ -81,6 +86,7 @@
.Nm ftp
.Op Fl C
.Op Fl o Ar output
+.Op Fl s Ar srcaddr
.Sm off
.Ar host : No / Ar file Oo /
.Oc
@@ -220,6 +226,12 @@ if the server does not support passive c
.It Fl r Ar seconds
Retry to connect if failed, pausing for number of
.Ar seconds .
+.It Fl s
pkg_add/pkg_delete: include PID in syslog messages
This simple diff makes pkg_add and pkg_delete include their PID when
logging to syslog. This is useful when trying to determine whether
several packages were added (or removed) by the same pkg_add (or
pkg_delete) process.
Here is some sample output:
Mar 3 22:15:17 obsd-amd64 pkg_add[3530]: Added nano-2.2.6
Mar 3 22:15:26 obsd-amd64 pkg_delete[21136]: Removed nano-2.2.6
Mar 3 22:16:51 obsd-amd64 pkg_add[30666]: Added redland-1.0.8p1
Mar 3 22:16:54 obsd-amd64 pkg_add[30666]: Added mozilla-dicts-en-GB-1.3
Mar 3 22:16:56 obsd-amd64 pkg_add[30666]: Added hunspell-1.2.12
Mar 3 22:23:22 obsd-amd64 pkg_add[30666]: Added libreoffice-3.4.5.2v0
Mar 3 23:33:23 obsd-amd64 pkg_add[5948]: Added kdiff3-0.9.96p1
Comments?
Thanks,
Lawrence
Index: AddDelete.pm
===
RCS file: /cvs/src/usr.sbin/pkg_add/OpenBSD/AddDelete.pm,v
retrieving revision 1.53
diff -u -p -r1.53 AddDelete.pm
--- AddDelete.pm9 Oct 2011 16:43:50 - 1.53
+++ AddDelete.pm4 Mar 2012 04:37:04 -
@@ -185,7 +185,7 @@ sub handle_options
$state-{loglevel} //= $state-config-value(loglevel) // 1;
if ($state-{loglevel}) {
require Sys::Syslog;
- Sys::Syslog::openlog($state-{cmd}, nofatal);
+ Sys::Syslog::openlog($state-{cmd}, nofatal,pid);
}
}
Re: request for the inclusion of the pcap-filter manpage
On Mon, Mar 05, 2012 at 12:43:07AM +0100, Ingo Schwarze wrote: http://www.manpagez.com/man/7/pcap-filter/ http://www.tcpdump.org/release/libpcap-1.2.1.tar.gz Please consider adding it to the distribution. From cursory inspection, it looks like OpenBSD is using a fork of libpcap 0.4 or 0.5, selectively including later tcpdump.org additions, but not tracking upstream development closely, so including parts of the libpcap 1.2.1 manual would seem wrong. Yes, the last selective sync was with libpcap 0.9.4 in 2006. On a related note, I sent a diff to tech@ back in November 2011 that imports a number of core functions from libpcap 1.2.0 to libpcap in base. It makes it easier to port programs that need the newer libpcap (like Snort 2.9.x) to OpenBSD. The diff could use some testing if anyone's interested. :) http://marc.info/?l=openbsd-techm=132029368027597w=2 Thanks, Lawrence
queue(3) TAILQ example causes compiler warning
The following example code in the queue(3) man page to delete all
elements in a tail queue generates a warning in gcc and clang.
while (np = TAILQ_FIRST(head)) {
TAILQ_REMOVE(head, np, entries);
free(np);
}
Here's a demo:
===BEGIN===
$ cat tailq.c
#include sys/queue.h
#include stdlib.h
struct entry {
int foo;
TAILQ_ENTRY(entry) entries;
} *np;
TAILQ_HEAD(tailhead, entry) head;
int
main(int argc, char *argv[])
{
TAILQ_INIT(head);
while (np = TAILQ_FIRST(head)) {
TAILQ_REMOVE(head, np, entries);
free(np);
}
return 0;
}
$ gcc -O2 -Wall -o tailq tailq.c
tailq.c: In function 'main':
tailq.c:16: warning: suggest parentheses around assignment used as truth value
===END===
The following diff fixes the example to prevent the warning from being
triggered.
Comments?
Thanks,
Lawrence
Index: queue.3
===
RCS file: /cvs/src/share/man/man3/queue.3,v
retrieving revision 1.54
diff -u -p -r1.54 queue.3
--- queue.3 11 Jan 2012 19:26:34 - 1.54
+++ queue.3 2 Mar 2012 05:29:24 -
@@ -966,7 +966,7 @@ TAILQ_FOREACH(np, head, entries)
for (np = n2; np != NULL; np = TAILQ_NEXT(np, entries))
np- ...
/* Delete. */
-while (np = TAILQ_FIRST(head)) {
+while ((np = TAILQ_FIRST(head)) != NULL) {
TAILQ_REMOVE(head, np, entries);
free(np);
}
