Re: iked: add a tag macro for EAP identity

[Mike Belopuhov]

On 1 June 2017 at 10:57, Stuart Henderson  wrote:
>
> I have an iked VPN box that needs to restrict access to certain
> resources by user. For connections using a client cert this can be
> done by using PF tags based on the ID from the cert, but this
> falls short for EAP.
>
> This diff adds an $eapid macro that can be used instead. If eapid
> isn't set (non-EAP connection) it just skips expanding the macro.
>
> OK?
>
> (I'd really like per-user IP address setting, but this gets the
> job done in a minimal way.. :)

LGTM, OK mikeb

Re: i386 clang: fix libc build

[Mark Kettenis]

> Date: Thu, 1 Jun 2017 13:33:05 +0200
> From: Christian Weisgerber 
> 
> >From kettenis@'s corresponding commit on amd64:
> 
>   Remove branch prediction hints from conditional branch instructions.
>   These hints are not recognized by clang's builtin assembler and
>   the opcode prefixes they generate have been no-ops for all CPUs
>   after the Pentium 4.
> 
> Admittedly, CPUs <= Pentium 4 are the target for i386, but I think
> we can afford the loss of this micro-optimization.
> 
> ok?

Agreed; ok kettenis@

> Index: arch/i386/SYS.h
> ===
> RCS file: /cvs/src/lib/libc/arch/i386/SYS.h,v
> retrieving revision 1.25
> diff -u -p -r1.25 SYS.h
> --- arch/i386/SYS.h   7 May 2016 19:05:21 -   1.25
> +++ arch/i386/SYS.h   31 May 2017 22:52:21 -
> @@ -89,7 +89,7 @@
>   movl$-1, %eax;  \
>   movl$-1, %edx   /* for lseek */
>  #define HANDLE_ERRNO()   \
> - jnc,pt  99f;\
> + jnc 99f;\
>   SET_ERRNO();\
>   99:
>  
> -- 
> Christian "naddy" Weisgerber  na...@mips.inka.de
> 
> 

i386 clang: fix libc build

[Christian Weisgerber]

>From kettenis@'s corresponding commit on amd64:

  Remove branch prediction hints from conditional branch instructions.
  These hints are not recognized by clang's builtin assembler and
  the opcode prefixes they generate have been no-ops for all CPUs
  after the Pentium 4.

Admittedly, CPUs <= Pentium 4 are the target for i386, but I think
we can afford the loss of this micro-optimization.

ok?

Index: arch/i386/SYS.h
===
RCS file: /cvs/src/lib/libc/arch/i386/SYS.h,v
retrieving revision 1.25
diff -u -p -r1.25 SYS.h
--- arch/i386/SYS.h 7 May 2016 19:05:21 -   1.25
+++ arch/i386/SYS.h 31 May 2017 22:52:21 -
@@ -89,7 +89,7 @@
movl$-1, %eax;  \
movl$-1, %edx   /* for lseek */
 #define HANDLE_ERRNO() \
-   jnc,pt  99f;\
+   jnc 99f;\
SET_ERRNO();\
99:
 
-- 
Christian "naddy" Weisgerber  na...@mips.inka.de

Re: efiboot serial console support

[YASUOKA Masahiko]

Yes, I think this is the proper fix.  Please commit.

ok yasuoka

