Re: smtpd remove implicit ruleset behavior

2019-11-24 Thread Gilles Chehade
On Mon, Nov 25, 2019 at 08:30:21AM +0100, Gilles Chehade wrote:
> On Mon, Nov 25, 2019 at 01:09:20AM +0100, Joerg Jung wrote:
> > On Sun, Nov 24, 2019 at 10:54:14AM +0100, Gilles Chehade wrote:
> > > 
> > > Ten years ago, it seemed a very neat idea that OpenSMTPD would have some
> > > implicit defaults to avoid people creating open relays.
> > > 
> > > Back then I was trying to make the smtpd.conf as compact as possible and
> > > came up with the very nice idea of "implicit local" so that we would get
> > > a very compact:
> > > 
> > >   accept for any relay
> > > 
> > > which would not be an open relay as it translated to:
> > > 
> > >   accept from local for any relay
> > > 
> > > 
> > > This idea was carried when we moved the syntax to match/action.
> > > 
> > > I think this was an error from the beginning and we should only have the
> > > explicit notation as I see a trend in people coming up with:
> > > 
> > >   match for domain foobar.org action "deliver"
> > > 
> > > which, read loud, seems to imply that mail for domain foobar.org will be
> > > delivered but which actually fails because it translates as:
> > > 
> > >   match from local for domain foobar.org action "deliver"
> > > 
> > > and actually limits the scope to local users...
> > > 
> > > People keep making this mistake over and over which as safe as it is, is
> > > a serious hint that the mistake is on smtpd's side.
> > > 
> > > 
> > > Is there strong objection to move to a mode where implicit notation will
> > > no longer be allowed ?
> > 
> > No objections. Yes, please make the notation explicit and remove the
> > syntactic sugar which often seems to be the reason for confusions.
> >  
> > > This could start with us adding the explicit notation to default config,
> > > then put a startup warning in the next release so configurations are not
> > > broken but people spot that this is no longer encouraged and we can then
> > > later kill it.
> > 
> > Sounds like a good plan to me.
> > 
> 
> This diff makes default smtpd.conf use the explicit notation.
> 
> ok ?
> 

and this diff makes smtpd warn at startup that implicit rules were used:

laptop$ doas smtpd
smtpd: ruleset relies on implicit 'from' at line 10
smtpd: ruleset relies on implicit 'for' at line 11
laptop$

alternatively these warnx() can be turned into errx() if we want to
go right away into explicit mode without warning for a release.


