Re: pf: honor quick on anchor rules

2018-10-15 Thread Fabian Mueller-Knapp
On 18-10-15 23:45:58, Alexandr Nedvedicky wrote:
> Hello,
> 
> > That is still different from the original (2006) behaviour which
> > would terminate ruleset-evaluation IFF any rules inside the anchor
> > actually match. 
> 
> Perhaps I still misunderstand the whole anchor thing.
> 
> let me clarify the proposed change by two examples. Ruleset below allows
> new connection because pass rule matches in 'anchor quick' block (the
> 'block all' rule is never reached).
> 
>   pass all
>   anchor quick {
>   pass all
>   }
>   block all
> 
> ruleset below does not allow connection, because the 'anchor quick' is
> empty. As such it does not match packet, therefore the whole anchor does
> not match, thus 'quick' does not count. PF continues to apply rules. It
> finds 'block all' for packet in question. Packet is blocked and game
> is over, the connection attempt times out.
> 
>   pass all
>   anchor quick {
>   }
>   block all
> 
> Do those two examples follow the behavior described in pf.conf(5) or not?
> if not what is the expected behavior for those two simple rulesets?

In my understanding they do, but you are missing the point. Given the
following ruleset:

pass
anchor quick {
block inet6
}
block

With the current fix ruleset-evaluation terminates after the anchor,
before the last block even *if no ipv6-traffic was encountered*. That is,
no rule inside the (not empty) anchor matched, but evaluation is stopped
nevertheless.

On 6.1 the anchor is ignored if no rule inside matches and evaluation
continues, and that is what the original commit-log states:

>>   Allow the 'quick' keyword on an anchor. IFF there is a matching 
>> rule inside
>>   the anchor, terminate ruleset evaluation when stepping out of the 
>> anchor.
>>
>>   This means that if you absolutely want the anchor to be terminal, 
>> you
>>   probably want to use a 'block all' or 'pass all' rule at the start 
>> of the
>>   anchor.

> 
> thanks a lot for your help
> regards
> sashan 

Regards
Fabian



AEAD_XChaCha20_Poly1305 support for libressl libcrypto

2018-10-15 Thread David Gwynne
XChaCha20 is a variant of ChaCha20 that provides an extended nonce,
which in turn makes it more usable when using randomly generated nonces
for long running connections. The longer nonce makes it less likely that
a randomly generated one will collide.

I want this cos a particular protocol uses AEAD_XChaCha20_Poly1305. It
also uses blake2, but that diff adds files so I'll save it for later.

This is based on draft-arciszewski-xchacha-02, which I only just noticed
was updated recently. The test vectors in particular were useful when
building this code. One thing to note is that the code doesn't
stricly follow the spec, since this spec assumes the IETF specified
chacha, which has a longer nonce and shorter counter than the original
chacha we're using. Even if we pick up the IETF chacha, I think I'd
leave the code the way it is cos it means less playing around with the
nonce for the subsequent chacha calls.

There are some future opportunities to reduce duplication in the code.
The actual encryption and authentication of the AEAD bits could be
factored out, and the chacha rounds could be split out into their own
function.

ok?

Index: lib/libcrypto/Symbols.list
===
RCS file: /cvs/src/lib/libcrypto/Symbols.list,v
retrieving revision 1.76
diff -u -p -r1.76 Symbols.list
--- lib/libcrypto/Symbols.list  12 Sep 2018 06:35:38 -  1.76
+++ lib/libcrypto/Symbols.list  15 Oct 2018 21:06:51 -
@@ -671,6 +679,7 @@ CRYPTO_get_mem_ex_functions
 CRYPTO_get_mem_functions
 CRYPTO_get_new_dynlockid
 CRYPTO_get_new_lockid
+CRYPTO_hchacha_20
 CRYPTO_is_mem_check_on
 CRYPTO_lock
 CRYPTO_malloc
@@ -712,6 +721,7 @@ CRYPTO_set_mem_ex_functions
 CRYPTO_set_mem_functions
 CRYPTO_strdup
 CRYPTO_thread_id
+CRYPTO_xchacha_20
 CRYPTO_xts128_encrypt
 Camellia_cbc_encrypt
 Camellia_cfb128_encrypt
@@ -1476,6 +1486,7 @@ EVP_add_digest
 EVP_aead_aes_128_gcm
 EVP_aead_aes_256_gcm
 EVP_aead_chacha20_poly1305
+EVP_aead_xchacha20_poly1305
 EVP_aes_128_cbc
 EVP_aes_128_cbc_hmac_sha1
 EVP_aes_128_ccm
Index: lib/libcrypto/chacha/chacha-merged.c
===
RCS file: /cvs/src/lib/libcrypto/chacha/chacha-merged.c,v
retrieving revision 1.8
diff -u -p -r1.8 chacha-merged.c
--- lib/libcrypto/chacha/chacha-merged.c13 Aug 2017 16:55:31 -  
1.8
+++ lib/libcrypto/chacha/chacha-merged.c15 Oct 2018 21:06:52 -
@@ -277,3 +277,49 @@ chacha_encrypt_bytes(chacha_ctx *x, cons
m += 64;
}
 }
+
+void
+CRYPTO_hchacha_20(unsigned char subkey[32], const unsigned char key[32],
+const unsigned char nonce[16])
+{
+   uint32_t x[16];
+   int i;
+
+   x[0] = U8TO32_LITTLE(sigma + 0);
+   x[1] = U8TO32_LITTLE(sigma + 4);
+   x[2] = U8TO32_LITTLE(sigma + 8);
+   x[3] = U8TO32_LITTLE(sigma + 12);
+   x[4] = U8TO32_LITTLE(key + 0);
+   x[5] = U8TO32_LITTLE(key + 4);
+   x[6] = U8TO32_LITTLE(key + 8);
+   x[7] = U8TO32_LITTLE(key + 12);
+   x[8] = U8TO32_LITTLE(key + 16);
+   x[9] = U8TO32_LITTLE(key + 20);
+   x[10] = U8TO32_LITTLE(key + 24);
+   x[11] = U8TO32_LITTLE(key + 28);
+   x[12] = U8TO32_LITTLE(nonce + 0);
+   x[13] = U8TO32_LITTLE(nonce + 4);
+   x[14] = U8TO32_LITTLE(nonce + 8);
+   x[15] = U8TO32_LITTLE(nonce + 12);
+
+   for (i = 20; i > 0; i -= 2) {
+   QUARTERROUND(x[0], x[4], x[8], x[12])
+   QUARTERROUND(x[1], x[5], x[9], x[13])
+   QUARTERROUND(x[2], x[6], x[10], x[14])
+   QUARTERROUND(x[3], x[7], x[11], x[15])
+   QUARTERROUND(x[0], x[5], x[10], x[15])
+   QUARTERROUND(x[1], x[6], x[11], x[12])
+   QUARTERROUND(x[2], x[7], x[8], x[13])
+   QUARTERROUND(x[3], x[4], x[9], x[14])
+   }
+
+   U32TO8_LITTLE(subkey + 0, x[0]);
+   U32TO8_LITTLE(subkey + 4, x[1]);
+   U32TO8_LITTLE(subkey + 8, x[2]);
+   U32TO8_LITTLE(subkey + 12, x[3]);
+
+   U32TO8_LITTLE(subkey + 16, x[12]);
+   U32TO8_LITTLE(subkey + 20, x[13]);
+   U32TO8_LITTLE(subkey + 24, x[14]);
+   U32TO8_LITTLE(subkey + 28, x[15]);
+}
Index: lib/libcrypto/chacha/chacha.c
===
RCS file: /cvs/src/lib/libcrypto/chacha/chacha.c,v
retrieving revision 1.7
diff -u -p -r1.7 chacha.c
--- lib/libcrypto/chacha/chacha.c   9 Dec 2015 14:07:55 -   1.7
+++ lib/libcrypto/chacha/chacha.c   15 Oct 2018 21:06:52 -
@@ -75,3 +75,13 @@ CRYPTO_chacha_20(unsigned char *out, con
 
chacha_encrypt_bytes(, in, out, (uint32_t)len);
 }
