[m...@umaxx.net: Re: [patch] add a timeout to filter registration]

[Edgar Pettijohn]

- Forwarded message from Joerg Jung  -

Date: Mon, 4 May 2020 13:39:09 +0200
From: Joerg Jung 
To: Edgar Pettijohn 
Cc: m...@opensmtpd.org
Subject: Re: [patch] add a timeout to filter registration
X-Mailer: Apple Mail (2.3608.60.0.2.5)


> On 2. May 2020, at 17:41, Edgar Pettijohn  wrote:
> 
> When playing with filters its easy to forget to register
> stdout, etc...
> 
> Here is a patch to add a timeout and give a little helpful info as
> opposed to just hanging in an unusable state.

The patch makes sense to me, can you send it to tech@openbsd.org 
 
for review, please?


- End forwarded message -
Index: lka_filter.c
===
RCS file: /cvs/src/usr.sbin/smtpd/lka_filter.c,v
retrieving revision 1.62
diff -u -p -u -r1.62 lka_filter.c
--- lka_filter.c24 Apr 2020 11:34:07 -  1.62
+++ lka_filter.c2 May 2020 15:37:56 -
@@ -66,6 +66,7 @@ static void   filter_result_disconnect(uin
 static voidfilter_session_io(struct io *, int, void *);
 void   lka_filter_process_response(const char *, const char *);
 
+static voidlka_proc_timeout(int, short, void *);
 
 struct filter_session {
uint64_tid;
@@ -180,6 +181,7 @@ struct processor_instance {
char*name;
struct io   *io;
struct io   *errfd;
+   struct event tmo;
int  ready;
uint32_t subsystems;
 };
@@ -213,10 +215,13 @@ lka_proc_config(struct processor_instanc
io_printf(pi->io, "config
 }
 
+#define TIMEOUT 10
+
 void
 lka_proc_forked(const char *name, uint32_t subsystems, int fd)
 {
struct processor_instance   *processor;
+   struct timeval timeout = { TIMEOUT, 0 };
 
if (!processors_inited) {
dict_init();
@@ -232,6 +237,10 @@ lka_proc_forked(const char *name, uint32
 
io_set_fd(processor->io, fd);
io_set_callback(processor->io, processor_io, processor->name);
+
+   evtimer_set(>tmo, lka_proc_timeout, processor);
+   evtimer_add(>tmo, );
+
dict_xset(, name, processor);
 }
 
@@ -269,6 +278,7 @@ processor_register(const char *name, con
processor = dict_xget(, name);
 
if (strcmp(line, "register
+   evtimer_del(>tmo);
processor->ready = 1;
return;
}
@@ -1741,4 +1751,12 @@ lka_report_proc(const char *name, const 
sp = ep + 1;
 
lka_report_filter_report(reqid, name, 0, direction, , sp);
+}
+
+static void
+lka_proc_timeout(int fd, short events, void *arg)
+{
+   struct processor_instance *processor = arg;
+
+   fatalx("%s: failed to register", processor->name);
 }

[PATCH] pipex(4): rework PPP input

[Sergey Ryazanov]

Split checks from frame accepting with header removing in the common
PPP input function. This should fix packet capture on a PPP interfaces.

Also forbid IP/IPv6 frames (without PPP header) passing to BPF on
PPP interfaces to avoid mess.

Initialy this change was made as a part of pipex(4) and ppp(4)
integration work. But, since this change make the core a bit more clear
I would like to publish it now.

Ok?

---
 sys/net/pipex.c | 95 -
 1 file changed, 54 insertions(+), 41 deletions(-)

diff --git sys/net/pipex.c sys/net/pipex.c
index c433e4beaa6..e0066a61598 100644
--- sys/net/pipex.c
+++ sys/net/pipex.c
@@ -970,41 +970,68 @@ drop:
 Static void
 pipex_ppp_input(struct mbuf *m0, struct pipex_session *session, int decrypted)
 {
-   int proto, hlen = 0;
+   int proto, hlen = 0, align = 0;
struct mbuf *n;
 
KASSERT(m0->m_pkthdr.len >= PIPEX_PPPMINLEN);
proto = pipex_ppp_proto(m0, session, 0, );
+   switch (proto) {
+   case PPP_IP:
+   if (session->ip_forward == 0)
+   goto drop;
+   if (!decrypted && pipex_session_is_mppe_required(session))
+   /*
+* if ip packet received when mppe
+* is required, discard it.
+*/
+   goto drop;
+   align = 1;
+   break;
+#ifdef INET6
+   case PPP_IPV6:
+   if (session->ip6_forward == 0)
+   goto drop;
+   if (!decrypted && pipex_session_is_mppe_required(session))
+   /*
+* if ip packet received when mppe
+* is required, discard it.
+*/
+   goto drop;
+   align = 1;
+   break;
+#endif
 #ifdef PIPEX_MPPE
-   if (proto == PPP_COMP) {
+   case PPP_COMP:
if (decrypted)
goto drop;
 
/* checked this on ppp_common_input() already. */
KASSERT(pipex_session_is_mppe_accepted(session));
-
-   m_adj(m0, hlen);
-   pipex_mppe_input(m0, session);
-   return;
-   }
-   if (proto == PPP_CCP) {
+   break;
+   case PPP_CCP:
if (decrypted)
goto drop;
+   break;
+#endif
+   default:
+   if (decrypted)
+   goto drop;
+   /* protocol must be checked on pipex_common_input() already */
+   KASSERT(0);
+   goto drop;
+   }
 
 #if NBPFILTER > 0
-   {
+   {
struct ifnet *ifp = session->pipex_iface->ifnet_this;
+
if (ifp->if_bpf && ifp->if_type == IFT_PPP)
bpf_mtap(ifp->if_bpf, m0, BPF_DIRECTION_IN);
-   }
-#endif
-   m_adj(m0, hlen);
-   pipex_ccp_input(m0, session);
-   return;
}
 #endif
+
m_adj(m0, hlen);
-   if (!ALIGNED_POINTER(mtod(m0, caddr_t), uint32_t)) {
+   if (align && !ALIGNED_POINTER(mtod(m0, caddr_t), uint32_t)) {
n = m_dup_pkt(m0, 0, M_NOWAIT);
if (n == NULL)
goto drop;
@@ -1014,35 +1041,21 @@ pipex_ppp_input(struct mbuf *m0, struct pipex_session 
*session, int decrypted)
 
switch (proto) {
case PPP_IP:
-   if (session->ip_forward == 0)
-   goto drop;
-   if (!decrypted && pipex_session_is_mppe_required(session))
-   /*
-* if ip packet received when mppe
-* is required, discard it.
-*/
-   goto drop;
pipex_ip_input(m0, session);
-   return;
+   break;
 #ifdef INET6
case PPP_IPV6:
-   if (session->ip6_forward == 0)
-   goto drop;
-   if (!decrypted && pipex_session_is_mppe_required(session))
-   /*
-* if ip packet received when mppe
-* is required, discard it.
-*/
-   goto drop;
pipex_ip6_input(m0, session);
-   return;
+   break;
+#endif
+#ifdef PIPEX_MPPE
+   case PPP_COMP:
+   pipex_mppe_input(m0, session);
+   break;
+   case PPP_CCP:
+   pipex_ccp_input(m0, session);
+   break;
 #endif
-   default:
-   if (decrypted)
-   goto drop;
-   /* protocol must be checked on pipex_common_input() already */
-   KASSERT(0);
-   goto drop;
}
 
return;
@@ -1105,7 +1118,7 @@ pipex_ip_input(struct mbuf *m0, struct pipex_session 
*session)
len = 

[PATCH] ppp(4): use common bpf filter hook

[Sergey Ryazanov]

Use bpf filter hook from the common interface structure. This simplifies
the code by unifying it and prepare ppp(4) for pipex(4) support.

Ok?

---
 sys/net/if_ppp.c| 16 
 sys/net/if_pppvar.h |  1 -
 2 files changed, 8 insertions(+), 9 deletions(-)

diff --git sys/net/if_ppp.c sys/net/if_ppp.c
index 192ec7c91e0..4cba9a8778c 100644
--- sys/net/if_ppp.c
+++ sys/net/if_ppp.c
@@ -204,9 +204,11 @@ int
 ppp_clone_create(struct if_clone *ifc, int unit)
 {
struct ppp_softc *sc;
+   struct ifnet *ifp;
 
sc = malloc(sizeof(*sc), M_DEVBUF, M_WAITOK|M_ZERO);
sc->sc_unit = unit;
+   ifp = >sc_if;
snprintf(sc->sc_if.if_xname, sizeof sc->sc_if.if_xname, "%s%d",
ifc->ifc_name, unit);
sc->sc_if.if_softc = sc;
@@ -224,7 +226,7 @@ ppp_clone_create(struct if_clone *ifc, int unit)
if_attach(>sc_if);
if_alloc_sadl(>sc_if);
 #if NBPFILTER > 0
-   bpfattach(>sc_bpf, >sc_if, DLT_PPP, PPP_HDRLEN);
+   bpfattach(>if_bpf, ifp, DLT_PPP, PPP_HDRLEN);
 #endif
NET_LOCK();
LIST_INSERT_HEAD(_softc_list, sc, sc_list);
@@ -754,11 +756,9 @@ pppoutput(struct ifnet *ifp, struct mbuf *m0, struct 
sockaddr *dst,
}
 
 #if NBPFILTER > 0
-   /*
-* See if bpf wants to look at the packet.
-*/
-   if (sc->sc_bpf)
-   bpf_mtap(sc->sc_bpf, m0, BPF_DIRECTION_OUT);
+   /* See if bpf wants to look at the packet. */
+   if (ifp->if_bpf)
+   bpf_mtap(ifp->if_bpf, m0, BPF_DIRECTION_OUT);
 #endif
 
/*
@@ -1369,8 +1369,8 @@ ppp_inproc(struct ppp_softc *sc, struct mbuf *m)
 
 #if NBPFILTER > 0
/* See if bpf wants to look at the packet. */
-   if (sc->sc_bpf)
-   bpf_mtap(sc->sc_bpf, m, BPF_DIRECTION_IN);
+   if (ifp->if_bpf)
+   bpf_mtap(ifp->if_bpf, m, BPF_DIRECTION_IN);
 #endif
 
rv = 0;
diff --git sys/net/if_pppvar.h sys/net/if_pppvar.h
index 87f7d1798bb..9dc774a0515 100644
--- sys/net/if_pppvar.h
+++ sys/net/if_pppvar.h
@@ -113,7 +113,6 @@ struct ppp_softc {
struct  mbuf *sc_togo;  /* output packet ready to go */
struct  mbuf_list sc_npqueue;   /* output packets not to be sent yet */
struct  pppstat sc_stats;   /* count of bytes/pkts sent/rcvd */
-   caddr_t sc_bpf; /* hook for BPF */
enumNPmode sc_npmode[NUM_NP]; /* what to do with each NP */
struct  compressor *sc_xcomp;   /* transmit compressor */
void*sc_xc_state;   /* transmit compressor state */
-- 
2.26.0

[PATCH] tcpdump: add ppp address/protocol compression support

[Sergey Ryazanov]

Add support for parsing ppp frames with compressed address and(or)
protocol fields. Since we have no apriory information than try to
guess such frames by inability to parse a frame in a regular way.

ok?

---
 usr.sbin/tcpdump/print-ppp.c | 29 +
 1 file changed, 17 insertions(+), 12 deletions(-)

diff --git usr.sbin/tcpdump/print-ppp.c usr.sbin/tcpdump/print-ppp.c
index 21f5d154847..60027178942 100644
--- usr.sbin/tcpdump/print-ppp.c
+++ usr.sbin/tcpdump/print-ppp.c
@@ -341,20 +341,26 @@ void
 ppp_print(const u_char *p, u_int length)
 {
uint16_t proto;
-   int l;
+   int l, pfl;
 
l = snapend - p;
 
-   if (l < sizeof(proto)) {
+   /* Check for compressed protocol field */
+   if (l >= 1 && (p[0] & 0x1) != 0)
+   pfl = sizeof(uint8_t);
+   else
+   pfl = sizeof(uint16_t);
+
+   if (l < pfl) {
printf("[|ppp]");
return;
}
 
-   proto = EXTRACT_16BITS(p);
+   proto = pfl == sizeof(uint8_t) ? p[0] : EXTRACT_16BITS(p);
 
-   p += sizeof(proto);
-   l -= sizeof(proto);
-   length -= sizeof(proto);
+   p += pfl;
+   l -= pfl;
+   length -= pfl;
 
if (eflag)
ppp_protoname(proto);
@@ -1385,12 +1391,11 @@ ppp_hdlc_print(const u_char *p, u_int length)
address = p[0];
control = p[1];
 
-   p += sizeof(address) + sizeof(control);
-   l -= sizeof(address) + sizeof(control);
-   length -= sizeof(address) + sizeof(control);
-
switch (address) {
case 0xff: /* All-Stations */
+   p += sizeof(address) + sizeof(control);
+   l -= sizeof(address) + sizeof(control);
+   length -= sizeof(address) + sizeof(control);
if (eflag)
printf("%02x %02x %u ", address, control, length);
 
@@ -1402,8 +1407,8 @@ ppp_hdlc_print(const u_char *p, u_int length)
ppp_print(p, length);
break;
 
-   default:
-   printf("ppp address 0x%02x unknown", address);
+   default: /* Assume address compression */
+   ppp_print(p, length);
break;
}
return;
-- 
2.26.0

iwn: fix connection hangs with some APs

[Stefan Sperling]

For some reason, changes I made to iwn(4) in the commit quoted below
have caused connections to get stuck on some APs during Tx bursts.

This does not occur with every type of AP. It was observed on an Apple
Airport Extreme 6th gen, and on a b-box 3V+ (Sagemcom Mac address).

The patch below reverts all changes related to interaction between driver
and firmware from the relevant commit and works around the problem in all
known cases. It is unclear why it helps, but at least this patch should
avoid the problem from affecting 6.7-release.

ok?

The relevant commit was:

---
Module name:src
Changes by: s...@cvs.openbsd.org2020/04/27 02:02:24

Modified files:
sys/dev/pci: if_iwn.c if_iwnvar.h 

Log message:
Fix processing of compressed block ack notifications sent by iwn(4) firmware.

Fix wrong assumptions about what the data in these notifications is supposed
to represent, and actually piece information about individual subframes of
aggregated frames (A-MPDUs) back together when reporting to MiRA, rather than
reporting unrelated subframes to MiRA individually.

Testing by cwen@, Josh Grosse, f.holop, benno@
ok jmatthew@
---


diff 523aa541f5e65a29bda64c13043addf764654fa1 /usr/src
blob - 620cbd4c138516398a4683d83c1b6bf8aac57c82
file + sys/dev/pci/if_iwn.c
--- sys/dev/pci/if_iwn.c
+++ sys/dev/pci/if_iwn.c
@@ -158,6 +158,8 @@ voidiwn_rx_phy(struct iwn_softc *, struct 
iwn_rx_des
 void   iwn_rx_done(struct iwn_softc *, struct iwn_rx_desc *,
struct iwn_rx_data *, struct mbuf_list *);
 void   iwn_mira_choose(struct iwn_softc *, struct ieee80211_node *);
+void   iwn_ampdu_rate_control(struct iwn_softc *, struct 
ieee80211_node *,
+   struct iwn_tx_ring *, int, uint16_t, uint16_t);
 void   iwn_rx_compressed_ba(struct iwn_softc *, struct iwn_rx_desc *,
struct iwn_rx_data *);
 void   iwn5000_rx_calib_results(struct iwn_softc *,
@@ -2248,83 +2250,16 @@ iwn_mira_choose(struct iwn_softc *sc, struct ieee80211
iwn_set_link_quality(sc, ni);
 }
 
-/*
- * Process an incoming Compressed BlockAck.
- * Note that these block ack notifications are generated by firmware and do
- * not necessarily correspond to contents of block ack frames seen on the air.
- */
 void
-iwn_rx_compressed_ba(struct iwn_softc *sc, struct iwn_rx_desc *desc,
-struct iwn_rx_data *data)
+iwn_ampdu_rate_control(struct iwn_softc *sc, struct ieee80211_node *ni,
+struct iwn_tx_ring *txq, int tid, uint16_t seq, uint16_t ssn)
 {
-   struct iwn_compressed_ba *cba = (struct iwn_compressed_ba *)(desc + 1);
struct ieee80211com *ic = >sc_ic;
-   struct ieee80211_node *ni;
-   struct ieee80211_tx_ba *ba;
-   struct iwn_node *wn;
-   struct iwn_tx_ring *txq;
-   uint16_t seq, ssn, idx, end_idx;
+   struct iwn_node *wn = (void *)ni;
+   struct ieee80211_tx_ba *ba = >ni_tx_ba[tid];
int min_ampdu_id, max_ampdu_id, id;
-   int qid;
+   int idx, end_idx;
 
-   if (ic->ic_state != IEEE80211_S_RUN)
-   return;
-
-   bus_dmamap_sync(sc->sc_dmat, data->map, sizeof (*desc), sizeof (*cba),
-   BUS_DMASYNC_POSTREAD);
-
-   if (!IEEE80211_ADDR_EQ(ic->ic_bss->ni_macaddr, cba->macaddr))
-   return;
-
-   ni = ic->ic_bss;
-   wn = (void *)ni;
-
-   qid = le16toh(cba->qid);
-   if (qid < sc->first_agg_txq || qid >= sc->ntxqs)
-   return;
-
-   txq = >txq[qid];
-
-   /* Protect against a firmware bug where the queue/TID are off. */
-   if (qid != sc->first_agg_txq + cba->tid)
-   return;
-
-   ba = >ni_tx_ba[cba->tid];
-   if (ba->ba_state != IEEE80211_BA_AGREED)
-   return;
-
-   /*
-* The first bit in cba->bitmap corresponds to the sequence number
-* stored in the sequence control field cba->seq.
-* Any frames older than this can now be discarded; they should
-* already have been reported as failures or been acknowledged.
-*
-* Multiple BA notifications in a row may be using this number, with
-* additional bits being set in cba->bitmap. It is unclear how the
-* firmware decides to shift this window forward.
-*/
-   seq = le16toh(cba->seq) >> IEEE80211_SEQ_SEQ_SHIFT;
-   if (!SEQ_LT(seq, ba->ba_winstart)) {
-   ieee80211_output_ba_move_window(ic, ni, cba->tid, seq);
-   iwn_ampdu_txq_advance(sc, txq, qid,
-   IWN_AGG_SSN_TO_TXQ_IDX(seq));
-   iwn_clear_oactive(sc, txq);
-   }
-   /* Our BA window should now correspond to the bitmap. */
-   if (ba->ba_winstart != seq)
-   return;
-
-   /* Skip rate control if our Tx rate is fixed. */
-   if (ic->ic_fixed_mcs != -1)
-   return;
-
-   /*
-* The firmware's new BA window starting sequence number
-* corresponds to the 

Diff for www:mail

[bsd]

Hi,

Here a diff for www page: mail

Please, review this diff to add French ML

Right?



Index: mail.html
===
RCS file: /cvs/www/mail.html,v
retrieving revision 1.170
diff -u -r1.170 mail.html
--- mail.html   24 Apr 2020 21:51:21 -  1.170
+++ mail.html   4 May 2020 13:38:21 -
@@ -423,6 +423,14 @@
  PLEASE KEEP THIS LIST SORTED, EXCEPT FOR TRANSLATIONS, WHERE YOU SHOULD PUT
  THE LIST IN YOUR LANGUAGE, IF ONE EXISTS, HEAD OF LIST.
  -->
+
+French:
+bla...@openbsd.fr.eu.org
+To subscribe, send a message to 
+mailto:blabla+subscr...@openbsd.fr.eu.org;>
+blabla+subscr...@openbsd.fr.eu.org
+
+(https://openbsd.fr.eu.org/blabla/;>Archive)
 
 
 Spanish:

--
" Fully Basic System Distinguish Life! " ~ " Libre as a BSD " +=<<<

Stephane HUC as PengouinBSD or CIOTBSD
b...@stephane-huc.net

smtpd stricter forkmda()

[Gilles Chehade]

hello,

forkmda() is never supposed to be called with an action dispatcher which
is not local, this would indicate that the code path was abused somehow.

idea suggested by Demi M. Obenour


diff --git a/smtpd/smtpd.c b/smtpd/smtpd.c
index ce1262fa..4c5fc3d9 100644
--- a/smtpd/smtpd.c
+++ b/smtpd/smtpd.c
@@ -1409,6 +1409,8 @@ forkmda(struct mproc *p, uint64_t id, struct deliver 
*deliver)
const char  *pw_dir;
 
dsp = dict_xget(env->sc_dispatchers, deliver->dispatcher);
+   if (dsp->type != DISPATCHER_LOCAL)
+   fatalx("non-local dispatcher called from forkmda()");
 
log_debug("debug: smtpd: forking mda for session %016"PRIx64
": %s as %s", id, deliver->userinfo.username,


-- 
Gilles Chehade @poolpOrg

https://www.poolp.orgpatreon: https://www.patreon.com/gilles

Re: smtpd stricter forkmda()

[Joerg Jung]

> On 4. May 2020, at 11:17, Gilles Chehade  wrote:
> 
> forkmda() is never supposed to be called with an action dispatcher which
> is not local, this would indicate that the code path was abused somehow.
> 
> idea suggested by Demi M. Obenour


ok jung@ (for post-lock)

> diff --git a/smtpd/smtpd.c b/smtpd/smtpd.c
> index ce1262fa..4c5fc3d9 100644
> --- a/smtpd/smtpd.c
> +++ b/smtpd/smtpd.c
> @@ -1409,6 +1409,8 @@ forkmda(struct mproc *p, uint64_t id, struct deliver 
> *deliver)
>   const char  *pw_dir;
> 
>   dsp = dict_xget(env->sc_dispatchers, deliver->dispatcher);
> + if (dsp->type != DISPATCHER_LOCAL)
> + fatalx("non-local dispatcher called from forkmda()");
> 
>   log_debug("debug: smtpd: forking mda for session %016"PRIx64
>   ": %s as %s", id, deliver->userinfo.username,