Re: use tasks and a task_list to manage if_addrhooks
syzkaller managed to trigger one of the assertions added by this
diff.
https://syzkaller.appspot.com/bug?id=2f4de8101553f64fcf847f8ed15cd1862b355122
On Thu, Nov 07, 2019 at 09:22:17PM +1000, David Gwynne wrote:
> this applies the use of tasks and a task_list to interface address
> hooks. it's like the detach and linkstate hooks, except it seems other
> things run the hooks more than things register hooks, and i can't tell
> if the places that run the hooks have the NET_LOCK or not. not by casual
> reading anyway.
>
> to cope with if_addrhooks_run maybe not being called with NET_LOCK being
> held, i made it safe to call the hook runner multiple times
> concurrently.
>
> one of the users of address hooks is pf, and the pfi_kif struct. it's
> part of the ABI, pfctl and snmpd use it, so i kept it using a void * and
> had it allocate the task separately. it should be as robust as it was
> before.
>
> everything else was pretty straightforward.
>
> tests? ok?
>
> Index: kern/kern_task.c
> ===
> RCS file: /cvs/src/sys/kern/kern_task.c,v
> retrieving revision 1.26
> diff -u -p -r1.26 kern_task.c
> --- kern/kern_task.c 23 Jun 2019 12:56:10 - 1.26
> +++ kern/kern_task.c 7 Nov 2019 11:21:00 -
> @@ -258,6 +258,8 @@ taskq_barrier_task(void *p)
> void
> task_set(struct task *t, void (*fn)(void *), void *arg)
> {
> + KASSERT(fn != NULL);
> +
> t->t_func = fn;
> t->t_arg = arg;
> t->t_flags = 0;
> Index: net/if.c
> ===
> RCS file: /cvs/src/sys/net/if.c,v
> retrieving revision 1.591
> diff -u -p -r1.591 if.c
> --- net/if.c 7 Nov 2019 08:03:18 - 1.591
> +++ net/if.c 7 Nov 2019 11:21:00 -
> @@ -630,9 +630,7 @@ if_attach_common(struct ifnet *ifp)
> ifp->if_iqs = ifp->if_rcv.ifiq_ifiqs;
> ifp->if_niqs = 1;
>
> - ifp->if_addrhooks = malloc(sizeof(*ifp->if_addrhooks),
> - M_TEMP, M_WAITOK);
> - TAILQ_INIT(ifp->if_addrhooks);
> + TAILQ_INIT(>if_addrhooks);
> TAILQ_INIT(>if_linkstatehooks);
> TAILQ_INIT(>if_detachhooks);
>
> @@ -1046,19 +1044,18 @@ if_netisr(void *unused)
> void
> if_hooks_run(struct task_list *hooks)
> {
> - struct task *t, *nt, cursor;
> + struct task *t, *nt;
> + struct task cursor = { .t_func = NULL };
> void (*func)(void *);
> void *arg;
>
> - /*
> - * holding the NET_LOCK guarantees that concurrent if_hooks_run
> - * calls can't happen, and they therefore can't try and call
> - * each others cursors as actual hooks.
> - */
> - NET_ASSERT_LOCKED();
> -
> mtx_enter(_hooks_mtx);
> for (t = TAILQ_FIRST(hooks); t != NULL; t = nt) {
> + while (t->t_func == NULL) { /* skip cursors */
> + t = TAILQ_NEXT(t, t_entry);
> + if (t == NULL)
> + break;
> + }
> func = t->t_func;
> arg = t->t_arg;
>
> @@ -1177,7 +1174,7 @@ if_detach(struct ifnet *ifp)
> }
> }
>
> - free(ifp->if_addrhooks, M_TEMP, sizeof(*ifp->if_addrhooks));
> + KASSERT(TAILQ_EMPTY(>if_addrhooks));
> KASSERT(TAILQ_EMPTY(>if_linkstatehooks));
> KASSERT(TAILQ_EMPTY(>if_detachhooks));
>
> @@ -3100,7 +3097,7 @@ ifnewlladdr(struct ifnet *ifp)
> ifa = _ifpforlinklocal(ifp, 0)->ia_ifa;
> if (ifa) {
> in6_purgeaddr(ifa);
> - dohooks(ifp->if_addrhooks, 0);
> + if_hooks_run(>if_addrhooks);
> in6_ifattach(ifp);
> }
> }
> @@ -3112,6 +3109,28 @@ ifnewlladdr(struct ifnet *ifp)
> (*ifp->if_ioctl)(ifp, SIOCSIFFLAGS, (caddr_t));
> }
> splx(s);
> +}
> +
> +void
> +if_addrhook_add(struct ifnet *ifp, struct task *t)
> +{
> + mtx_enter(_hooks_mtx);
> + TAILQ_INSERT_TAIL(>if_addrhooks, t, t_entry);
> + mtx_leave(_hooks_mtx);
> +}
> +
> +void
> +if_addrhook_del(struct ifnet *ifp, struct task *t)
> +{
> + mtx_enter(_hooks_mtx);
> + TAILQ_REMOVE(>if_addrhooks, t, t_entry);
> + mtx_leave(_hooks_mtx);
> +}
> +
> +void
> +if_addrhooks_run(struct ifnet *ifp)
> +{
> + if_hooks_run(>if_addrhooks);
> }
>
> int net_ticks;
> Index: net/if_pppx.c
> ===
> RCS file: /cvs/src/sys/net/if_pppx.c,v
> retrieving revision 1.68
> diff -u -p -r1.68 if_pppx.c
> --- net/if_pppx.c 24 Jun 2019 13:43:19 - 1.68
> +++ net/if_pppx.c 7 Nov 2019 11:21:00 -
> @@ -919,7 +919,7 @@ pppx_add_session(struct pppx_dev *pxd, s
> printf("pppx: unable to set addresses for %s, error=%d\n",
> ifp->if_xname, error);
> } else {
> - dohooks(ifp->if_addrhooks, 0);
> + if_addrhooks_run(ifp);
> }
> rw_enter_write(_ifs_lk);
>
iked(8): log reason when SA is freed
Currently we print a log message whenever an SA state is changed. When the SA
is deleted we print something like "sa_state: ... -> CLOSED", without giving
any context why it was dropped.
This diff adds a mechanism to specify the reason why the SA was freed.
The old output looks like something is broken:
spi=0x175f15ff82672003: sa_state: AUTH_SUCCESS -> ESTABLISHED from
10.0.1.33:500 to 10.0.1.34:500 policy 'test'
spi=0x5e3d6af7b2ea15ed: ikev2_ikesa_recv_delete: received delete
spi=0x5e3d6af7b2ea15ed: sa_state: CLOSING -> CLOSED from 10.0.1.33:500 to
10.0.1.34:500 policy 'test'
With the diff it is clear that the SA was freed because it has been rekeyed:
spi=0x175f15ff82672003: sa_state: AUTH_SUCCESS -> ESTABLISHED from
10.0.1.34:500 to 10.0.1.33:500 policy 'test'
spi=0x5e3d6af7b2ea15ed: ikev2_ikesa_delete: sent delete, closing SA
spi=0x5e3d6af7b2ea15ed: sa_state: ESTABLISHED -> CLOSED from 10.0.1.34:500 to
10.0.1.33:500 policy 'test'
spi=0x5e3d6af7b2ea15ed: sa_free: SA rekeyed
It's pretty useful for debugging because it helps to find out what went wrong.
ok?
diff --git a/sbin/iked/iked.h b/sbin/iked/iked.h
index 1717723a08d..a9c71178f1e 100644
--- a/sbin/iked/iked.h
+++ b/sbin/iked/iked.h
@@ -412,6 +412,7 @@ struct iked_sa {
struct timeval sa_timeused;
char*sa_tag;
+ char*sa_reason; /* reason for close */
struct iked_kex sa_kex;
/* XXX compat defines until everything is converted */
@@ -820,6 +821,7 @@ int ikev2_childsa_delete(struct iked *, struct iked_sa *,
uint8_t, uint64_t, uint64_t *, int);
voidikev2_ikesa_recv_delete(struct iked *, struct iked_sa *);
voidikev2_ike_sa_timeout(struct iked *env, void *);
+voidikev2_ike_sa_setreason(struct iked_sa *, char *);
struct ibuf *
ikev2_prfplus(struct iked_hash *, struct ibuf *, struct ibuf *,
diff --git a/sbin/iked/ikev2.c b/sbin/iked/ikev2.c
index f6148be352b..514cf40a268 100644
--- a/sbin/iked/ikev2.c
+++ b/sbin/iked/ikev2.c
@@ -499,6 +499,7 @@ ikev2_recv(struct iked *env, struct iked_message *msg)
(ibuf_length(msg->msg_data) != ibuf_length(sa->sa_1stmsg) ||
memcmp(ibuf_data(msg->msg_data), ibuf_data(sa->sa_1stmsg),
ibuf_length(sa->sa_1stmsg)) != 0)) {
+ ikev2_ike_sa_setreason(sa, NULL);
sa_free(env, sa);
msg->msg_sa = sa = NULL;
goto done;
@@ -515,6 +516,8 @@ ikev2_recv(struct iked *env, struct iked_message *msg)
if (r == -1) {
log_warn("%s: failed to retransmit a "
"response", __func__);
+ ikev2_ike_sa_setreason(sa,
+ "retransmitting response failed");
sa_free(env, sa);
}
return;
@@ -558,6 +561,7 @@ done:
if (sa != NULL && sa->sa_state == IKEV2_STATE_CLOSED) {
log_debug("%s: closing SA", __func__);
+ ikev2_ike_sa_setreason(sa, "closed");
sa_free(env, sa);
}
}
@@ -813,8 +817,11 @@ ikev2_init_recv(struct iked *env, struct iked_message *msg,
betoh64(hdr->ike_ispi), betoh64(hdr->ike_rspi), 1,
NULL)) == NULL || sa != msg->msg_sa) {
log_debug("%s: invalid new SA", __func__);
- if (sa)
+ if (sa) {
+ ikev2_ike_sa_setreason(sa, "invalid new SA");
sa_free(env, sa);
+ sa = NULL;
+ }
}
break;
case IKEV2_EXCHANGE_IKE_AUTH:
@@ -928,6 +935,7 @@ ikev2_init_ike_sa_timeout(struct iked *env, void *arg)
print_spi(sa->sa_hdr.sh_ispi, 8),
print_spi(sa->sa_hdr.sh_rspi, 8));
+ ikev2_ike_sa_setreason(sa, "SA_INIT timeout");
sa_free(env, sa);
}
@@ -1103,6 +,7 @@ ikev2_init_ike_sa_peer(struct iked *env, struct
iked_policy *pol,
closeonly:
if (ret == -1) {
log_debug("%s: closing SA", __func__);
+ ikev2_ike_sa_setreason(sa, "failed to send SA_INIT");
sa_free(env, sa);
}
@@ -2366,11 +2375,13 @@ ikev2_resp_recv(struct iked *env, struct iked_message
*msg,
if (msg->msg_error == 0)
msg->msg_error = IKEV2_N_NO_PROPOSAL_CHOSEN;
ikev2_send_init_error(env, msg);
+ ikev2_ike_sa_setreason(sa, "no proposal chosen");
sa_state(env, sa, IKEV2_STATE_CLOSED);
return;
}
if (ikev2_resp_ike_sa_init(env, msg) != 0) {
Re: fix xhci 'actlen' calculation
On Tue, Nov 12, 2019 at 07:00:00PM +0100, Patrick Wildt wrote:
> On Tue, Nov 12, 2019 at 01:43:28PM +0100, Patrick Wildt wrote:
> > On Tue, Nov 12, 2019 at 10:45:39AM +0100, Gerhard Roth wrote:
> > > Hi,
> > >
> > > xhci's calculation of 'xfer->actlen' is wrong if the xfer was split into
> > > multiple TRBs. That's because the code just looks at the remainder
> > > reported by the status TRB. However, this remainder only refers to the
> > > total size of this single TRB; not to the total size of the xfer.
> > >
> > > Example: assume a 16 kb xfer is split into two TRBs for 8 kb each.
> > > Then we get a short read of 6 kb. The status TRB will refer
> > > to the first TRB with remainder set to 2 kb. Currently xhci
> > > would assume that actlen is 'xfer->length' (16 kb) minus the
> > > remainder (2 kb). That yields a wrong actlen of 14 kb instead
> > > of 6 kb.
> > >
> > > The patch below fixes this by adding up all the remainders of all the
> > > transfer TRBs up to the current one.
> > >
> > > Gerhard
> > >
> >
> > Very goood catch, this is very similar to the issue we had with isoc
> > transfers. Diff looks good to me!
>
> Unfortunately this is *not* ok. It turns out that this doesn't work
> for short ctrl transfers. Compared to bulk/intr transfers, ctrl xfers
> have a setup, data and status trb. Now your diff will sum all TRBs,
> but the thing is that only the data TRB is actually short. :( But I
> think this should be easily fixable, I'll have a look.
This changes the diff so that it only sums actual data TRBs (NORMAL and
DATA).
That makes my umb(4) device behave again.
Patrick
diff --git a/sys/dev/usb/xhci.c b/sys/dev/usb/xhci.c
index f9d7eb3a401..c801bd96ad1 100644
--- a/sys/dev/usb/xhci.c
+++ b/sys/dev/usb/xhci.c
@@ -810,6 +810,29 @@ xhci_event_xfer(struct xhci_softc *sc, uint64_t paddr,
uint32_t status,
xhci_xfer_done(xfer);
}
+uint32_t
+xhci_xfer_length_generic(struct xhci_xfer *xx, struct xhci_pipe *xp,
+int trb_idx)
+{
+ int trb0_idx;
+ uint32_t len = 0, type;
+
+ trb0_idx =
+ ((xx->index + xp->ring.ntrb) - xx->ntrb) % (xp->ring.ntrb - 1);
+
+ while (1) {
+ type = xp->ring.trbs[trb0_idx].trb_flags & XHCI_TRB_TYPE_MASK;
+ if (type == XHCI_TRB_TYPE_NORMAL || type == XHCI_TRB_TYPE_DATA)
+ len += le32toh(XHCI_TRB_LEN(
+ xp->ring.trbs[trb0_idx].trb_status));
+ if (trb0_idx == trb_idx)
+ break;
+ if (++trb0_idx == xp->ring.ntrb)
+ trb0_idx = 0;
+ }
+ return len;
+}
+
int
xhci_event_xfer_generic(struct xhci_softc *sc, struct usbd_xfer *xfer,
struct xhci_pipe *xp, uint32_t remain, int trb_idx,
@@ -819,16 +842,22 @@ xhci_event_xfer_generic(struct xhci_softc *sc, struct
usbd_xfer *xfer,
switch (code) {
case XHCI_CODE_SUCCESS:
- /*
-* This might be the last TRB of a TD that ended up
-* with a Short Transfer condition, see below.
-*/
- if (xfer->actlen == 0)
- xfer->actlen = xfer->length - remain;
+ if (xfer->actlen == 0) {
+ if (remain)
+ xfer->actlen =
+ xhci_xfer_length_generic(xx, xp, trb_idx) -
+ remain;
+ else
+ xfer->actlen = xfer->length;
+ }
xfer->status = USBD_NORMAL_COMPLETION;
break;
case XHCI_CODE_SHORT_XFER:
- xfer->actlen = xfer->length - remain;
+ /*
+* Use values from the transfer TRB instead of the status TRB.
+*/
+ xfer->actlen = xhci_xfer_length_generic(xx, xp, trb_idx) -
+ remain;
/*
* If this is not the last TRB of a transfer, we should
* theoretically clear the IOC at the end of the chain
Re: sysupgrade: Allow to use another directory for the sets
On 12/11/2019 08:29, Theo de Raadt wrote:
Renaud, please test it for me like this:
sysupgrade -d /
This interface is dangerously incorrect.
What about this one?
Index: sysupgrade.8
===
RCS file: /cvs/src/usr.sbin/sysupgrade/sysupgrade.8,v
retrieving revision 1.10
diff -u -p -r1.10 sysupgrade.8
--- sysupgrade.83 Oct 2019 12:43:58 - 1.10
+++ sysupgrade.812 Nov 2019 18:01:04 -
@@ -24,6 +24,7 @@
.Nm
.Op Fl fkn
.Op Fl r | s
+.Op Fl d Ar directory
.Op Ar installurl
.Sh DESCRIPTION
.Nm
@@ -48,6 +49,13 @@ triggering a one-shot upgrade using the
.Pp
The options are as follows:
.Bl -tag -width Ds
+.It Fl d Ar directory
+Choose the prefix of the
+.Ar directory
+in which the sets will be downloaded.
+_sysupgrade will be appended to that name.
+Default is
+.Pa /home .
.It Fl f
Force an already applied upgrade.
The default is to upgrade to latest snapshot only if available.
Index: sysupgrade.sh
===
RCS file: /cvs/src/usr.sbin/sysupgrade/sysupgrade.sh,v
retrieving revision 1.32
diff -u -p -r1.32 sysupgrade.sh
--- sysupgrade.sh 11 Nov 2019 18:26:52 - 1.32
+++ sysupgrade.sh 12 Nov 2019 18:01:04 -
@@ -25,7 +25,6 @@ umask 0022
export PATH=/usr/bin:/bin:/usr/sbin:/sbin
ARCH=$(uname -m)
-SETSDIR=/home/_sysupgrade
ug_err()
{
@@ -34,7 +33,7 @@ ug_err()
usage()
{
- ug_err "usage: ${0##*/} [-fkn] [-r | -s] [installurl]"
+ ug_err "usage: ${0##*/} [-fkn] [-r | -s] [-d directory] [installurl]"
}
unpriv()
@@ -73,14 +72,16 @@ rmel() {
echo -n "$_c"
}
+SETSDIR=/home/_sysupgrade
RELEASE=false
SNAP=false
FORCE=false
KEEP=false
REBOOT=true
-while getopts fknrs arg; do
+while getopts d:fknrs arg; do
case ${arg} in
+ d) SETSDIR=${OPTARG}/_sysupgrade;;
f) FORCE=true;;
k) KEEP=true;;
n) REBOOT=false;;
@@ -195,7 +196,7 @@ ${KEEP} && > keep
cat <<__EOT >/auto_upgrade.conf
Location of sets = disk
-Pathname to the sets = /home/_sysupgrade/
+Pathname to the sets = ${SETSDIR}
Set name(s) = done
Directory does not contain SHA256.sig. Continue without verification = yes
__EOT
@@ -203,7 +204,7 @@ __EOT
if ! ${KEEP}; then
CLEAN=$(echo SHA256 ${SETS} | sed -e 's/ /,/g')
cat <<__EOT > /etc/rc.firsttime
-rm -f /home/_sysupgrade/{${CLEAN}}
+rm -f ${SETSDIR}/{${CLEAN}}
__EOT
fi
Re: fix xhci 'actlen' calculation
On Tue, Nov 12, 2019 at 01:43:28PM +0100, Patrick Wildt wrote: > On Tue, Nov 12, 2019 at 10:45:39AM +0100, Gerhard Roth wrote: > > Hi, > > > > xhci's calculation of 'xfer->actlen' is wrong if the xfer was split into > > multiple TRBs. That's because the code just looks at the remainder > > reported by the status TRB. However, this remainder only refers to the > > total size of this single TRB; not to the total size of the xfer. > > > > Example: assume a 16 kb xfer is split into two TRBs for 8 kb each. > > Then we get a short read of 6 kb. The status TRB will refer > > to the first TRB with remainder set to 2 kb. Currently xhci > > would assume that actlen is 'xfer->length' (16 kb) minus the > > remainder (2 kb). That yields a wrong actlen of 14 kb instead > > of 6 kb. > > > > The patch below fixes this by adding up all the remainders of all the > > transfer TRBs up to the current one. > > > > Gerhard > > > > Very goood catch, this is very similar to the issue we had with isoc > transfers. Diff looks good to me! Unfortunately this is *not* ok. It turns out that this doesn't work for short ctrl transfers. Compared to bulk/intr transfers, ctrl xfers have a setup, data and status trb. Now your diff will sum all TRBs, but the thing is that only the data TRB is actually short. :( But I think this should be easily fixable, I'll have a look.
unwind(8): refactor & simplify refcounting
Did I get this right? I'd appreciate it if someone could give this a
once over.
Since resolve() switched to a callback mechanism all uw_resolver objects
pass through resolve() and either asr_resolve_done() or
ub_resolve_done().
With that we can pull resolver_ref() and resolver_unref() into those
functions to make the reference counting easier.
Only check_resolver is special since it needs to refcount the to be
checked resolver. But the resolver doing the actual work is
automatically refcounted by resolve() and *_resolve_done().
One last piece of the puzzle is to track the uw_resolver object in
cb_data so that the *_resolve_done() functions have access to it.
This also allowes us to remove the ad-hoc passing of the resolver in
query_imsg. Since the callback functions all need access to the
resolver that did the work we pass it in as first argument.
diff --git resolver.c resolver.c
index d1dce2dec71..2b7d81d29fc 100644
--- resolver.c
+++ resolver.c
@@ -92,14 +92,11 @@ struct uw_resolver {
int64_t histogram[nitems(histogram_limits)];
};
-struct check_resolver_data {
- struct uw_resolver *res;
- struct uw_resolver *check_res;
-};
-
struct resolver_cb_data {
- void(*cb)(void *, int, void *, int, int, char *);
- void*data;
+ void(*cb)(struct uw_resolver *, void *, int, void *,
+ int, int, char *);
+ void*data;
+ struct uw_resolver *res;
};
__dead void resolver_shutdown(void);
@@ -108,9 +105,10 @@ void
resolver_dispatch_frontend(int, short, void *);
voidresolver_dispatch_captiveportal(int, short, void *);
voidresolver_dispatch_main(int, short, void *);
int resolve(struct uw_resolver *, const char*, int, int,
-void*, void (*cb)(void *, int, void *, int, int,
-char *));
-voidresolve_done(void *, int, void *, int, int, char *);
+void*, void (*cb)(struct uw_resolver *, void *,
+int, void *, int, int, char *));
+voidresolve_done(struct uw_resolver *, void *, int, void *,
+int, int, char *);
voidub_resolve_done(void *, int, void *, int, int, char *,
int);
voidasr_resolve_done(struct asr_result *, void *);
@@ -129,8 +127,8 @@ void set_forwarders_oppdot(struct
uw_resolver *,
voidresolver_check_timo(int, short, void *);
voidresolver_free_timo(int, short, void *);
voidcheck_resolver(struct uw_resolver *);
-voidcheck_resolver_done(void *, int, void *, int, int,
-char *);
+voidcheck_resolver_done(struct uw_resolver *, void *, int,
+void *, int, int, char *);
voidschedule_recheck_all_resolvers(void);
int check_forwarders_changed(struct uw_forwarder_head *,
struct uw_forwarder_head *);
@@ -154,8 +152,8 @@ int check_captive_portal_changed(struct
uw_conf *,
struct uw_conf *);
voidtrust_anchor_resolve(void);
voidtrust_anchor_timo(int, short, void *);
-voidtrust_anchor_resolve_done(void *, int, void *, int,
-int, char *);
+voidtrust_anchor_resolve_done(struct uw_resolver *, void *,
+int, void *, int, int, char *);
voidadd_autoconf_forwarders(struct imsg_rdns_proposal *);
voidrem_autoconf_forwarders(struct imsg_rdns_proposal *);
struct uw_forwarder*find_forwarder(struct uw_forwarder_head *,
@@ -480,14 +478,10 @@ resolver_dispatch_frontend(int fd, short event, void
*bula)
log_debug("%s: choosing %s", __func__,
uw_resolver_type_str[res->type]);
- query_imsg->resolver = res;
- resolver_ref(res);
-
clock_gettime(CLOCK_MONOTONIC, _imsg->tp);
- if ((resolve(res, query_imsg->qname, query_imsg->t,
- query_imsg->c, query_imsg, resolve_done)) != 0)
- resolver_unref(res);
+ resolve(res, query_imsg->qname, query_imsg->t,
+ query_imsg->c, query_imsg, resolve_done);
break;
case IMSG_FORWARDER:
/* make sure this is a string */
@@ -773,16 +767,20 @@ resolver_dispatch_main(int fd, short event,
Re: iked(8): add configuration option for esn
On Tue, Nov 12, 2019 at 04:07:51PM +0100, Tobias Heider wrote:
> Makes sense. Here is the updated diff including a fix for bluhms
> comment.
OK bluhm@
> Index: iked.conf.5
> ===
> RCS file: /mount/openbsd/cvs/src/sbin/iked/iked.conf.5,v
> retrieving revision 1.57
> diff -u -p -r1.57 iked.conf.5
> --- iked.conf.5 24 Aug 2019 13:24:49 - 1.57
> +++ iked.conf.5 12 Nov 2019 13:49:37 -
> @@ -409,6 +409,7 @@ multiple crypto transforms.
> .Ic auth Ar algorithm
> .Ic enc Ar algorithm
> .Ic group Ar group
> +.Ic esn
> .Xc
> These parameters define the cryptographic transforms to be used for
> the Child SA negotiation, also known as phase 2.
> @@ -421,6 +422,7 @@ Possible values for
> .Ic auth ,
> .Ic enc ,
> .Ic group ,
> +.Ic esn ,
> and the default proposals are described below in
> .Sx CRYPTO TRANSFORMS .
> If omitted,
> @@ -849,6 +851,17 @@ not encryption:
> .It Li aes-192-gmac Ta "224 bits" Ta "[ESP only]"
> .It Li aes-256-gmac Ta "288 bits" Ta "[ESP only]"
> .It Li null Ta "" Ta "[ESP only]"
> +.El
> +.Pp
> +The Extended Sequence Numbers option can be enabled or disabled with the
> +.Ic esn
> +or
> +.Ic noesn
> +keywords:
> +.Bl -column "noesn" "[ESP only]" -offset indent
> +.It Em ESN
> +.It Li esn Ta "[ESP only]"
> +.It Li noesn Ta "[ESP only]"
> .El
> .Pp
> Transforms followed by
> Index: parse.y
> ===
> RCS file: /mount/openbsd/cvs/src/sbin/iked/parse.y,v
> retrieving revision 1.84
> diff -u -p -r1.84 parse.y
> --- parse.y 26 Sep 2019 07:33:36 - 1.84
> +++ parse.y 12 Nov 2019 13:54:04 -
> @@ -127,6 +127,8 @@ struct ipsec_transforms {
> unsigned int nencxf;
> const struct ipsec_xf **groupxf;
> unsigned int ngroupxf;
> + const struct ipsec_xf **esnxf;
> + unsigned int nesnxf;
> };
>
> struct ipsec_mode {
> @@ -259,6 +261,12 @@ const struct ipsec_xf groupxfs[] = {
> { NULL }
> };
>
> +const struct ipsec_xf esnxfs[] = {
> + { "esn",IKEV2_XFORMESN_ESN },
> + { "noesn", IKEV2_XFORMESN_NONE },
> + { NULL }
> +};
> +
> const struct ipsec_xf methodxfs[] = {
> { "none", IKEV2_AUTH_NONE },
> { "rsa",IKEV2_AUTH_RSA_SIG },
> @@ -395,7 +403,7 @@ typedef struct {
> %}
>
> %token FROM ESP AH IN PEER ON OUT TO SRCID DSTID PSK PORT
> -%token FILENAME AUTHXF PRFXF ENCXF ERROR IKEV2 IKESA CHILDSA
> +%token FILENAME AUTHXF PRFXF ENCXF ERROR IKEV2 IKESA CHILDSA ESN NOESN
> %token PASSIVE ACTIVE ANY TAG TAP PROTO LOCAL GROUP NAME CONFIG EAP
> USER
> %token IKEV1 FLOW SA TCPMD5 TUNNEL TRANSPORT COUPLE DECOUPLE SET
> %token INCLUDE LIFETIME BYTES INET INET6 QUICK SKIP DEFAULT
> @@ -425,6 +433,7 @@ typedef struct {
> %type byte_spec time_spec ikelifetime
> %type name
> %type cfg ikecfg ikecfgvals
> +%type transform_esn
> %%
>
> grammar : /* empty */
> @@ -802,6 +811,24 @@ transform: AUTHXF STRING {
> ipsec_transforms->groupxf = xfs;
> ipsec_transforms->ngroupxf++;
> }
> + | transform_esn {
> + const struct ipsec_xf **xfs = ipsec_transforms->esnxf;
> + size_t nxfs = ipsec_transforms->nesnxf;
> + xfs = recallocarray(xfs, nxfs, nxfs + 1,
> + sizeof(struct ipsec_xf *));
> + if (xfs == NULL)
> + err(1, "transform: recallocarray");
> + if ((xfs[nxfs] = parse_xf($1, 0, esnxfs)) == NULL) {
> + yyerror("%s not a valid transform", $1);
> + YYERROR;
> + }
> + ipsec_transforms->esnxf = xfs;
> + ipsec_transforms->nesnxf++;
> + }
> + ;
> +
> +transform_esn: ESN { $$ = "esn"; }
> + | NOESN { $$ = "noesn"; }
> ;
>
> ike_sas : {
> @@ -1180,6 +1207,7 @@ lookup(char *s)
> { "dstid", DSTID },
> { "eap",EAP },
> { "enc",ENCXF },
> + { "esn",ESN },
> { "esp",ESP },
> { "file", FILENAME },
> { "flow", FLOW },
> @@ -1198,6 +1226,7 @@ lookup(char *s)
> { "local", LOCAL },
> { "mobike", MOBIKE },
> { "name", NAME },
> + { "noesn", NOESN },
> {
Re: iked(8): add configuration option for esn
On Tue, 12 Nov 2019 at 16:08, Tobias Heider wrote: > On Tue, Nov 12, 2019 at 09:57:31AM +0100, Mike Belopuhov wrote: > > Hi Tobias, > > > > I see, however, I don't think iked would negotiate an SA > > without ESN support if the other side supports ESN, so I'm > > not sure how "enforcing" changes that. > > It doesn't, but if I have an iked on both sides one will have to > make the decision. I have another case where I actually can not > use ESN, with two ikeds this can not be configured currently. > > > In any case, I'm not opposed to adding a toggle if you guys > > need it, but could you please adjust the grammar so that "esn" > > and "no esn" are used instead of "on" and "off" since that's > > what we're normally doing. "on" and "off" are clutches for > > simple file formats, parse.y allows you to make it a bit nicer. > > Makes sense. Here is the updated diff including a fix for bluhms > comment. > > While I meant "no esn" with a space, I see that you and Patrick have been adding things like "nofragmentation" and "nomobike". These should be written with a space as well. Nevertheless ok mikeb, hopefully you'll come around fixing grammar later on.
Re: iked(8): add configuration option for esn
On Tue, Nov 12, 2019 at 09:57:31AM +0100, Mike Belopuhov wrote:
> Hi Tobias,
>
> I see, however, I don't think iked would negotiate an SA
> without ESN support if the other side supports ESN, so I'm
> not sure how "enforcing" changes that.
It doesn't, but if I have an iked on both sides one will have to
make the decision. I have another case where I actually can not
use ESN, with two ikeds this can not be configured currently.
> In any case, I'm not opposed to adding a toggle if you guys
> need it, but could you please adjust the grammar so that "esn"
> and "no esn" are used instead of "on" and "off" since that's
> what we're normally doing. "on" and "off" are clutches for
> simple file formats, parse.y allows you to make it a bit nicer.
Makes sense. Here is the updated diff including a fix for bluhms
comment.
Index: iked.conf.5
===
RCS file: /mount/openbsd/cvs/src/sbin/iked/iked.conf.5,v
retrieving revision 1.57
diff -u -p -r1.57 iked.conf.5
--- iked.conf.5 24 Aug 2019 13:24:49 - 1.57
+++ iked.conf.5 12 Nov 2019 13:49:37 -
@@ -409,6 +409,7 @@ multiple crypto transforms.
.Ic auth Ar algorithm
.Ic enc Ar algorithm
.Ic group Ar group
+.Ic esn
.Xc
These parameters define the cryptographic transforms to be used for
the Child SA negotiation, also known as phase 2.
@@ -421,6 +422,7 @@ Possible values for
.Ic auth ,
.Ic enc ,
.Ic group ,
+.Ic esn ,
and the default proposals are described below in
.Sx CRYPTO TRANSFORMS .
If omitted,
@@ -849,6 +851,17 @@ not encryption:
.It Li aes-192-gmac Ta "224 bits" Ta "[ESP only]"
.It Li aes-256-gmac Ta "288 bits" Ta "[ESP only]"
.It Li null Ta "" Ta "[ESP only]"
+.El
+.Pp
+The Extended Sequence Numbers option can be enabled or disabled with the
+.Ic esn
+or
+.Ic noesn
+keywords:
+.Bl -column "noesn" "[ESP only]" -offset indent
+.It Em ESN
+.It Li esn Ta "[ESP only]"
+.It Li noesn Ta "[ESP only]"
.El
.Pp
Transforms followed by
Index: parse.y
===
RCS file: /mount/openbsd/cvs/src/sbin/iked/parse.y,v
retrieving revision 1.84
diff -u -p -r1.84 parse.y
--- parse.y 26 Sep 2019 07:33:36 - 1.84
+++ parse.y 12 Nov 2019 13:54:04 -
@@ -127,6 +127,8 @@ struct ipsec_transforms {
unsigned int nencxf;
const struct ipsec_xf **groupxf;
unsigned int ngroupxf;
+ const struct ipsec_xf **esnxf;
+ unsigned int nesnxf;
};
struct ipsec_mode {
@@ -259,6 +261,12 @@ const struct ipsec_xf groupxfs[] = {
{ NULL }
};
+const struct ipsec_xf esnxfs[] = {
+ { "esn",IKEV2_XFORMESN_ESN },
+ { "noesn", IKEV2_XFORMESN_NONE },
+ { NULL }
+};
+
const struct ipsec_xf methodxfs[] = {
{ "none", IKEV2_AUTH_NONE },
{ "rsa",IKEV2_AUTH_RSA_SIG },
@@ -395,7 +403,7 @@ typedef struct {
%}
%token FROM ESP AH IN PEER ON OUT TO SRCID DSTID PSK PORT
-%token FILENAME AUTHXF PRFXF ENCXF ERROR IKEV2 IKESA CHILDSA
+%token FILENAME AUTHXF PRFXF ENCXF ERROR IKEV2 IKESA CHILDSA ESN NOESN
%token PASSIVE ACTIVE ANY TAG TAP PROTO LOCAL GROUP NAME CONFIG EAP USER
%token IKEV1 FLOW SA TCPMD5 TUNNEL TRANSPORT COUPLE DECOUPLE SET
%token INCLUDE LIFETIME BYTES INET INET6 QUICK SKIP DEFAULT
@@ -425,6 +433,7 @@ typedef struct {
%typebyte_spec time_spec ikelifetime
%typename
%type cfg ikecfg ikecfgvals
+%typetransform_esn
%%
grammar: /* empty */
@@ -802,6 +811,24 @@ transform : AUTHXF STRING {
ipsec_transforms->groupxf = xfs;
ipsec_transforms->ngroupxf++;
}
+ | transform_esn {
+ const struct ipsec_xf **xfs = ipsec_transforms->esnxf;
+ size_t nxfs = ipsec_transforms->nesnxf;
+ xfs = recallocarray(xfs, nxfs, nxfs + 1,
+ sizeof(struct ipsec_xf *));
+ if (xfs == NULL)
+ err(1, "transform: recallocarray");
+ if ((xfs[nxfs] = parse_xf($1, 0, esnxfs)) == NULL) {
+ yyerror("%s not a valid transform", $1);
+ YYERROR;
+ }
+ ipsec_transforms->esnxf = xfs;
+ ipsec_transforms->nesnxf++;
+ }
+ ;
+
+transform_esn : ESN { $$ = "esn"; }
+ | NOESN { $$ = "noesn"; }
;
ike_sas: {
@@ -1180,6 +1207,7 @@ lookup(char *s)
{ "dstid", DSTID },
{ "eap",EAP },
{ "enc",ENCXF },
+
smtpd: add support for cidr in hostname resolution for spf walk
Hello,
Here's a patch for smtpctl spf resolution, adding support for target
specified as a hostname + cidr.
Yes, SPF lets you specify targets like a:example.com/24.
Due to the async and recursive nature of DNS resolution in spfwalk.c,
it's kind of hard to pass data around without too much memory
allocation, so I decided to just pass cidr(s) as integer value instead
of keeping the original string and passing pointers around.
Comments welcome, thanks!
Index: usr.sbin/smtpd/spfwalk.c
===
RCS file: /var/cvs/src/usr.sbin/smtpd/spfwalk.c,v
retrieving revision 1.15
diff -u -r1.15 spfwalk.c
--- usr.sbin/smtpd/spfwalk.c11 Nov 2019 17:20:25 - 1.15
+++ usr.sbin/smtpd/spfwalk.c12 Nov 2019 12:43:25 -
@@ -40,15 +40,24 @@
#include "unpack_dns.h"
#include "parser.h"
+struct sender {
+ void(*cb)(struct dns_rr *, struct sender *);
+ char *target;
+ int cidr4;
+ int cidr6;
+};
+
+void *xmalloc(size_t);
int spfwalk(int, struct parameter *);
-static voiddispatch_txt(struct dns_rr *);
-static voiddispatch_mx(struct dns_rr *);
-static voiddispatch_a(struct dns_rr *);
-static voiddispatch_(struct dns_rr *);
-static voidlookup_record(int, const char *, void (*)(struct dns_rr *));
+static voiddispatch_txt(struct dns_rr *, struct sender *);
+static voiddispatch_mx(struct dns_rr *, struct sender *);
+static voiddispatch_a(struct dns_rr *, struct sender *);
+static voiddispatch_(struct dns_rr *, struct sender *);
+static voidlookup_record(int, struct sender *);
static voiddispatch_record(struct asr_result *, void *);
static ssize_t parse_txt(const char *, size_t, char *, size_t);
+static int parse_sender(struct sender *);
int ip_v4 = 0;
int ip_v6 = 0;
@@ -59,6 +68,7 @@
int
spfwalk(int argc, struct parameter *argv)
{
+ struct sendersdr;
const char *ip_family = NULL;
char*line = NULL;
size_t linesize = 0;
@@ -81,12 +91,17 @@
dict_init();
event_init();
+ sdr.cidr4 = sdr.cidr6 = -1;
+ sdr.cb = dispatch_txt;
+
while ((linelen = getline(, , stdin)) != -1) {
while (linelen-- > 0 && isspace(line[linelen]))
line[linelen] = '\0';
- if (linelen > 0)
- lookup_record(T_TXT, line, dispatch_txt);
+ if (linelen > 0) {
+ sdr.target = line;
+ lookup_record(T_TXT, );
+ }
}
free(line);
@@ -100,20 +115,23 @@
}
void
-lookup_record(int type, const char *record, void (*cb)(struct dns_rr *))
+lookup_record(int type, struct sender *sdr)
{
struct asr_query *as;
+ struct sender *nsdr;
- as = res_query_async(record, C_IN, type, NULL);
+ as = res_query_async(sdr->target, C_IN, type, NULL);
if (as == NULL)
err(1, "res_query_async");
- event_asr_run(as, dispatch_record, cb);
+ nsdr = xmalloc(sizeof(*nsdr));
+ *nsdr = *sdr;
+ event_asr_run(as, dispatch_record, (void *)nsdr);
}
void
dispatch_record(struct asr_result *ar, void *arg)
{
- void (*cb)(struct dns_rr *) = arg;
+ struct sender *sdr = arg;
struct unpack pack;
struct dns_header h;
struct dns_query q;
@@ -121,7 +139,7 @@
/* best effort */
if (ar->ar_h_errno && ar->ar_h_errno != NO_DATA)
- return;
+ goto end;
unpack_init(, ar->ar_data, ar->ar_datalen);
unpack_header(, );
@@ -130,19 +148,22 @@
for (; h.ancount; h.ancount--) {
unpack_rr(, );
/**/
- cb();
+ sdr->cb(, sdr);
}
+end:
+ free(sdr);
}
void
-dispatch_txt(struct dns_rr *rr)
+dispatch_txt(struct dns_rr *rr, struct sender *sdr)
{
+ char buf[4096];
+ char *argv[512];
+ char buf2[512];
+ struct sender lsdr;
struct in6_addr ina;
-char buf[4096];
-char buf2[512];
-char *in = buf;
-char *argv[512];
-char **ap = argv;
+ char **ap = argv;
+ char *in = buf;
char *end;
ssize_t n;
@@ -173,6 +194,8 @@
if (**ap == '+' || **ap == '?')
(*ap)++;
+ lsdr.cidr4 = lsdr.cidr6 = -1;
+
if (strncasecmp("ip4:", *ap, 4) == 0) {
if ((ip_v4 == 1 || ip_both == 1) &&
inet_net_pton(AF_INET, *(ap) + 4,
@@ -190,35 +213,55 @@
if (strcasecmp("a", *ap) == 0) {
print_dname(rr->rr_dname, buf2, sizeof(buf2));
buf2[strlen(buf2) - 1] = '\0';
- lookup_record(T_A, buf2, dispatch_a);
- lookup_record(T_, buf2, dispatch_);
+
Re: fix xhci 'actlen' calculation
On Tue, Nov 12, 2019 at 10:45:39AM +0100, Gerhard Roth wrote:
> Hi,
>
> xhci's calculation of 'xfer->actlen' is wrong if the xfer was split into
> multiple TRBs. That's because the code just looks at the remainder
> reported by the status TRB. However, this remainder only refers to the
> total size of this single TRB; not to the total size of the xfer.
>
> Example: assume a 16 kb xfer is split into two TRBs for 8 kb each.
> Then we get a short read of 6 kb. The status TRB will refer
> to the first TRB with remainder set to 2 kb. Currently xhci
> would assume that actlen is 'xfer->length' (16 kb) minus the
> remainder (2 kb). That yields a wrong actlen of 14 kb instead
> of 6 kb.
>
> The patch below fixes this by adding up all the remainders of all the
> transfer TRBs up to the current one.
>
> Gerhard
>
Very goood catch, this is very similar to the issue we had with isoc
transfers. Diff looks good to me!
>
> Index: sys/dev/usb/xhci.c
> ===
> RCS file: /cvs/src/sys/dev/usb/xhci.c,v
> retrieving revision 1.106
> diff -u -p -u -p -r1.106 xhci.c
> --- sys/dev/usb/xhci.c6 Oct 2019 17:30:00 - 1.106
> +++ sys/dev/usb/xhci.c12 Nov 2019 09:29:47 -
> @@ -810,6 +810,28 @@ xhci_event_xfer(struct xhci_softc *sc, u
> xhci_xfer_done(xfer);
> }
>
> +uint32_t
> +xhci_xfer_length_generic(struct xhci_xfer *xx, struct xhci_pipe *xp,
> +int trb_idx)
> +{
> + int trb0_idx;
> + uint32_t len = 0;
> +
> + KASSERT(xx->index >= 0);
> + trb0_idx =
> + ((xx->index + xp->ring.ntrb) - xx->ntrb) % (xp->ring.ntrb - 1);
> +
> + while (1) {
> + len +=
> + le32toh(XHCI_TRB_LEN(xp->ring.trbs[trb0_idx].trb_status));
> + if (trb0_idx == trb_idx)
> + break;
> + if (++trb0_idx == xp->ring.ntrb)
> + trb0_idx = 0;
> + }
> + return len;
> +}
> +
> int
> xhci_event_xfer_generic(struct xhci_softc *sc, struct usbd_xfer *xfer,
> struct xhci_pipe *xp, uint32_t remain, int trb_idx,
> @@ -823,12 +845,22 @@ xhci_event_xfer_generic(struct xhci_soft
>* This might be the last TRB of a TD that ended up
>* with a Short Transfer condition, see below.
>*/
> - if (xfer->actlen == 0)
> - xfer->actlen = xfer->length - remain;
> + if (xfer->actlen == 0) {
> + if (remain)
> + xfer->actlen =
> + xhci_xfer_length_generic(xx, xp, trb_idx) -
> + remain;
> + else
> + xfer->actlen = xfer->length;
> + }
> xfer->status = USBD_NORMAL_COMPLETION;
> break;
> case XHCI_CODE_SHORT_XFER:
> - xfer->actlen = xfer->length - remain;
> + /*
> + * Use values from the transfer TRB instead of the status TRB.
> + */
> + xfer->actlen = xhci_xfer_length_generic(xx, xp, trb_idx) -
> + remain;
> /*
>* If this is not the last TRB of a transfer, we should
>* theoretically clear the IOC at the end of the chain
>
Re: ssh "kex_exchange_identification: Connection closed by remote host"
On Tue, Nov 12, 2019 at 08:52:58PM +1100, Darren Tucker wrote:
> On Tue, 12 Nov 2019 at 20:47, Darren Tucker wrote:
> > I got this on the second try although the log is not very helpful.
> > I'd suggest checking your MaxStartups setting in sshd_config and
> > comparing the settings to the numbers of connections you have.
>
> Confirmed that exceeding MaxStartups matches the observed behaviour.
> It'll produce the following log message but only at LogLevel verbose
> or higher:
>
> drop connection #1 from [127.0.0.1]:45006 on [127.0.0.1]:2022 past MaxStartups
The SSH protocol does actually allow text prior to the protocol banner
exchange (RFC4253 section 4.2) so doing something like this is actually
protocol compliant, although our client only shows it at LogLevel
debug1.
$ ssh -v -p 2022 localhost
[...]
debug1: Connecting to localhost [127.0.0.1] port 2022.
debug1: fd 3 clearing O_NONBLOCK
debug1: Connection established.
[...]
debug1: Local version string SSH-2.0-OpenSSH_8.1
debug1: kex_exchange_identification: banner line 0: exceeded MaxStartups
kex_exchange_identification: Connection closed by remote host
Index: sshd.c
===
RCS file: /cvs/src/usr.bin/ssh/sshd.c,v
retrieving revision 1.539
diff -u -p -r1.539 sshd.c
--- sshd.c 31 Oct 2019 21:23:19 - 1.539
+++ sshd.c 12 Nov 2019 10:29:15 -
@@ -1098,6 +1098,7 @@ server_accept_loop(int *sock_in, int *so
if (drop_connection(startups) == 1) {
char *laddr = get_local_ipaddr(*newsock);
char *raddr = get_peer_ipaddr(*newsock);
+ char msg[] = "Exceeded MaxStartups\r\n";
verbose("drop connection #%d from [%s]:%d "
"on [%s]:%d past MaxStartups", startups,
@@ -1105,6 +1106,7 @@ server_accept_loop(int *sock_in, int *so
laddr, get_local_port(*newsock));
free(laddr);
free(raddr);
+ write(*newsock, msg, strlen(msg));
close(*newsock);
continue;
}
--
Darren Tucker (dtucker at dtucker.net)
GPG key 11EAA6FA / A86E 3E07 5B19 5880 E860 37F4 9357 ECEF 11EA A6FA (new)
Good judgement comes with experience. Unfortunately, the experience
usually comes from bad judgement.
Re: sysupgrade: Allow to use another directory for the sets
On 11/12/19 8:29 AM, Theo de Raadt wrote:
Renaud Allard wrote:
+.It Fl d Ar directory
+Choose the
+.Ar directory
+in which the sets will be downloaded.
+Default is
+.Pa /home/_sysupgrade .
...
+ d) SETSDIR=${OPTARG};;
...
-rm -f /home/_sysupgrade/{${CLEAN}}
+rm -f ${SETSDIR}/{${CLEAN}}
Renaud, please test it for me like this:
sysupgrade -d /
This interface is dangerously incorrect.
No need to blank the disk :)
What about a prefix approach then? Like if you specify /var, it goes to
/var/_sysupgrade.
Re: ssh "kex_exchange_identification: Connection closed by remote host"
On Tue, 12 Nov 2019 at 20:47, Darren Tucker wrote: > I got this on the second try although the log is not very helpful. > I'd suggest checking your MaxStartups setting in sshd_config and > comparing the settings to the numbers of connections you have. Confirmed that exceeding MaxStartups matches the observed behaviour. It'll produce the following log message but only at LogLevel verbose or higher: drop connection #1 from [127.0.0.1]:45006 on [127.0.0.1]:2022 past MaxStartups -- Darren Tucker (dtucker at dtucker.net) GPG key 11EAA6FA / A86E 3E07 5B19 5880 E860 37F4 9357 ECEF 11EA A6FA (new) Good judgement comes with experience. Unfortunately, the experience usually comes from bad judgement.
Re: ssh "kex_exchange_identification: Connection closed by remote host"
On Tue, 12 Nov 2019 at 20:31, Darren Tucker wrote: [..] > I'd start by cranking up the client side log level (LogLevel debug3 in > ~/.ssh/config) and use CVS_RSH="ssh -E logfile" or ssh -y to send the > logs to syslog. > > Is this a public mirror, and if so which one? bleh, it doesn't support spaces, at least not in the obvious way, so something like $ cat ~/bin/ssh-with-logging #!/bin/sh exec ssh -vvv -E /tmp/ssh.log $@ $ CVS_RSH=~/bin/ssh-with-logging cvs -d anon...@anoncvs.spacehopper.org:/cvs up -dPA I got this on the second try although the log is not very helpful. I'd suggest checking your MaxStartups setting in sshd_config and comparing the settings to the numbers of connections you have. $ CVS_RSH=~/bin/ssh-with-logging cvs -d anon...@anoncvs.spacehopper.org:/cvs co src cvs [checkout aborted]: end of file from server (consult above messages if any) $ cat /tmp/ssh.log OpenSSH_8.1, LibreSSL 3.0.2 debug1: Reading configuration data /home/dtucker/.ssh/config debug1: /home/dtucker/.ssh/config line 1: Applying options for * debug1: /home/dtucker/.ssh/config line 3: Deprecated option "useroaming" debug3: kex names ok: [diffie-hellman-group1-sha1,diffie-hellman-group14-sha1] debug2: checking match for 'Host gate' host anoncvs.spacehopper.org originally anoncvs.spacehopper.org debug3: /home/dtucker/.ssh/config line 99: not matched 'Host "anoncvs.spacehopper.org"' debug2: match not found debug3: kex names ok: [curve25519-sha...@libssh.org,ecdh-sha2-nistp256] debug3: kex names ok: [ecdh-sha2-nistp521,diffie-hellman-group-exchange-sha256] debug3: kex names ok: [diffie-hellman-group1-sha1] debug3: kex names ok: [diffie-hellman-group14-sha1,diffie-hellman-group1-sha1] debug1: /home/dtucker/.ssh/config line 394: Applying options for * debug1: Reading configuration data /etc/ssh/ssh_config debug1: /etc/ssh/ssh_config line 19: Applying options for * debug1: Security key provider $SSH_SK_PROVIDER did not resolve; disabling debug2: resolving "anoncvs.spacehopper.org" port 22 debug2: ssh_connect_direct debug1: Connecting to anoncvs.spacehopper.org [195.95.187.28] port 22. debug2: fd 4 setting O_NONBLOCK debug1: fd 4 clearing O_NONBLOCK debug1: Connection established. debug3: timeout: 29385 ms remain after connect debug1: identity file /home/dtucker/.ssh/id_rsa type 0 debug1: identity file /home/dtucker/.ssh/id_rsa-cert type -1 debug1: identity file /home/dtucker/.ssh/id_dsa type 1 debug1: identity file /home/dtucker/.ssh/id_dsa-cert type -1 debug1: identity file /home/dtucker/.ssh/id_ecdsa type 2 debug1: identity file /home/dtucker/.ssh/id_ecdsa-cert type -1 debug1: identity file /home/dtucker/.ssh/id_ecdsa_sk type -1 debug1: identity file /home/dtucker/.ssh/id_ecdsa_sk-cert type -1 debug1: identity file /home/dtucker/.ssh/id_ed25519 type 3 debug1: identity file /home/dtucker/.ssh/id_ed25519-cert type -1 debug1: identity file /home/dtucker/.ssh/id_xmss type -1 debug1: identity file /home/dtucker/.ssh/id_xmss-cert type -1 debug1: Local version string SSH-2.0-OpenSSH_8.1 kex_exchange_identification: Connection closed by remote host -- Darren Tucker (dtucker at dtucker.net) GPG key 11EAA6FA / A86E 3E07 5B19 5880 E860 37F4 9357 ECEF 11EA A6FA (new) Good judgement comes with experience. Unfortunately, the experience usually comes from bad judgement.
fix xhci 'actlen' calculation
Hi,
xhci's calculation of 'xfer->actlen' is wrong if the xfer was split into
multiple TRBs. That's because the code just looks at the remainder
reported by the status TRB. However, this remainder only refers to the
total size of this single TRB; not to the total size of the xfer.
Example: assume a 16 kb xfer is split into two TRBs for 8 kb each.
Then we get a short read of 6 kb. The status TRB will refer
to the first TRB with remainder set to 2 kb. Currently xhci
would assume that actlen is 'xfer->length' (16 kb) minus the
remainder (2 kb). That yields a wrong actlen of 14 kb instead
of 6 kb.
The patch below fixes this by adding up all the remainders of all the
transfer TRBs up to the current one.
Gerhard
Index: sys/dev/usb/xhci.c
===
RCS file: /cvs/src/sys/dev/usb/xhci.c,v
retrieving revision 1.106
diff -u -p -u -p -r1.106 xhci.c
--- sys/dev/usb/xhci.c 6 Oct 2019 17:30:00 - 1.106
+++ sys/dev/usb/xhci.c 12 Nov 2019 09:29:47 -
@@ -810,6 +810,28 @@ xhci_event_xfer(struct xhci_softc *sc, u
xhci_xfer_done(xfer);
}
+uint32_t
+xhci_xfer_length_generic(struct xhci_xfer *xx, struct xhci_pipe *xp,
+int trb_idx)
+{
+ int trb0_idx;
+ uint32_t len = 0;
+
+ KASSERT(xx->index >= 0);
+ trb0_idx =
+ ((xx->index + xp->ring.ntrb) - xx->ntrb) % (xp->ring.ntrb - 1);
+
+ while (1) {
+ len +=
+ le32toh(XHCI_TRB_LEN(xp->ring.trbs[trb0_idx].trb_status));
+ if (trb0_idx == trb_idx)
+ break;
+ if (++trb0_idx == xp->ring.ntrb)
+ trb0_idx = 0;
+ }
+ return len;
+}
+
int
xhci_event_xfer_generic(struct xhci_softc *sc, struct usbd_xfer *xfer,
struct xhci_pipe *xp, uint32_t remain, int trb_idx,
@@ -823,12 +845,22 @@ xhci_event_xfer_generic(struct xhci_soft
* This might be the last TRB of a TD that ended up
* with a Short Transfer condition, see below.
*/
- if (xfer->actlen == 0)
- xfer->actlen = xfer->length - remain;
+ if (xfer->actlen == 0) {
+ if (remain)
+ xfer->actlen =
+ xhci_xfer_length_generic(xx, xp, trb_idx) -
+ remain;
+ else
+ xfer->actlen = xfer->length;
+ }
xfer->status = USBD_NORMAL_COMPLETION;
break;
case XHCI_CODE_SHORT_XFER:
- xfer->actlen = xfer->length - remain;
+ /*
+* Use values from the transfer TRB instead of the status TRB.
+*/
+ xfer->actlen = xhci_xfer_length_generic(xx, xp, trb_idx) -
+ remain;
/*
* If this is not the last TRB of a transfer, we should
* theoretically clear the IOC at the end of the chain
Re: ssh "kex_exchange_identification: Connection closed by remote host"
On Tue, 12 Nov 2019 at 20:06, Stuart Henderson wrote: > Occasionally I see this when connecting to anoncvs on my mirror, > > $ cvs -d $CVSROOT di > kex_exchange_identification: Connection closed by remote host > cvs [diff aborted]: end of file from server (consult above messages if any) > > On the server side, this is logged: > > sshd[13009]: error: kex_exchange_identification: read: Connection reset by > peer > > And others have reported it too. I haven't noticed it with e.g. http/https > connections to the server. > > Does anyone have advice about tracking it down? I'd start by cranking up the client side log level (LogLevel debug3 in ~/.ssh/config) and use CVS_RSH="ssh -E logfile" or ssh -y to send the logs to syslog. Is this a public mirror, and if so which one? -- Darren Tucker (dtucker at dtucker.net) GPG key 11EAA6FA / A86E 3E07 5B19 5880 E860 37F4 9357 ECEF 11EA A6FA (new) Good judgement comes with experience. Unfortunately, the experience usually comes from bad judgement.
Re: iked(8): add configuration option for esn
Hi Tobias, I see, however, I don't think iked would negotiate an SA without ESN support if the other side supports ESN, so I'm not sure how "enforcing" changes that. In any case, I'm not opposed to adding a toggle if you guys need it, but could you please adjust the grammar so that "esn" and "no esn" are used instead of "on" and "off" since that's what we're normally doing. "on" and "off" are clutches for simple file formats, parse.y allows you to make it a bit nicer. Regards, Mike On Mon, 11 Nov 2019 at 16:38, Tobias Heider wrote: > Sure, I have a crypto device that only supports SAs with ESN. > For it to be used I have to force iked to only negotiate SAs with ESP > support. > Another one is high-speed network cards: > Accepting a policy with ESN disabled can throttle my throughput because it > exhausts the sequence number space forcing me to rekey more often than I > would > like. > > On Mon, Nov 11, 2019 at 04:15:32PM +0100, Mike Belopuhov wrote: > > On Mon, 11 Nov 2019 at 16:08, Tobias Heider > wrote: > > > > > Hi Mike, > > > > > > the default behaviour is the same as before. I ran into cases where it > is > > > necessary for me to enforce ESN to be enabled/disabled, which is not > > > possible > > > currently. > > > > > > > Can you please describe those cases where you had to enforce it? >
ssh "kex_exchange_identification: Connection closed by remote host"
Occasionally I see this when connecting to anoncvs on my mirror, $ cvs -d $CVSROOT di kex_exchange_identification: Connection closed by remote host cvs [diff aborted]: end of file from server (consult above messages if any) On the server side, this is logged: sshd[13009]: error: kex_exchange_identification: read: Connection reset by peer And others have reported it too. I haven't noticed it with e.g. http/https connections to the server. Does anyone have advice about tracking it down?