+
+void
+CRYPTO_xchacha_20(unsigned char *out, const unsigned char *in, size_t len,
+const unsigned char key[32], const unsigned char iv[24])
+{
+   uint8_t subkey[32];
+
+   CRYPTO_hchacha_20(subkey, key, iv);
+   CRYPTO_chacha_20(out, in, len, subkey, iv + 16, 0);
+}
Index: lib/libcrypto/chacha/chacha.h

Re: spf walk: lookup aaaa records with "a" mechanism

2018-10-15 Thread Todd T. Fries
In principal I like this.

In practice, I note something is missing.  I get a different output:

$ echo netsend.nl | smtpctl spf walk
32.1.9.129
32.1.9.132
80.127.98.234
80.127.135.115

I also note the dns is being requested, as per below, just not printed for some 
reason.

09:42:12.175535 d0:7e:35:12:9a:03 f8:18:97:94:b1:cd 0800 70: 192.168.1.85.11695 
> 8.8.8.8.53: [udp sum ok] 49902+ TXT? netsend.nl.(28) (ttl 64, id 37703, len 
56)
09:42:12.504853 f8:18:97:94:b1:cd d0:7e:35:12:9a:03 0800 112: 8.8.8.8.53 > 
192.168.1.85.11695: [udp sum ok] 49902 1/0/0 netsend.nl. TXT "v=spf1 
a:smtp.netsend.nl -all[|domain] (ttl 119, id 12691, len 98)
09:42:12.505483 d0:7e:35:12:9a:03 f8:18:97:94:b1:cd 0800 75: 192.168.1.85.45977 
> 8.8.8.8.53: [udp sum ok] 37034+ A? smtp.netsend.nl.(33) (ttl 64, id 19808, 
len 61)
09:42:12.505526 d0:7e:35:12:9a:03 f8:18:97:94:b1:cd 0800 75: 192.168.1.85.31571 
> 8.8.8.8.53: [udp sum ok] 58052+ ? smtp.netsend.nl.(33) (ttl 64, id 22845, 
len 61)
09:42:12.668825 f8:18:97:94:b1:cd d0:7e:35:12:9a:03 0800 131: 8.8.8.8.53 > 
192.168.1.85.31571: [udp sum ok] 58052 2/0/0 smtp.netsend.nl.  
2001:981:8a34:1:80:127:135:115, smtp.netsend.nl.  
2001:984:6a6f:1:468a:5bff:fed9:87(89) (ttl 119, id 58881, len 117)
09:42:12.748121 f8:18:97:94:b1:cd d0:7e:35:12:9a:03 0800 107: 8.8.8.8.53 > 
192.168.1.85.45977: [udp sum ok] 37034 2/0/0 smtp.netsend.nl. A 80.127.98.234, 
smtp.netsend.nl. A 80.127.135.115(65) (ttl 56, id 60155, len 93)

Thanks,

Penned by Tim Kuijsten on 20181014 18:08.06, we have:
| Hi,
| 
| When the "a" designated sender mechanism is used in an spf txt record, both
| v4 and v6 addresses are matched according to [1], so let `smtpctl spf walk`
| resolve both A and  records.
| 
| Current output:
| $ echo netsend.nl | smtpctl spf walk
| 80.127.135.115
| 80.127.98.234
| 
| Expected output:
| $ echo netsend.nl | ./smtpctl spf walk
| 80.127.135.115
| 80.127.98.234
| 2001:981:8a34:1:80:127:135:115
| 2001:984:6a6f:1:468a:5bff:fed9:87
| 
| -Tim
| 
| [1] https://tools.ietf.org/html/rfc7208#section-5.3