On Thu, 1 Jun 2017 10:41:57 +0200
Patrick Wildt  wrote:
> On Thu, Jun 01, 2017 at 10:27:28AM +0200, Stefan Sperling wrote:
>> On Tue, May 30, 2017 at 02:31:48PM +0200, YASUOKA Masahiko wrote:
>> > +  status = EFI_CALL(BS->LocateHandle, ByProtocol, _guid, 0, , 0);
>> > +  if (status == EFI_BUFFER_TOO_SMALL) {
>> > +  handles = alloc(sz);
>> > +  status = EFI_CALL(BS->LocateHandle, ByProtocol, _guid,
>> > +  0, , handles);
>> > +  }
>> > +  if (handles == NULL || EFI_ERROR(status))
>> > +  panic("could not get handles of serial i/o");
>> 
>> Hi,
>> 
>> On my thinkpad helix 2 the boot loader now keeps rebooting with
>> the above panic message before I can even type anything.
>> 
>> Could this panic be changed into a non-fatal error?
>> 
>> Thanks!
>> 
> 
> This should probably fix it / work around it.
> 
> diff --git a/sys/arch/amd64/stand/efiboot/efiboot.c 
> b/sys/arch/amd64/stand/efiboot/efiboot.c
> index 25e34c1a93b..998e6875f08 100644
> --- a/sys/arch/amd64/stand/efiboot/efiboot.c
> +++ b/sys/arch/amd64/stand/efiboot/efiboot.c
> @@ -526,8 +526,10 @@ efi_com_probe(struct consdev *cn)
>   status = EFI_CALL(BS->LocateHandle, ByProtocol, _guid,
>   0, , handles);
>   }
> - if (handles == NULL || EFI_ERROR(status))
> - panic("could not get handles of serial i/o");
> + if (handles == NULL || EFI_ERROR(status)) {
> + free(handles, sz);
> + return;
> + }
>  
>   for (i = 0; i < sz / sizeof(EFI_HANDLE); i++) {
>   /*
> 

fix missed beacon handling bugs in iwn(4) and iwm(4)

[Stefan Sperling]

Prevent both drivers from sending additional probe requests while we're
already waiting for some response from the AP. Check the ic_mgt_timer for
this purpose. Fixes misbehaviour when hardware sends many "missed beacon"
interrupts. We ended up flooding the AP with probe requests because the
ic_mgt_timer got reset faster than the AP could respond:

Jun  1 12:02:38 jim /bsd: iwn0: sending probe_req to xx:xx:xx:xx:xx:xx on 
channel 40 mode 11n
Jun  1 12:03:09 jim last message repeated 227 times

Additionally:
In iwn(4), read the missed beacon counter value after DMA sync.
In iwm(4), byteswap the missed beacon counter value when reading it.

Index: if_iwn.c
===
RCS file: /cvs/src/sys/dev/pci/if_iwn.c,v
retrieving revision 1.189
diff -u -p -r1.189 if_iwn.c
--- if_iwn.c31 May 2017 16:12:39 -  1.189
+++ if_iwn.c1 Jun 2017 10:07:07 -
@@ -2504,7 +2504,7 @@ iwn_notif_intr(struct iwn_softc *sc)
{
struct iwn_beacon_missed *miss =
(struct iwn_beacon_missed *)(desc + 1);
-   uint32_t missed = letoh32(miss->consecutive);
+   uint32_t missed;
 
if ((ic->ic_opmode != IEEE80211_M_STA) ||
(ic->ic_state != IEEE80211_S_RUN))
@@ -2512,6 +2512,7 @@ iwn_notif_intr(struct iwn_softc *sc)
 
bus_dmamap_sync(sc->sc_dmat, data->map, sizeof (*desc),
sizeof (*miss), BUS_DMASYNC_POSTREAD);
+   missed = letoh32(miss->consecutive);
 
/*
 * If more than 5 consecutive beacons are missed,
@@ -2526,7 +2527,7 @@ iwn_notif_intr(struct iwn_softc *sc)
 * state machine will drop us into scanning after timing
 * out waiting for a probe response.
 */
-   if (missed > ic->ic_bmissthres)
+   if (missed > ic->ic_bmissthres && !ic->ic_mgt_timer)
IEEE80211_SEND_MGMT(ic, ic->ic_bss,
IEEE80211_FC0_SUBTYPE_PROBE_REQ, 0);
break;
Index: if_iwm.c
===
RCS file: /cvs/src/sys/dev/pci/if_iwm.c,v
retrieving revision 1.189
diff -u -p -r1.189 if_iwm.c
--- if_iwm.c31 May 2017 13:22:16 -  1.189
+++ if_iwm.c1 Jun 2017 10:08:49 -
@@ -3544,6 +3544,7 @@ iwm_rx_bmiss(struct iwm_softc *sc, struc
 {
struct ieee80211com *ic = >sc_ic;
struct iwm_missed_beacons_notif *mbn = (void *)pkt->data;
+   uint32_t missed;
 
if ((ic->ic_opmode != IEEE80211_M_STA) ||
(ic->ic_state != IEEE80211_S_RUN))
@@ -3552,7 +3553,8 @@ iwm_rx_bmiss(struct iwm_softc *sc, struc
bus_dmamap_sync(sc->sc_dmat, data->map, sizeof(*pkt),
sizeof(*mbn), BUS_DMASYNC_POSTREAD);
 
-   if (mbn->consec_missed_beacons_since_last_rx > ic->ic_bmissthres) {
+   missed = le32toh(mbn->consec_missed_beacons_since_last_rx);
+   if (missed > ic->ic_bmissthres && ic->ic_mgt_timer == 0) {
/*
 * Rather than go directly to scan state, try to send a
 * directed probe request first. If that fails then the

iked: add a tag macro for EAP identity

[Stuart Henderson]

I have an iked VPN box that needs to restrict access to certain
resources by user. For connections using a client cert this can be
done by using PF tags based on the ID from the cert, but this
falls short for EAP.

This diff adds an $eapid macro that can be used instead. If eapid
isn't set (non-EAP connection) it just skips expanding the macro.

OK?

(I'd really like per-user IP address setting, but this gets the
job done in a minimal way.. :)

Index: iked.conf.5
===
RCS file: /cvs/src/sbin/iked/iked.conf.5,v
retrieving revision 1.49
diff -u -p -r1.49 iked.conf.5
--- iked.conf.5 27 Mar 2017 15:45:19 -  1.49
+++ iked.conf.5 1 Jun 2017 08:48:33 -
@@ -572,6 +572,8 @@ from ASN1_DN IDs, for example
 .Ar ASN1_ID//C=DE/../CN=10.1.1.1/..
 will be expanded to
 .Ar 10.1.1.1 .
+.It Ar $eapid
+For a connection using EAP, the identity (username) used by the remote peer.
 .It Ar $domain
 Extract the domain from IDs of type FQDN, UFQDN or ASN1_DN.
 .It Ar $name
Index: ikev2.c
===
RCS file: /cvs/src/sbin/iked/ikev2.c,v
retrieving revision 1.154
diff -u -p -r1.154 ikev2.c
--- ikev2.c 26 Apr 2017 10:42:38 -  1.154
+++ ikev2.c 1 Jun 2017 08:48:33 -
@@ -4560,6 +4560,14 @@ ikev2_sa_tag(struct iked_sa *sa, struct 
}
}
 
+   if (strstr(format, "$eapid") != NULL && sa->sa_eapid != NULL) {
+   if (expand_string(sa->sa_tag, len, "$eapid",
+   sa->sa_eapid) != 0) {
+   log_debug("%s: failed to expand tag", __func__);
+   goto fail;
+   }
+   }
+
if (strstr(format, "$name") != NULL) {
if (expand_string(sa->sa_tag, len, "$name",
sa->sa_policy->pol_name) != 0) {

Re: efiboot serial console support

[Patrick Wildt]

On Thu, Jun 01, 2017 at 10:27:28AM +0200, Stefan Sperling wrote:
> On Tue, May 30, 2017 at 02:31:48PM +0200, YASUOKA Masahiko wrote:
> > +   status = EFI_CALL(BS->LocateHandle, ByProtocol, _guid, 0, , 0);
> > +   if (status == EFI_BUFFER_TOO_SMALL) {
> > +   handles = alloc(sz);
> > +   status = EFI_CALL(BS->LocateHandle, ByProtocol, _guid,
> > +   0, , handles);
> > +   }
> > +   if (handles == NULL || EFI_ERROR(status))
> > +   panic("could not get handles of serial i/o");
> 
> Hi,
> 
> On my thinkpad helix 2 the boot loader now keeps rebooting with
> the above panic message before I can even type anything.
> 
> Could this panic be changed into a non-fatal error?
> 
> Thanks!
> 

This should probably fix it / work around it.

diff --git a/sys/arch/amd64/stand/efiboot/efiboot.c 
b/sys/arch/amd64/stand/efiboot/efiboot.c
index 25e34c1a93b..998e6875f08 100644
--- a/sys/arch/amd64/stand/efiboot/efiboot.c
+++ b/sys/arch/amd64/stand/efiboot/efiboot.c
@@ -526,8 +526,10 @@ efi_com_probe(struct consdev *cn)
status = EFI_CALL(BS->LocateHandle, ByProtocol, _guid,
0, , handles);
}
-   if (handles == NULL || EFI_ERROR(status))
-   panic("could not get handles of serial i/o");
+   if (handles == NULL || EFI_ERROR(status)) {
+   free(handles, sz);
+   return;
+   }
 
for (i = 0; i < sz / sizeof(EFI_HANDLE); i++) {
/*

Re: efiboot serial console support

[Stefan Sperling]

On Tue, May 30, 2017 at 02:31:48PM +0200, YASUOKA Masahiko wrote:
> + status = EFI_CALL(BS->LocateHandle, ByProtocol, _guid, 0, , 0);
> + if (status == EFI_BUFFER_TOO_SMALL) {
> + handles = alloc(sz);
> + status = EFI_CALL(BS->LocateHandle, ByProtocol, _guid,
> + 0, , handles);
> + }
> + if (handles == NULL || EFI_ERROR(status))
> + panic("could not get handles of serial i/o");

Hi,

On my thinkpad helix 2 the boot loader now keeps rebooting with
the above panic message before I can even type anything.

Could this panic be changed into a non-fatal error?

Thanks!

Re: let's add PF_LOCK()

[Alexandr Nedvedicky]

Hello,


> > diff -r 6abbb123112a .hgtags
> > --- /dev/null   Thu Jan 01 00:00:00 1970 +
> > +++ b/.hgtags   Wed May 31 10:42:50 2017 +0200
> > @@ -0,0 +1,1 @@
> > +d545881e2652dbc0c057691a39a095bce92f441f pf-lock.baseline
> 
> Please be careful and don't include VCS's goo in diffs.
> It'd be nice if you could manage to instruct mercurial
> to remove a/ and b/ parts.

I see. Adding a 'noprefix=true' to [diff] section in .hgrc
removes the prefix.


> > +   }
> > +   PF_UNLOCK();
> > +   /*
> > +* Fragments don't require PF_LOCK(), they use their own mutex.
> > +*/
> > +   if (nloops >= pf_default_rule.timeout[PFTM_INTERVAL]) {
> > +   pf_purge_expired_fragments();
> > nloops = 0;
> > }
> >  
> 
> Minor nit: change comment above to say "they use their own lock."
> to remove the reference to a specific lock implementation.

fixed.

> 
> > @@ -1320,7 +1327,6 @@ pf_remove_state(struct pf_state *cur)
> > }
> > RB_REMOVE(pf_state_tree_id, _id, cur);
> >  #if NPFLOW > 0
> > -   if (cur->state_flags & PFSTATE_PFLOW)
> > export_pflow(cur);
> 
> If you're removing this "if" statemenet, please remove the tab
> from the next line.

the "if" statement must stay. I've mismerged the code. Thank
you for catching this!!!

> 
> > @@ -6692,6 +6699,9 @@ pf_test(sa_family_t af, int fwdir, struc
> > }
> > pd.m->m_pkthdr.pf.flags |= PF_TAG_PROCESSED;
> >  
> > +   /* lock the look-up/write section of pf_test() */
> Minor nit: spell "lookup" without a dash.

fixed


> > diff -r 6abbb123112a src/sys/net/pf_ioctl.c
> > --- a/src/sys/net/pf_ioctl.cWed May 31 10:21:18 2017 +0200
> > +++ b/src/sys/net/pf_ioctl.cWed May 31 10:42:50 2017 +0200
> > @@ -129,6 +129,10 @@ struct {
> >  TAILQ_HEAD(pf_tags, pf_tagname)pf_tags = 
> > TAILQ_HEAD_INITIALIZER(pf_tags),
> > pf_qids = TAILQ_HEAD_INITIALIZER(pf_qids);
> >  
> > +#ifdef WITH_PF_LOCK
> > +struct rwlock   pf_lock = RWLOCK_INITIALIZER("pf_lock");
> > +#endif /* WITH_PF_LOCK */
> > +
> >  #if (PF_QNAME_SIZE != PF_TAG_NAME_SIZE)
> >  #error PF_QNAME_SIZE must be equal to PF_TAG_NAME_SIZE
> >  #endif
> 
> Where do you want to define this WITH_PF_LOCK?
> 

I currently add 

option WITH_PF_LOCK

to sys/arch/amd64/compile/GENERIC.MP to build PF with PF_LOCK.
But there should be better place in tree. Perhaps sys/conf/GENERIC?

--- src/sys/conf/GENERICThu Jun 01 09:21:03 2017 +0200
+++ src/sys/conf/GENERICThu Jun 01 09:21:52 2017 +0200
@@ -16,6 +16,7 @@ optionACCOUNTING  # acct(2) process acc
 option KMEMSTATS   # collect malloc(9) statistics
 option PTRACE  # ptrace(2) system call
 #optionWITNESS # witness(4) lock checker
+#optionWITH_PF_LOCK# PF lock support
 
 #optionKVA_GUARDPAGES  # slow virtual address recycling (+ 
guarding)
 option POOL_DEBUG  # pool corruption detection


> I suggest to make this look a tiny bit prettier:
> 
> #define PF_LOCK() do {\
>   NET_ASSERT_LOCKED();\
>   rw_enter_write(_lock);   \
>   } while (0)
> 
> #define PF_UNLOCK()   do {\
>   PF_ASSERT_LOCKED(); \
>   rw_exit_write(_lock);\
>   } while (0)


indeed, it should look nice now.

Note: the updated diff adds '#option WITH_PF_LOCK' to src/sys/conf/GENERIC

thanks a lot
regards
sasha

8<---8<---8<--8<
diff -r ccb9f01e56a7 .hgtags
--- /dev/null   Thu Jan 01 00:00:00 1970 +
+++ .hgtags Thu Jun 01 09:29:15 2017 +0200
@@ -0,0 +1,1 @@
+d545881e2652dbc0c057691a39a095bce92f441f pf-lock.baseline
diff -r ccb9f01e56a7 src/sys/conf/GENERIC
--- src/sys/conf/GENERICThu Jun 01 09:19:56 2017 +0200
+++ src/sys/conf/GENERICThu Jun 01 09:29:15 2017 +0200
@@ -16,7 +16,7 @@ optionACCOUNTING  # acct(2) process acc
 option KMEMSTATS   # collect malloc(9) statistics
 option PTRACE  # ptrace(2) system call
 #optionWITNESS # witness(4) lock checker
-#optionPF_LOCK # build with pf lock support
+#optionWITH_PF_LOCK# PF lock support
 
 #optionKVA_GUARDPAGES  # slow virtual address recycling (+ 
guarding)
 option POOL_DEBUG  # pool corruption detection
diff -r ccb9f01e56a7 src/sys/net/pf.c
--- src/sys/net/pf.cThu Jun 01 09:19:56 2017 +0200
+++ src/sys/net/pf.cThu Jun 01 09:29:15 2017 +0200
@@ -923,7 +923,7 @@ int
 pf_state_insert(struct pfi_kif *kif, struct pf_state_key **skw,
 struct pf_state_key **sks, struct pf_state *s)
 {
-