Re: follow up to 'once rule' expiration

2019-07-18 Thread Lawrence Teo
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

2019-05-22 Thread Lawrence Teo
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

2019-05-21 Thread Lawrence Teo
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

2019-05-19 Thread Lawrence Teo
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

2019-05-11 Thread Lawrence Teo
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

2019-03-24 Thread Lawrence Teo
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

2018-01-20 Thread Lawrence Teo
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

2018-01-18 Thread Lawrence Teo
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

2018-01-18 Thread Lawrence Teo
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

2017-08-27 Thread Lawrence Teo
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)

2016-12-21 Thread Lawrence Teo
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

2016-10-08 Thread Lawrence Teo
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

2015-09-12 Thread Lawrence Teo
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

2015-09-09 Thread Lawrence Teo
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

2015-09-09 Thread Lawrence Teo
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

2015-09-08 Thread Lawrence Teo
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

2015-09-03 Thread Lawrence Teo
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

2015-04-06 Thread Lawrence Teo
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

2015-03-04 Thread Lawrence Teo
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

2015-02-08 Thread Lawrence Teo
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

2015-01-14 Thread Lawrence Teo
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

2015-01-14 Thread Lawrence Teo
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()

2015-01-09 Thread Lawrence Teo
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

2014-12-30 Thread Lawrence Teo
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

2014-12-29 Thread Lawrence Teo
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

2014-12-15 Thread Lawrence Teo
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

2014-12-15 Thread Lawrence Teo
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

2014-07-12 Thread Lawrence Teo
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

2014-07-11 Thread Lawrence Teo
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

2014-07-11 Thread Lawrence Teo
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

2014-07-11 Thread Lawrence Teo
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

2014-07-09 Thread Lawrence Teo
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

2014-07-09 Thread Lawrence Teo
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

2014-07-08 Thread Lawrence Teo
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

2014-06-16 Thread Lawrence Teo
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

2014-06-13 Thread Lawrence Teo
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

2014-06-10 Thread Lawrence Teo
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

2014-06-03 Thread Lawrence Teo
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()

2014-03-17 Thread Lawrence Teo
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

2014-03-11 Thread Lawrence Teo
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

2014-03-09 Thread Lawrence Teo
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()

2014-03-07 Thread Lawrence Teo
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()

2014-03-06 Thread Lawrence Teo
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

2014-01-22 Thread Lawrence Teo
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

2014-01-09 Thread Lawrence Teo
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)

2014-01-06 Thread Lawrence Teo
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

2013-10-24 Thread Lawrence Teo
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

2013-10-24 Thread Lawrence Teo
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

2013-10-19 Thread Lawrence Teo
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

2013-10-18 Thread Lawrence Teo
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

2013-10-07 Thread Lawrence Teo
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

2013-08-19 Thread Lawrence Teo
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

2013-08-07 Thread Lawrence Teo
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

2013-08-05 Thread Lawrence Teo
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

2013-08-05 Thread Lawrence Teo
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

2013-07-02 Thread Lawrence Teo
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

2013-06-01 Thread Lawrence Teo
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()

2013-06-01 Thread Lawrence Teo
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

2013-05-22 Thread Lawrence Teo
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

2013-04-29 Thread Lawrence Teo
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

2013-04-18 Thread Lawrence Teo
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

2013-04-05 Thread Lawrence Teo
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

2013-03-29 Thread Lawrence Teo
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

2013-03-21 Thread Lawrence Teo
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

2013-03-13 Thread Lawrence Teo
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

2013-03-04 Thread Lawrence Teo
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()

2013-01-12 Thread Lawrence Teo
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

2012-12-16 Thread Lawrence Teo
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

2012-11-05 Thread Lawrence Teo
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

2012-11-03 Thread Lawrence Teo
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

2012-08-30 Thread Lawrence Teo
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

2012-08-20 Thread Lawrence Teo
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

2012-08-20 Thread Lawrence Teo
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

2012-08-14 Thread Lawrence Teo
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

2012-08-13 Thread Lawrence Teo
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

2012-08-13 Thread Lawrence Teo
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

2012-07-23 Thread Lawrence Teo
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

2012-07-08 Thread Lawrence Teo
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]

2012-07-07 Thread Lawrence Teo
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

2012-06-28 Thread Lawrence Teo
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

2012-06-14 Thread Lawrence Teo
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

2012-06-14 Thread Lawrence Teo
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

2012-05-30 Thread Lawrence Teo
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

2012-05-30 Thread Lawrence Teo
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

2012-05-28 Thread Lawrence Teo
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)

2012-05-15 Thread Lawrence Teo
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)

2012-05-15 Thread Lawrence Teo
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

2012-05-07 Thread Lawrence Teo
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

2012-05-07 Thread Lawrence Teo
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

2012-05-04 Thread Lawrence Teo
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

2012-04-27 Thread Lawrence Teo
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: remove redundant check

2012-04-26 Thread Lawrence Teo
This diff removes a redundant check from the pfctl_show_rules() function.

There is no functional change, and pfctl still passes all regression tests
with this change.

Comments?

Lawrence


Index: pfctl.c
===
RCS file: /cvs/src/sbin/pfctl/pfctl.c,v
retrieving revision 1.310
diff -U8 -p -r1.310 pfctl.c
--- pfctl.c 18 Apr 2012 14:42:17 -  1.310
+++ pfctl.c 26 Apr 2012 03:47:34 -
@@ -820,18 +820,16 @@ pfctl_show_rules(int dev, char *path, in
if (opts  PF_OPT_SHOWALL) {
pr.rule.action = PF_PASS;
if (ioctl(dev, DIOCGETRULES, pr)) {
warn(DIOCGETRULES);
ret = -1;
goto error;
}
header++;
-   }
-   if (opts  PF_OPT_SHOWALL) {
if (format == PFCTL_SHOW_RULES  (pr.nr  0 || header))
pfctl_print_title(FILTER RULES:);
else if (format == PFCTL_SHOW_LABELS  labels)
pfctl_print_title(LABEL COUNTERS:);
}
if (opts  PF_OPT_CLRRULECTRS)
pr.action = PF_GET_CLR_CNTR;



pfctl: fix printing of 'foo/*' anchors

2012-04-26 Thread Lawrence Teo
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

2012-04-24 Thread Lawrence Teo
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()

2012-04-19 Thread Lawrence Teo
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()

2012-04-18 Thread Lawrence Teo
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

2012-04-18 Thread Lawrence Teo
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

2012-04-18 Thread Lawrence Teo
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

2012-03-06 Thread Lawrence Teo
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

2012-03-04 Thread Lawrence Teo
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



  1   2   >