| diff --git a/usr.sbin/smtpd/spfwalk.c b/usr.sbin/smtpd/spfwalk.c
| index c4ce2e3d891..22b057963f9 100644
| --- a/usr.sbin/smtpd/spfwalk.c
| +++ b/usr.sbin/smtpd/spfwalk.c
| @@ -192,6 +192,7 @@ dispatch_txt(struct dns_rr *rr)
|   }
|   if (strncasecmp("a:", *ap, 2) == 0) {
|   lookup_record(T_A, *(ap) + 2, dispatch_a);
| + lookup_record(T_, *(ap) + 2, dispatch_);
|   continue;
|   }
|   if (strncasecmp("exists:", *ap, 7) == 0) {


-- 
Todd T. Fries . http://todd.fries.net/pgp.txt . @unix2mars . github:toddfries



switchctl(8): Add ability to reference control device for 'dump' option

2018-10-15 Thread Ayaka Koshibe
Hi all,

Currently, switchctl(8) is able to query a switch for information by specifying
the switch's network address. The following adds support for querying a
switch(4) instance via its control device:


$ doas switchctl switch /dev/switch0 dump features
any > /dev/switch0: version 1_3 type FEATURES_REQUEST length 8 xid 1 
/dev/switch0 > any: version 1_3 type FEATURES_REPLY length 32 xid 1
datapath_id 0x01 nbuffers 0 ntables 254 aux_id 0 
capabilities 0x0f


While testing this, I also found that the OpenFlow version isn't being set,
preventing feature request queries. A (5-line) fix is also included here.

Since switchctl(8) behaves like a controller in this case, a switch can't be
connected to switchd(8) while it is being queried in this way.

Feedback appreciated.


Thanks,
Ayaka

Index: ofpclient.c
===
RCS file: /cvs/src/usr.sbin/switchctl/ofpclient.c,v
retrieving revision 1.6
diff -u -p -u -r1.6 ofpclient.c
--- ofpclient.c 9 Jan 2017 16:42:14 -   1.6
+++ ofpclient.c 15 Oct 2018 21:47:40 -
@@ -57,6 +57,7 @@ ofpclient(struct parse_result *res, stru
struct switch_connection con;
struct switchd   sc;
int  s, timeout;
+   struct sockaddr_un  *un;
 
memset(, 0, sizeof(sc));
sc.sc_tap = -1;
@@ -93,6 +94,17 @@ ofpclient(struct parse_result *res, stru
 
con.con_fd = s;
break;
+   case SWITCH_CONN_LOCAL:
+   un = (struct sockaddr_un *)>uri.swa_addr;
+
+   if (strncmp(un->sun_path, "/dev/switch",
+   strlen("/dev/switch")) != 0)
+   fatalx("device path not supported");
+
+   if ((s = open(un->sun_path, O_RDWR | O_NONBLOCK)) == -1)
+   fatalx("failed to open %s", un->sun_path);
+   con.con_fd = s;
+   break;
default:
fatalx("connect type not supported");
}
@@ -205,6 +217,12 @@ ofpclient_read(struct switch_connection 
if (ofp_validate(con->con_sc,
>con_peer, >con_local, oh, ibuf, oh->oh_version) != 0)
fatal("ofp_validate");
+
+   if (con->con_state == OFP_STATE_CLOSED) {
+   con->con_version = oh->oh_version;
+   ofp_recv_hello(con->con_sc, con, oh, ibuf);
+   con->con_state = OFP_STATE_ESTABLISHED;
+   }
 
ibuf_free(ibuf);
 }
Index: parser.c
===
RCS file: /cvs/src/usr.sbin/switchctl/parser.c,v
retrieving revision 1.8
diff -u -p -u -r1.8 parser.c
--- parser.c1 Aug 2017 13:11:11 -   1.8
+++ parser.c15 Oct 2018 21:47:40 -
@@ -452,7 +452,13 @@ match_token(char *word, const struct tok
res.uri.swa_type = SWITCH_CONN_TCP;
else if (strncmp(word, "tls:", len) == 0)
res.uri.swa_type = SWITCH_CONN_TLS;
-   else {
+   else if (strncmp(word, "/dev", len) == 0) {
+   res.uri.swa_type = SWITCH_CONN_LOCAL;
+   parse_addr(word, _addr);
+   match++;
+   t = [i];
+   break;
+   } else {
/* set the default */
res.uri.swa_type = SWITCH_CONN_TCP;
len = 0;
Index: switchctl.8
===
RCS file: /cvs/src/usr.sbin/switchctl/switchctl.8,v
retrieving revision 1.5
diff -u -p -u -r1.5 switchctl.8
--- switchctl.8 15 Nov 2016 09:30:03 -  1.5
+++ switchctl.8 15 Oct 2018 21:47:40 -
@@ -69,7 +69,9 @@ Close the client connection to a remote 
 .Xr switch 4
 control device.
 .It Cm dump Ar address request
-Request information from a remote switch.
+Request information from a remote switch or a
+.Xr switch 4
+control device.
 .Nm
 will send an OpenFlow request to the remote switch that is specified by
 .Ar address .
Index: switchctl.c
===
RCS file: /cvs/src/usr.sbin/switchctl/switchctl.c,v
retrieving revision 1.7
diff -u -p -u -r1.7 switchctl.c
--- switchctl.c 31 Jan 2017 05:53:08 -  1.7
+++ switchctl.c 15 Oct 2018 21:47:40 -
@@ -125,8 +125,10 @@ main(int argc, char *argv[])
 * dns - for parsehostport() in the device spec.
 * inet - for handling tcp connections with OpenFlow peers.
 * unix - for opening the control socket.
+* rpath - for reading from the /dev/switch device.
+* wpath - for accessing the /dev/switch device.
 */
-   if 

Re: pf: honor quick on anchor rules

2018-10-15 Thread Alexandr Nedvedicky
Hello,

> That is still different from the original (2006) behaviour which
> would terminate ruleset-evaluation IFF any rules inside the anchor
> actually match. 

Perhaps I still misunderstand the whole anchor thing.

let me clarify the proposed change by two examples. Ruleset below allows
new connection because pass rule matches in 'anchor quick' block (the
'block all' rule is never reached).

pass all
anchor quick {
pass all
}
block all

ruleset below does not allow connection, because the 'anchor quick' is
empty. As such it does not match packet, therefore the whole anchor does
not match, thus 'quick' does not count. PF continues to apply rules. It
finds 'block all' for packet in question. Packet is blocked and game
is over, the connection attempt times out.

pass all
anchor quick {
}
block all

Do those two examples follow the behavior described in pf.conf(5) or not?
if not what is the expected behavior for those two simple rulesets?

thanks a lot for your help
regards
sashan 



Re: pf: honor quick on anchor rules

2018-10-15 Thread Fabian Mueller-Knapp
On 18-10-15 13:38:32, Alexandr Nedvedicky wrote:
> Hello,
> 
> I just got back on-line after a week. Thank you all for detailed analysis.
> This particular bug, which Klemens tries to fix is caused by my commit 1.1024:
> 
> - percpu anchor stacks
>   we actually don't need to pre-allocate per_anchor_stack[], if we use
>   a 'natural' recursion, when doing anchor tree traversal.
> 
> Diff below fixes the case reported by Fabian:
> 
> > snap# uname -a
> > OpenBSD snap.my.domain 6.4 GENERIC#333 amd64
> > snap# pfctl -sr 
> >
> > pass all flags S/SA
> > anchor quick all {
> > }
> > block drop all
> > 
> 
> The idea is to override 'anchor quick' for empty rulesets. If the particular
> anchor, which pf_step_into_anchor() is about to enter, is empty, then the
> pf_step_into_anchor() bails out immediately with PF_TEST_OK result to keep
> PF processing the rules. I think this was the missing piece in mosaic.

That is still different from the original (2006) behaviour which
would terminate ruleset-evaluation IFF any rules inside the anchor
actually match. 

> 
> OK ?
> 
> sorry for inconveniences
> regards
> sashan
> 
> 8<---8<---8<--8<
> diff --git a/sys/net/pf.c b/sys/net/pf.c
> index 0bdf90a8d13..72841c9b8f0 100644
> --- a/sys/net/pf.c
> +++ b/sys/net/pf.c
> @@ -3126,7 +3126,7 @@ pf_step_into_anchor(struct pf_test_ctx *ctx, struct 
> pf_rule *r)
>   break;
>   }
>   }
> - } else {
> + } else if (!TAILQ_EMPTY(r->anchor->ruleset.rules.active.ptr)) {
>   rv = pf_match_rule(ctx, >anchor->ruleset);
>   /*
>* Unless there was an error inside the anchor,
> @@ -3134,7 +3134,8 @@ pf_step_into_anchor(struct pf_test_ctx *ctx, struct 
> pf_rule *r)
>*/
>   if (rv != PF_TEST_FAIL && r->quick == PF_TEST_QUICK)
>   rv = PF_TEST_QUICK;
> - }
> + } else
> + rv = PF_TEST_OK;
>  
>   ctx->depth--;
>  



Re: spf walk: lookup aaaa records with "a" mechanism

2018-10-15 Thread Gilles Chehade
On Mon, Oct 15, 2018 at 01:08:06AM +0200, Tim Kuijsten wrote:
> Hi,
> 

Hi,


> When the "a" designated sender mechanism is used in an spf txt record, both
> v4 and v6 addresses are matched according to [1], so let `smtpctl spf walk`
> resolve both A and  records.
> 
> [...]
>
> -Tim
> 
> [1] https://tools.ietf.org/html/rfc7208#section-5.3

Correct, unfortunately this comes slightly too late for 6.4

Thanks for your diff


> diff --git a/usr.sbin/smtpd/spfwalk.c b/usr.sbin/smtpd/spfwalk.c
> index c4ce2e3d891..22b057963f9 100644
> --- a/usr.sbin/smtpd/spfwalk.c
> +++ b/usr.sbin/smtpd/spfwalk.c
> @@ -192,6 +192,7 @@ dispatch_txt(struct dns_rr *rr)
>   }
>   if (strncasecmp("a:", *ap, 2) == 0) {
>   lookup_record(T_A, *(ap) + 2, dispatch_a);
> + lookup_record(T_, *(ap) + 2, dispatch_);
>   continue;
>   }
>   if (strncasecmp("exists:", *ap, 7) == 0) {


-- 
Gilles Chehade

https://www.poolp.org  @poolpOrg



Re: doas(1): Exit Status

2018-10-15 Thread Theo de Raadt
Stefan Filipek  wrote:

> According to this discussion, there is an issue with nice(1)
> documentation, which the patch was trying to emulate:
> 
> > EXIT STATUS
> > ...
> > Otherwise, the exit status of nice shall be that of utility.
> 
> Which is incorrect, since 'nice' doesn't exit in this case at all,
> just like 'doas'.

Indeed, the noun is "exit status"

But not precisely like doas, as discussed before.  doas has cases where
it exits and provides an exit status, itself.

> Perhaps this line should be omitted, or perhaps removing 'of nice' (or
> some other rewording) would make it more accurate while still being
> helpful to any user who may not know the full implementation details.

Yes, I would agree with removing "of nice".

> In any case, doas(1) and nice(1) are documented inconsistently for the
> same behavior.

Quite a high bar you are setting there.  It is possible these wordings
could be improved (in particular through vagueness), but it doesn't
follow all things must use all the same words.

In particular, doas is special since it has two cases: it returns error
indication itself for some operations, in other cases it has done an
exec and the error comes from the replacement program.



Re: user mod -u lead to segfault

2018-10-15 Thread Alexander Bluhm
On Sun, Oct 14, 2018 at 09:58:08PM -0600, Todd C. Miller wrote:
> Updated diff that adds wrapper functions for improved readability.
> It's arguable that user(8) should check the local master.passwd and
> group files directly but that's a discussion for another time.

OK bluhm@

> Index: usr.sbin/user/user.c
> ===
> RCS file: /cvs/src/usr.sbin/user/user.c,v
> retrieving revision 1.122
> diff -u -p -u -r1.122 user.c
> --- usr.sbin/user/user.c  26 Sep 2018 14:54:58 -  1.122
> +++ usr.sbin/user/user.c  15 Oct 2018 03:54:42 -
> @@ -319,6 +319,22 @@ copydotfiles(char *skeldir, char *dir)
>   return n;
>  }
>  
> +/* returns 1 if the specified gid exists in the group file, else 0 */
> +static int
> +gid_exists(gid_t gid)
> +{
> +return group_from_gid(gid, 1) != NULL;
> +}
> +
> +/* return 1 if the specified group exists in the group file, else 0 */
> +static int
> +group_exists(const char *group)
> +{
> +gid_t gid;
> +
> +return gid_from_group(group, ) != -1;
> +}
> +
>  /* create a group entry with gid `gid' */
>  static int
>  creategid(char *group, gid_t gid, const char *name)
> @@ -332,7 +348,7 @@ creategid(char *group, gid_t gid, const 
>   int wroteit = 0;
>   size_t  len;
>  
> - if (getgrnam(group) != NULL) {
> + if (group_exists(group)) {
>   warnx("group `%s' already exists", group);
>   return 0;
>   }
> @@ -673,7 +689,7 @@ static int
>  getnextgid(uid_t *gidp, uid_t lo, uid_t hi)
>  {
>   for (*gidp = lo ; *gidp < hi ; *gidp += 1) {
> - if (getgrgid((gid_t)*gidp) == NULL) {
> + if (!gid_exists((gid_t)*gidp)) {
>   return 1;
>   }
>   }
> @@ -857,14 +873,30 @@ read_defaults(user_t *up)
>   up->u_defrc = up->u_rc;
>  }
>  
> +/* return 1 if the specified uid exists in the passwd file, else 0 */
> +static int
> +uid_exists(uid_t uid)
> +{
> +return user_from_uid(uid, 1) != NULL;
> +}
> +
> +/* return 1 if the specified user exists in the passwd file, else 0 */
> +static int
> +user_exists(const char *user)
> +{
> +uid_t uid;
> +
> +return uid_from_user(user, ) != -1;
> +}
> +
>  /* return the next valid unused uid */
>  static int
>  getnextuid(int sync_uid_gid, uid_t *uid, uid_t low_uid, uid_t high_uid)
>  {
>   for (*uid = low_uid ; *uid <= high_uid ; (*uid)++) {
> - if (getpwuid((uid_t)(*uid)) == NULL && *uid != NOBODY_UID) {
> + if (!uid_exists((uid_t)*uid) && *uid != NOBODY_UID) {
>   if (sync_uid_gid) {
> - if (getgrgid((gid_t)(*uid)) == NULL) {
> + if (!gid_exists((gid_t)*uid)) {
>   return 1;
>   }
>   } else {
> @@ -1048,14 +1080,14 @@ adduser(char *login_name, user_t *up)
>   }
>   }
>   /* check uid isn't already allocated */
> - if (!(up->u_flags & F_DUPUID) && getpwuid((uid_t)(up->u_uid)) != NULL) {
> + if (!(up->u_flags & F_DUPUID) && uid_exists((uid_t)up->u_uid)) {
>   close(ptmpfd);
>   pw_abort();
>   errx(EXIT_FAILURE, "uid %u is already in use", up->u_uid);
>   }
>   /* if -g=uid was specified, check gid is unused */
>   if (sync_uid_gid) {
> - if (getgrgid((gid_t)(up->u_uid)) != NULL) {
> + if (gid_exists((gid_t)up->u_uid)) {
>   close(ptmpfd);
>   pw_abort();
>   errx(EXIT_FAILURE, "gid %u is already in use", 
> up->u_uid);
> @@ -1070,7 +1102,7 @@ adduser(char *login_name, user_t *up)
>   gid = grp->gr_gid;
>   }
>   /* check name isn't already in use */
> - if (!(up->u_flags & F_DUPUID) && getpwnam(login_name) != NULL) {
> + if (!(up->u_flags & F_DUPUID) && user_exists(login_name)) {
>   close(ptmpfd);
>   pw_abort();
>   errx(EXIT_FAILURE, "already a `%s' user", login_name);
> @@ -1186,8 +1218,7 @@ adduser(char *login_name, user_t *up)
>   (void) asystem("%s -R u+w %s", CHMOD, home);
>   }
>   }
> - if (strcmp(up->u_primgrp, "=uid") == 0 &&
> - getgrnam(login_name) == NULL &&
> + if (strcmp(up->u_primgrp, "=uid") == 0 && !group_exists(login_name) &&
>   !creategid(login_name, gid, "")) {
>   close(ptmpfd);
>   pw_abort();
> @@ -1369,7 +1400,6 @@ moduser(char *login_name, char *newlogin
>   int ptmpfd;
>   int rval;
>   int i;
> - uid_t   uid;
>  
>   if (!valid_login(newlogin)) {
>   errx(EXIT_FAILURE, "`%s' is not a valid login name", 
> login_name);
> @@ -1429,7 +1459,7 @@ moduser(char *login_name, char *newlogin
>   if (up->u_flags & F_USERNAME) {
>   

Re: doas(1): Exit Status

2018-10-15 Thread Stefan Filipek
According to this discussion, there is an issue with nice(1)
documentation, which the patch was trying to emulate:

> EXIT STATUS
> ...
> Otherwise, the exit status of nice shall be that of utility.

Which is incorrect, since 'nice' doesn't exit in this case at all,
just like 'doas'.

Perhaps this line should be omitted, or perhaps removing 'of nice' (or
some other rewording) would make it more accurate while still being
helpful to any user who may not know the full implementation details.

In any case, doas(1) and nice(1) are documented inconsistently for the
same behavior.



Re: top cpu stats are wrong with hyper threading

2018-10-15 Thread Scott Cheloha
On Fri, Oct 05, 2018 at 04:27:17PM +0200, Moritz Buhl wrote:
> 
> Due to the SMT stuff the output of top showed the first few cpus instead
> of the ones that are actually active.
> 
> To reproduce the bad output:
> Use a machine with hyper therading, top should show half the cpus, of
> which every second is disabled.
> 
> The following diff skips the disabled cpus and disabling/reenabling the
> cores with hw.smt also works.
> 
> The only problem is that the lines reserved for cpu-stats does not
> change with reenabling. Refreshing the output with '1' or resizing the
> window should help.

Sorry for the late reply.

The SMT-related changes to both systat(1) and top(1) have been reverted.

If you have a recent snapshot, from say within the last week, could you
confirm that your top(1) output is now OK?



Re: pf/pfctl: use PFR_FB_NONE consistently

2018-10-15 Thread Alexandr Nedvedicky
Hello,

On Sat, Oct 13, 2018 at 10:56:21PM +0200, Klemens Nanni wrote:
> Replace hardcoded 0 and implicit checks with enum as done in all other
> use cases of `pfra_fback'.

either way is fine with me, so I'm not going to object your change.

OK sashan



Re: pf: honor quick on anchor rules

2018-10-15 Thread Alexandr Nedvedicky
Hello,

I just got back on-line after a week. Thank you all for detailed analysis.
This particular bug, which Klemens tries to fix is caused by my commit 1.1024:

- percpu anchor stacks
  we actually don't need to pre-allocate per_anchor_stack[], if we use
  a 'natural' recursion, when doing anchor tree traversal.

Diff below fixes the case reported by Fabian:

> snap# uname -a
> OpenBSD snap.my.domain 6.4 GENERIC#333 amd64
> snap# pfctl -sr   
>  
> pass all flags S/SA
> anchor quick all {
> }
> block drop all
> 

The idea is to override 'anchor quick' for empty rulesets. If the particular
anchor, which pf_step_into_anchor() is about to enter, is empty, then the
pf_step_into_anchor() bails out immediately with PF_TEST_OK result to keep
PF processing the rules. I think this was the missing piece in mosaic.

OK ?

sorry for inconveniences
regards
sashan

8<---8<---8<--8<
diff --git a/sys/net/pf.c b/sys/net/pf.c
index 0bdf90a8d13..72841c9b8f0 100644
--- a/sys/net/pf.c
+++ b/sys/net/pf.c
@@ -3126,7 +3126,7 @@ pf_step_into_anchor(struct pf_test_ctx *ctx, struct 
pf_rule *r)
break;
}
}
-   } else {
+   } else if (!TAILQ_EMPTY(r->anchor->ruleset.rules.active.ptr)) {
rv = pf_match_rule(ctx, >anchor->ruleset);
/*
 * Unless there was an error inside the anchor,
@@ -3134,7 +3134,8 @@ pf_step_into_anchor(struct pf_test_ctx *ctx, struct 
pf_rule *r)
 */
if (rv != PF_TEST_FAIL && r->quick == PF_TEST_QUICK)
rv = PF_TEST_QUICK;
-   }
+   } else
+   rv = PF_TEST_OK;
 
ctx->depth--;
 



Re: pkg_add -Dsnap not fetching any updates and packages on -current

2018-10-15 Thread Neeraj Pal
It worked.
yeah, I configured the timezone and forgot to check the date and time.
So, now after syncing the date and time with ntp servers, everything works fine.

Thank you Benno for the help.

openbsd$ nc -cvvv fastly.cdn.openbsd.org 443
Connection to fastly.cdn.openbsd.org 443 port [tcp/https] succeeded!
TLS handshake negotiated TLSv1.2/ECDHE-RSA-AES128-GCM-SHA256 with host
fastly.cdn.openbsd.org
Peer name: fastly.cdn.openbsd.org
Subject: /C=US/ST=California/L=San Francisco/O=Fastly,
Inc./CN=osff.map.fastly.net
Issuer: /C=BE/O=GlobalSign nv-sa/CN=GlobalSign CloudSSL CA - SHA256 - G3
Valid From: Fri Oct 12 04:38:13 2018
Valid Until: Mon Feb 25 00:55:45 2019
Cert Hash: 
SHA256:28dd8ee0a34e3760554aea2944d8cc8c13601282fd51382215da748b8b06a826
OCSP URL: http://ocsp2.globalsign.com/cloudsslsha2g3
OCSP Stapling: good
  response_status=0 cert_status=0 crl_reason=0
  this update: Mon Oct 15 05:50:32 2018
  next update: Fri Oct 19 05:50:32 2018
  revocation:
GET / HTTP/1.1
Host: fastly.cdn.openbsd.org

HTTP/1.1 200 OK
Content-Type: text/html
Last-Modified: Sat, 18 Aug 2018 17:48:08 GMT
Server: OpenBSD httpd
Backend-Name: 5GnZ0LBU5CzDw9NCjFbkjI--F_ftp_openbsd_org__primary_
Via: 1.1 varnish
Content-Length: 5206
Accept-Ranges: bytes
Date: Mon, 15 Oct 2018 07:28:48 GMT
Via: 1.1 varnish
Age: 0
Connection: keep-alive
X-Served-By: cache-sea1044-SEA, cache-bom18221-BOM
X-Cache: HIT, MISS
X-Cache-Hits: 1, 0
X-Timer: S1539588528.413228,VS0,VE267
On Mon, Oct 15, 2018 at 12:03 PM Sebastian Benoit  wrote:
>
> Neeraj Pal(neerajpa...@gmail.com) on 2018.10.15 10:36:16 +0530:
> > Hi there,
> >
> > Yesterday I installed OpenBSD 6.3-stable then upgraded it to OpenBSD
> > -current by downloading and copying bsd.rd file into /
> > Then, after that, I am trying to update the userland but it is
> > throwing an error but it was working fine previously like one or two
> > days before because I have done the same with my other machines.
> > It seems there is some problem with server-side?
> > If I have forgotten something or doing something wrong then please let me 
> > know.
> >
> > openbsd$ cat /etc/installurl
> > https://fastly.cdn.openbsd.org/pub/OpenBSD
> > openbsd$
> >
> > openbsd$ doas pkg_add -Dsnap -u
> > doas (b...@openbsd.my.domain) password:
> > https://fastly.cdn.openbsd.org/pub/OpenBSD/snapshots/packages/amd64/:
> > ftp: SSL write error: ocsp verify failed: ocsp response not current
> > https://fastly.cdn.openbsd.org/pub/OpenBSD/snapshots/packages/amd64/: empty
> > Couldn't find updates for axel-2.4p0 gettext-0.19.8.1p1 libiconv-1.14p3
> >
> > openbsd$ doas pkg_add -Dsnap -iv vim
> > doas (b...@openbsd.my.domain) password:
> > https://fastly.cdn.openbsd.org/pub/OpenBSD/snapshots/packages/amd64/:
> > ftp: SSL write error: ocsp verify failed: ocsp response not current
> > https://fastly.cdn.openbsd.org/pub/OpenBSD/snapshots/packages/amd64/: empty
> > Can't find vim
> > openbsd$
> >
> > openbsd$ doas pkg_add -Dsnap -iv wget
> > doas (b...@openbsd.my.domain) password:
> > https://fastly.cdn.openbsd.org/pub/OpenBSD/snapshots/packages/amd64/:
> > ftp: SSL write error: ocsp verify failed: ocsp response not current
> > https://fastly.cdn.openbsd.org/pub/OpenBSD/snapshots/packages/amd64/: empty
> > Can't find wget
>
> Hi,
>
> this could be a server side problem, or a problem with the time on your
> system.
>
> If you still can reproduce the problem, please check that your systems time
> is correct.
>
> If it is, show the output of
>
>  nc -cvvv fastly.cdn.openbsd.org 443
>
> It should look something like this:
>
> Connection to fastly.cdn.openbsd.org 443 port [tcp/https] succeeded!
> TLS handshake negotiated TLSv1.2/ECDHE-RSA-AES128-GCM-SHA256 with host
> fastly.cdn.openbsd.org
> Peer name: fastly.cdn.openbsd.org
> Subject: /C=US/ST=California/L=San Francisco/O=Fastly,
> Inc./CN=osff.map.fastly.net
> Issuer: /C=BE/O=GlobalSign nv-sa/CN=GlobalSign CloudSSL CA - SHA256 - G3
> Valid From: Fri Oct 12 01:08:13 2018
> Valid Until: Sun Feb 24 20:25:45 2019
> Cert Hash:
> SHA256:28dd8ee0a34e3760554aea2944d8cc8c13601282fd51382215da748b8b06a826
> OCSP URL: http://ocsp2.globalsign.com/cloudsslsha2g3
> OCSP Stapling: good
>   response_status=0 cert_status=0 crl_reason=0
>   this update: Mon Oct 15 02:20:32 2018
>   next update: Fri Oct 19 02:20:32 2018
>   revocation:
>
>
> At that point type
>
>  GET / HTTP/1.1
>  Host: fastly.cdn.openbsd.org
>  
>
> and you should see:
>
> HTTP/1.1 200 OK
> Content-Type: text/html
> Last-Modified: Sat, 18 Aug 2018 17:48:08 GMT
> Server: OpenBSD httpd
> Backend-Name: 5GnZ0LBU5CzDw9NCjFbkjI--F_ftp_openbsd_org__primary_
> Via: 1.1 varnish
> Content-Length: 5206
> Accept-Ranges: bytes
> Date: Mon, 15 Oct 2018 06:30:24 GMT
> Via: 1.1 varnish
> Age: 805
> Connection: keep-alive
> X-Served-By: cache-sea1028-SEA, cache-hhn1527-HHN
> X-Cache: HIT, HIT
> X-Cache-Hits: 1, 2
> X-Timer: S1539585024.143724,VS0,VE4
>
> The X-Served-By: line is needed.
>
> Thanks,
> Benno



-- 

Thank you!
Sincere regards;

Neeraj Pal



Re: httpd(8): don't send HSTS headers over unencrypted connections

2018-10-15 Thread Florian Obser
OK florian@

On Mon, Oct 15, 2018 at 12:38:56AM -0600, Anthony J. Bentley wrote:
> Florian Obser writes:
> > On Sun, Oct 14, 2018 at 07:36:18PM -0600, Anthony J. Bentley wrote:
> > > Hi,
> > > 
> > > RFC 6797 says:
> > > 
> > >An HSTS Host MUST NOT include the STS header field in HTTP responses
> > >conveyed over non-secure transport.
> > > 
> > > Is this the correct check? With this I get what I expect: HSTS headers
> >
> > please use srv_conf->flags & SRVFLAG_TLS
> 
> With SRVFLAG_TLS:
> 
> Index: server_fcgi.c
> ===
> RCS file: /cvs/src/usr.sbin/httpd/server_fcgi.c,v
> retrieving revision 1.76
> diff -u -p -r1.76 server_fcgi.c
> --- server_fcgi.c 19 May 2018 13:56:56 -  1.76
> +++ server_fcgi.c 15 Oct 2018 06:32:08 -
> @@ -655,7 +655,8 @@ server_fcgi_header(struct client *clt, u
>   return (-1);
>  
>   /* HSTS header */
> - if (srv_conf->flags & SRVFLAG_SERVER_HSTS) {
> + if (srv_conf->flags & SRVFLAG_SERVER_HSTS &&
> + srv_conf->flags & SRVFLAG_TLS) {
>   if ((cl =
>   kv_add(>http_headers, "Strict-Transport-Security",
>   NULL)) == NULL ||
> Index: server_http.c
> ===
> RCS file: /cvs/src/usr.sbin/httpd/server_http.c,v
> retrieving revision 1.125
> diff -u -p -r1.125 server_http.c
> --- server_http.c 11 Oct 2018 09:52:22 -  1.125
> +++ server_http.c 15 Oct 2018 06:32:08 -
> @@ -950,7 +950,8 @@ server_abort_http(struct client *clt, un
>   goto done;
>   }
>  
> - if (srv_conf->flags & SRVFLAG_SERVER_HSTS) {
> + if (srv_conf->flags & SRVFLAG_SERVER_HSTS &&
> + srv_conf->flags & SRVFLAG_TLS) {
>   if (asprintf(, "Strict-Transport-Security: "
>   "max-age=%d%s%s\r\n", srv_conf->hsts_max_age,
>   srv_conf->hsts_flags & HSTSFLAG_SUBDOMAINS ?
> @@ -1452,7 +1453,8 @@ server_response_http(struct client *clt,
>   return (-1);
>  
>   /* HSTS header */
> - if (srv_conf->flags & SRVFLAG_SERVER_HSTS) {
> + if (srv_conf->flags & SRVFLAG_SERVER_HSTS &&
> + srv_conf->flags & SRVFLAG_TLS) {
>   if ((cl =
>   kv_add(>http_headers, "Strict-Transport-Security",
>   NULL)) == NULL ||
> 

-- 
I'm not entirely sure you are real.



Re: httpd(8): don't send HSTS headers over unencrypted connections

2018-10-15 Thread Anthony J. Bentley
Florian Obser writes:
> On Sun, Oct 14, 2018 at 07:36:18PM -0600, Anthony J. Bentley wrote:
> > Hi,
> > 
> > RFC 6797 says:
> > 
> >An HSTS Host MUST NOT include the STS header field in HTTP responses
> >conveyed over non-secure transport.
> > 
> > Is this the correct check? With this I get what I expect: HSTS headers
>
> please use srv_conf->flags & SRVFLAG_TLS

With SRVFLAG_TLS:

Index: server_fcgi.c
===
RCS file: /cvs/src/usr.sbin/httpd/server_fcgi.c,v
retrieving revision 1.76
diff -u -p -r1.76 server_fcgi.c
--- server_fcgi.c   19 May 2018 13:56:56 -  1.76
+++ server_fcgi.c   15 Oct 2018 06:32:08 -
@@ -655,7 +655,8 @@ server_fcgi_header(struct client *clt, u
return (-1);
 
/* HSTS header */
-   if (srv_conf->flags & SRVFLAG_SERVER_HSTS) {
+   if (srv_conf->flags & SRVFLAG_SERVER_HSTS &&
+   srv_conf->flags & SRVFLAG_TLS) {
if ((cl =
kv_add(>http_headers, "Strict-Transport-Security",
NULL)) == NULL ||
Index: server_http.c
===
RCS file: /cvs/src/usr.sbin/httpd/server_http.c,v
retrieving revision 1.125
diff -u -p -r1.125 server_http.c
--- server_http.c   11 Oct 2018 09:52:22 -  1.125
+++ server_http.c   15 Oct 2018 06:32:08 -
@@ -950,7 +950,8 @@ server_abort_http(struct client *clt, un
goto done;
}
 
-   if (srv_conf->flags & SRVFLAG_SERVER_HSTS) {
+   if (srv_conf->flags & SRVFLAG_SERVER_HSTS &&
+   srv_conf->flags & SRVFLAG_TLS) {
if (asprintf(, "Strict-Transport-Security: "
"max-age=%d%s%s\r\n", srv_conf->hsts_max_age,
srv_conf->hsts_flags & HSTSFLAG_SUBDOMAINS ?
@@ -1452,7 +1453,8 @@ server_response_http(struct client *clt,
return (-1);
 
/* HSTS header */
-   if (srv_conf->flags & SRVFLAG_SERVER_HSTS) {
+   if (srv_conf->flags & SRVFLAG_SERVER_HSTS &&
+   srv_conf->flags & SRVFLAG_TLS) {
if ((cl =
kv_add(>http_headers, "Strict-Transport-Security",
NULL)) == NULL ||



Re: pkg_add -Dsnap not fetching any updates and packages on -current

2018-10-15 Thread Sebastian Benoit
Neeraj Pal(neerajpa...@gmail.com) on 2018.10.15 10:36:16 +0530:
> Hi there,
> 
> Yesterday I installed OpenBSD 6.3-stable then upgraded it to OpenBSD
> -current by downloading and copying bsd.rd file into /
> Then, after that, I am trying to update the userland but it is
> throwing an error but it was working fine previously like one or two
> days before because I have done the same with my other machines.
> It seems there is some problem with server-side?
> If I have forgotten something or doing something wrong then please let me 
> know.
> 
> openbsd$ cat /etc/installurl
> https://fastly.cdn.openbsd.org/pub/OpenBSD
> openbsd$
> 
> openbsd$ doas pkg_add -Dsnap -u
> doas (b...@openbsd.my.domain) password:
> https://fastly.cdn.openbsd.org/pub/OpenBSD/snapshots/packages/amd64/:
> ftp: SSL write error: ocsp verify failed: ocsp response not current
> https://fastly.cdn.openbsd.org/pub/OpenBSD/snapshots/packages/amd64/: empty
> Couldn't find updates for axel-2.4p0 gettext-0.19.8.1p1 libiconv-1.14p3
> 
> openbsd$ doas pkg_add -Dsnap -iv vim
> doas (b...@openbsd.my.domain) password:
> https://fastly.cdn.openbsd.org/pub/OpenBSD/snapshots/packages/amd64/:
> ftp: SSL write error: ocsp verify failed: ocsp response not current
> https://fastly.cdn.openbsd.org/pub/OpenBSD/snapshots/packages/amd64/: empty
> Can't find vim
> openbsd$
> 
> openbsd$ doas pkg_add -Dsnap -iv wget
> doas (b...@openbsd.my.domain) password:
> https://fastly.cdn.openbsd.org/pub/OpenBSD/snapshots/packages/amd64/:
> ftp: SSL write error: ocsp verify failed: ocsp response not current
> https://fastly.cdn.openbsd.org/pub/OpenBSD/snapshots/packages/amd64/: empty
> Can't find wget

Hi,

this could be a server side problem, or a problem with the time on your
system.

If you still can reproduce the problem, please check that your systems time
is correct.

If it is, show the output of

 nc -cvvv fastly.cdn.openbsd.org 443

It should look something like this:

Connection to fastly.cdn.openbsd.org 443 port [tcp/https] succeeded!
TLS handshake negotiated TLSv1.2/ECDHE-RSA-AES128-GCM-SHA256 with host
fastly.cdn.openbsd.org
Peer name: fastly.cdn.openbsd.org
Subject: /C=US/ST=California/L=San Francisco/O=Fastly,
Inc./CN=osff.map.fastly.net
Issuer: /C=BE/O=GlobalSign nv-sa/CN=GlobalSign CloudSSL CA - SHA256 - G3
Valid From: Fri Oct 12 01:08:13 2018
Valid Until: Sun Feb 24 20:25:45 2019
Cert Hash:
SHA256:28dd8ee0a34e3760554aea2944d8cc8c13601282fd51382215da748b8b06a826
OCSP URL: http://ocsp2.globalsign.com/cloudsslsha2g3
OCSP Stapling: good
  response_status=0 cert_status=0 crl_reason=0
  this update: Mon Oct 15 02:20:32 2018
  next update: Fri Oct 19 02:20:32 2018
  revocation: 


At that point type

 GET / HTTP/1.1
 Host: fastly.cdn.openbsd.org
 

and you should see:

HTTP/1.1 200 OK
Content-Type: text/html
Last-Modified: Sat, 18 Aug 2018 17:48:08 GMT
Server: OpenBSD httpd
Backend-Name: 5GnZ0LBU5CzDw9NCjFbkjI--F_ftp_openbsd_org__primary_
Via: 1.1 varnish
Content-Length: 5206
Accept-Ranges: bytes
Date: Mon, 15 Oct 2018 06:30:24 GMT
Via: 1.1 varnish
Age: 805
Connection: keep-alive
X-Served-By: cache-sea1028-SEA, cache-hhn1527-HHN
X-Cache: HIT, HIT
X-Cache-Hits: 1, 2
X-Timer: S1539585024.143724,VS0,VE4

The X-Served-By: line is needed.

Thanks,
Benno



Re: httpd(8): don't send HSTS headers over unencrypted connections

2018-10-15 Thread Florian Obser
On Mon, Oct 15, 2018 at 07:33:52AM +0200, Bruno Flueckiger wrote:
> On 14.10.18 19:36, Anthony J. Bentley wrote:
> > Hi,
> > 
> > RFC 6797 says:
> > 
> >An HSTS Host MUST NOT include the STS header field in HTTP responses
> >conveyed over non-secure transport.
> > 
> > Is this the correct check? With this I get what I expect: HSTS headers
> > over TLS, and no HSTS headers over unencrypted HTTP.
> > 
> 
> If you don't want to send HSTS headers then don't set the option hsts in
> httpd.conf(5). Why would you provide an option for the admin to choose
> but restrict it to only encrypted connections? 
> 
> Your change would break the scenario of running httpd behind relayd(8)
> for TLS acceleaation, e. g. relayd on the public interface and httpd on
> localhost.

relayd should add the hsts header in that case with the "header set"
feature.

I wonder how I manage to not see the "MUST NOT" when I implemented
HSTS because I remember that I specifically looked for guidance on
when to set the header.

"MUST NOT" is the strongest language the IETF has - we should follow
it.

> 
> Cheers,
> Bruno
> 
> > Index: server_fcgi.c
> > ===
> > RCS file: /cvs/src/usr.sbin/httpd/server_fcgi.c,v
> > retrieving revision 1.76
> > diff -u -p -r1.76 server_fcgi.c
> > --- server_fcgi.c   19 May 2018 13:56:56 -  1.76
> > +++ server_fcgi.c   15 Oct 2018 01:30:28 -
> > @@ -655,7 +655,7 @@ server_fcgi_header(struct client *clt, u
> > return (-1);
> >  
> > /* HSTS header */
> > -   if (srv_conf->flags & SRVFLAG_SERVER_HSTS) {
> > +   if (srv_conf->flags & SRVFLAG_SERVER_HSTS && clt->clt_tls_ctx != NULL) {
> > if ((cl =
> > kv_add(>http_headers, "Strict-Transport-Security",
> > NULL)) == NULL ||
> > Index: server_http.c
> > ===
> > RCS file: /cvs/src/usr.sbin/httpd/server_http.c,v
> > retrieving revision 1.125
> > diff -u -p -r1.125 server_http.c
> > --- server_http.c   11 Oct 2018 09:52:22 -  1.125
> > +++ server_http.c   15 Oct 2018 01:30:28 -
> > @@ -950,7 +950,7 @@ server_abort_http(struct client *clt, un
> > goto done;
> > }
> >  
> > -   if (srv_conf->flags & SRVFLAG_SERVER_HSTS) {
> > +   if (srv_conf->flags & SRVFLAG_SERVER_HSTS && clt->clt_tls_ctx != NULL) {
> > if (asprintf(, "Strict-Transport-Security: "
> > "max-age=%d%s%s\r\n", srv_conf->hsts_max_age,
> > srv_conf->hsts_flags & HSTSFLAG_SUBDOMAINS ?
> > @@ -1452,7 +1452,7 @@ server_response_http(struct client *clt,
> > return (-1);
> >  
> > /* HSTS header */
> > -   if (srv_conf->flags & SRVFLAG_SERVER_HSTS) {
> > +   if (srv_conf->flags & SRVFLAG_SERVER_HSTS && clt->clt_tls_ctx != NULL) {
> > if ((cl =
> > kv_add(>http_headers, "Strict-Transport-Security",
> > NULL)) == NULL ||
> > 
> 

-- 
I'm not entirely sure you are real.



Re: httpd(8): don't send HSTS headers over unencrypted connections

2018-10-15 Thread Florian Obser
On Sun, Oct 14, 2018 at 07:36:18PM -0600, Anthony J. Bentley wrote:
> Hi,
> 
> RFC 6797 says:
> 
>An HSTS Host MUST NOT include the STS header field in HTTP responses
>conveyed over non-secure transport.
> 
> Is this the correct check? With this I get what I expect: HSTS headers

please use srv_conf->flags & SRVFLAG_TLS

> over TLS, and no HSTS headers over unencrypted HTTP.
> 
> Index: server_fcgi.c
> ===
> RCS file: /cvs/src/usr.sbin/httpd/server_fcgi.c,v
> retrieving revision 1.76
> diff -u -p -r1.76 server_fcgi.c
> --- server_fcgi.c 19 May 2018 13:56:56 -  1.76
> +++ server_fcgi.c 15 Oct 2018 01:30:28 -
> @@ -655,7 +655,7 @@ server_fcgi_header(struct client *clt, u
>   return (-1);
>  
>   /* HSTS header */
> - if (srv_conf->flags & SRVFLAG_SERVER_HSTS) {
> + if (srv_conf->flags & SRVFLAG_SERVER_HSTS && clt->clt_tls_ctx != NULL) {
>   if ((cl =
>   kv_add(>http_headers, "Strict-Transport-Security",
>   NULL)) == NULL ||
> Index: server_http.c
> ===
> RCS file: /cvs/src/usr.sbin/httpd/server_http.c,v
> retrieving revision 1.125
> diff -u -p -r1.125 server_http.c
> --- server_http.c 11 Oct 2018 09:52:22 -  1.125
> +++ server_http.c 15 Oct 2018 01:30:28 -
> @@ -950,7 +950,7 @@ server_abort_http(struct client *clt, un
>   goto done;
>   }
>  
> - if (srv_conf->flags & SRVFLAG_SERVER_HSTS) {
> + if (srv_conf->flags & SRVFLAG_SERVER_HSTS && clt->clt_tls_ctx != NULL) {
>   if (asprintf(, "Strict-Transport-Security: "
>   "max-age=%d%s%s\r\n", srv_conf->hsts_max_age,
>   srv_conf->hsts_flags & HSTSFLAG_SUBDOMAINS ?
> @@ -1452,7 +1452,7 @@ server_response_http(struct client *clt,
>   return (-1);
>  
>   /* HSTS header */
> - if (srv_conf->flags & SRVFLAG_SERVER_HSTS) {
> + if (srv_conf->flags & SRVFLAG_SERVER_HSTS && clt->clt_tls_ctx != NULL) {
>   if ((cl =
>   kv_add(>http_headers, "Strict-Transport-Security",
>   NULL)) == NULL ||
> 

-- 
I'm not entirely sure you are real.



Re: httpd(8): don't send HSTS headers over unencrypted connections

2018-10-15 Thread Anthony J. Bentley
Bruno Flueckiger writes:
> If you don't want to send HSTS headers then don't set the option hsts in
> httpd.conf(5). Why would you provide an option for the admin to choose
> but restrict it to only encrypted connections? 

Because it's possible to specify both "listen on * tls port 443" and
"listen on * port 80" in the same server block.

The other TLS-related options only apply to encrypted connections in
such a scenario. Then again, none of them work by injecting headers.

-- 
Anthony J. Bentley