Index: parse.y
===
RCS file: /cvs/src/usr.sbin/smtpd/parse.y,v
retrieving revision 1.264
diff -u -p -r1.264 parse.y
--- parse.y 12 Nov 2019 21:02:42 -  1.264
+++ parse.y 25 Nov 2019 07:39:28 -
@@ -1313,10 +1313,12 @@ MATCH {
rule = xcalloc(1, sizeof *rule);
 } match_options action {
if (!rule->flag_from) {
+   warnx("ruleset relies on implicit 'from' at line %d", 
file->lineno);
rule->table_from = strdup("");
rule->flag_from = 1;
}
if (!rule->flag_for) {
+   warnx("ruleset relies on implicit 'for' at line %d", 
file->lineno);
rule->table_for = strdup("");
rule->flag_for = 1;
}


-- 
Gilles Chehade @poolpOrg

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



Re: smtpd remove implicit ruleset behavior

2019-11-24 Thread Gilles Chehade
On Mon, Nov 25, 2019 at 01:09:20AM +0100, Joerg Jung wrote:
> On Sun, Nov 24, 2019 at 10:54:14AM +0100, Gilles Chehade wrote:
> > 
> > Ten years ago, it seemed a very neat idea that OpenSMTPD would have some
> > implicit defaults to avoid people creating open relays.
> > 
> > Back then I was trying to make the smtpd.conf as compact as possible and
> > came up with the very nice idea of "implicit local" so that we would get
> > a very compact:
> > 
> >   accept for any relay
> > 
> > which would not be an open relay as it translated to:
> > 
> >   accept from local for any relay
> > 
> > 
> > This idea was carried when we moved the syntax to match/action.
> > 
> > I think this was an error from the beginning and we should only have the
> > explicit notation as I see a trend in people coming up with:
> > 
> >   match for domain foobar.org action "deliver"
> > 
> > which, read loud, seems to imply that mail for domain foobar.org will be
> > delivered but which actually fails because it translates as:
> > 
> >   match from local for domain foobar.org action "deliver"
> > 
> > and actually limits the scope to local users...
> > 
> > People keep making this mistake over and over which as safe as it is, is
> > a serious hint that the mistake is on smtpd's side.
> > 
> > 
> > Is there strong objection to move to a mode where implicit notation will
> > no longer be allowed ?
> 
> No objections. Yes, please make the notation explicit and remove the
> syntactic sugar which often seems to be the reason for confusions.
>  
> > This could start with us adding the explicit notation to default config,
> > then put a startup warning in the next release so configurations are not
> > broken but people spot that this is no longer encouraged and we can then
> > later kill it.
> 
> Sounds like a good plan to me.
> 

This diff makes default smtpd.conf use the explicit notation.

ok ?

Index: smtpd.conf
===
RCS file: /cvs/src/etc/mail/smtpd.conf,v
retrieving revision 1.11
diff -u -p -r1.11 smtpd.conf
--- smtpd.conf  4 Jun 2018 21:10:58 -   1.11
+++ smtpd.conf  25 Nov 2019 07:28:48 -
@@ -15,5 +15,5 @@ action "relay" relay
 # Uncomment the following to accept external mail for domain "example.org"
 #
 # match from any for domain "example.org" action "local"
-match for local action "local"
-match for any action "relay"
+match from local for local action "local"
+match from local for any action "relay"



-- 
Gilles Chehade @poolpOrg

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



[PATCH] attach pvclock with lower priority if tsc is unstable

2019-11-24 Thread Pratik Vyas

Hello tech@,

This diff attaches pvclock with lower priority (500) in case of unstable
tsc (PVCLOCK_FLAG_TSC_STABLE) instead of not attaching at all.

For reference current priorities,
tsc (variant)  : -2000
i8254  : 0
acpitimer  : 1000
acpihpet0  : 1000
pvclock (stable tsc)   : 1500
tsc (invariant, stable): 2000

--
Pratik

Index: sys/dev/pv/pvclock.c
===
RCS file: /home/cvs/src/sys/dev/pv/pvclock.c,v
retrieving revision 1.4
diff -u -p -a -u -r1.4 pvclock.c
--- sys/dev/pv/pvclock.c13 May 2019 15:40:34 -  1.4
+++ sys/dev/pv/pvclock.c25 Nov 2019 06:49:09 -
@@ -29,11 +29,14 @@
#include 

#include 
+#include 
#include 

#include 
#include 

+uint pvclock_lastcount;
+
struct pvclock_softc {
struct devicesc_dev;
void*sc_time;
@@ -141,21 +144,22 @@ pvclock_attach(struct device *parent, st
flags = ti->ti_flags;
} while (!pvclock_read_done(ti, version));

-   if ((flags & PVCLOCK_FLAG_TSC_STABLE) == 0) {
-   wrmsr(KVM_MSR_SYSTEM_TIME, pa & ~PVCLOCK_SYSTEM_TIME_ENABLE);
-   km_free(sc->sc_time, PAGE_SIZE, _any, _zero);
-   printf(": unstable clock\n");
-   return;
-   }
-
sc->sc_tc = _timecounter;
sc->sc_tc->tc_name = DEVNAME(sc);
sc->sc_tc->tc_frequency = 10ULL;
sc->sc_tc->tc_priv = sc;

+   pvclock_lastcount = 0;
+
/* Better than HPET but below TSC */
sc->sc_tc->tc_quality = 1500;

+   if ((flags & PVCLOCK_FLAG_TSC_STABLE) == 0) {
+   /* if tsc is not stable, set a lower priority */
+   /* Better than i8254 but below HPET */
+   sc->sc_tc->tc_quality = 500;
+   }
+
tc_init(sc->sc_tc);

printf("\n");
@@ -216,10 +220,6 @@ pvclock_get_timecount(struct timecounter
flags = ti->ti_flags;
} while (!pvclock_read_done(ti, version));

-   /* This bit must be set as we attached based on the stable flag */
-   if ((flags & PVCLOCK_FLAG_TSC_STABLE) == 0)
-   panic("%s: unstable result on stable clock", DEVNAME(sc));
-
/*
 * The algorithm is described in
 * linux/Documentation/virtual/kvm/msr.txt
@@ -230,6 +230,14 @@ pvclock_get_timecount(struct timecounter
else
delta <<= shift;
ctr = ((delta * mul_frac) >> 32) + system_time;
+
+   if ((flags & PVCLOCK_FLAG_TSC_STABLE) != 0)
+   return (ctr);
+
+   if (ctr < pvclock_lastcount)
+   return (pvclock_lastcount);
+
+   atomic_swap_uint(_lastcount, ctr);

return (ctr);
}



Re: OpenSSH U2F/FIDO support in base

2019-11-24 Thread Christian Weisgerber
Ross L Richardson:

> Question: Given that the private key file contains only a "key handle",
> what's the significance of setting a passphrase for it?  Is there enough
> information in it for that to be considered a "factor" in multi-factor auth?

TL;DR: In practice, yes.

A U2F authenticator does not hold a single key pair.  Instead it
can issue numerous authentication key pairs.  The handle is required
so the authenticator can retrieve the relevant private key.  How
exactly it does this is up to the implementor.

Each time a key pair is requested, an authenticator could stash the
private key in an array in internal storage and return a simple
index as key handle.  This would allow all private key indices to
be enumerated.  An attacker in possession of both the authenticator
and the public key could re-create the key handle by trying all
private keys until finding the one matching the public key.

A different approach allows supporting an unlimited number of key
pairs without requiring any internal storage.  The authenticator
can use a device-internal secret and the key handle to rederive the
private key each time a signature is requested.  For instance, the
key handle could contain the actual private key encrypted with the
authenticator's master key.  This is explicitly mentioned in the
U2F spec.  In such a case, possession of the authenticator alone
would be worthless without the SSH private key file.

I expect the latter scheme to be the norm, but I can't be certain
what kind of U2F authenticators are out there.  The key handle
returned by the Yubico Security Key is a 64-byte blob.

-- 
Christian "naddy" Weisgerber  na...@mips.inka.de



Re: smtpd remove implicit ruleset behavior

2019-11-24 Thread Quentin Rameau
> Hello,

Hi Gilles,

> Is there strong objection to move to a mode where implicit notation will
> no longer be allowed ?

I think that's a good idea regarding the growing of matching facilities
in smtpd, as long as we make sure the removal of implicit "from local"
doesn't turn somehow into an implicit "from any", which it won't anyway.



Re: smtpd remove implicit ruleset behavior

2019-11-24 Thread Consus
On 10:54 Sun 24 Nov, Gilles Chehade wrote:
> Hello,
> 
> Ten years ago, it seemed a very neat idea that OpenSMTPD would have some
> implicit defaults to avoid people creating open relays.
> 
> Back then I was trying to make the smtpd.conf as compact as possible and
> came up with the very nice idea of "implicit local" so that we would get
> a very compact:
> 
>   accept for any relay
> 
> which would not be an open relay as it translated to:
> 
>   accept from local for any relay
> 
> 
> This idea was carried when we moved the syntax to match/action.
> 
> I think this was an error from the beginning and we should only have the
> explicit notation as I see a trend in people coming up with:
> 
>   match for domain foobar.org action "deliver"
> 
> which, read loud, seems to imply that mail for domain foobar.org will be
> delivered but which actually fails because it translates as:
> 
>   match from local for domain foobar.org action "deliver"
> 
> and actually limits the scope to local users...
> 
> People keep making this mistake over and over which as safe as it is, is
> a serious hint that the mistake is on smtpd's side.
> 
> 
> Is there strong objection to move to a mode where implicit notation will
> no longer be allowed ?
> 
> 
> This could start with us adding the explicit notation to default config,
> then put a startup warning in the next release so configurations are not
> broken but people spot that this is no longer encouraged and we can then
> later kill it.

Please do. 



smtpd remove implicit ruleset behavior

2019-11-24 Thread Gilles Chehade
Hello,

Ten years ago, it seemed a very neat idea that OpenSMTPD would have some
implicit defaults to avoid people creating open relays.

Back then I was trying to make the smtpd.conf as compact as possible and
came up with the very nice idea of "implicit local" so that we would get
a very compact:

  accept for any relay

which would not be an open relay as it translated to:

  accept from local for any relay


This idea was carried when we moved the syntax to match/action.

I think this was an error from the beginning and we should only have the
explicit notation as I see a trend in people coming up with:

  match for domain foobar.org action "deliver"

which, read loud, seems to imply that mail for domain foobar.org will be
delivered but which actually fails because it translates as:

  match from local for domain foobar.org action "deliver"

and actually limits the scope to local users...

People keep making this mistake over and over which as safe as it is, is
a serious hint that the mistake is on smtpd's side.


Is there strong objection to move to a mode where implicit notation will
no longer be allowed ?


This could start with us adding the explicit notation to default config,
then put a startup warning in the next release so configurations are not
broken but people spot that this is no longer encouraged and we can then
later kill it.


-- 
Gilles Chehade @poolpOrg

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