Re: Some NFS clients won't mount

2023-01-01 Thread Theo de Raadt
vitmau...@gmail.com  wrote:

> I did some tests and I'm now pretty sure the problem revolves around
> the point naddy made: Kodi and VLC try to mount my NFS share through a
> non-privileged port. As both Kodi and VLC use the same NFS client
> library (libnfs), I tried to find out a bit more about how it works.
> According to its readme, libnfs uses standard NFS ports when run as
> root and non-privileged ports when run non-root. Here is the relevant
> part of the readme file: "When running as root, libnfs tries to
> allocate a system port for its connection to the NFS server. When
> running as non-root it will use a normal ephemeral port". I find it
> strange that a client library should be run as root in order to use a
> privileged port. My (very poor, I confess) understanding was that only
> server processes should be run as root in order to use privileged
> ports. Anyway, as things stand I can only mount my OpenBSD NFS shares
> if the client is run as root, since the usual way to circumvent this
> problem on the server side (set the insecure flag on exports) is not
> available on OpenBSD and, I hope, won't ever be. As I don't have root
> access to my Fire Stick TV, there is no way to mount my OpenBSD NFS
> shares on it. As I'm no expert on security though, I'd like an opinion
> from you guys regarding this: is it reasonable to require an NFS
> client to be run as root?

Yes, it is very reasonable, because after five decades noone has written
a bug-free NFS server, so this mechanism limiting who can talk to it
makes sense.

More general comment on this matter: The use of reserved ports for
numerous protocols will continue to make sense FOREVER, no matter what
the naysayers say, because there are protocols with insufficient
alternative authentication methods or services you don't want generic
users to startup on a shared system and behave as the cannonical
offering of that service from such a shared system machine.  If anyone
things we are wrong, go to IETF and convince then to move http and https
to non-reserved well-known ports *by default*.  Anyone who tries this
will get nowhere there, and will get nowhere here either... practical
matters force this.

Sorry.



TLS certs for open{bgpd,ntpd}.org expired

2023-01-01 Thread Matthias Schmidt
Hi,

in case the admin is subscribed here or someone can deliver a ping.  The
certs for the above mentioned web sites expired on 20221225.

Cheers

Matthias



Re: Possible off-by-one bug in usr.sbin/rad/engine.c

2023-01-01 Thread Alejandro Colomar

Hello Florian, Ingo,

On 1/1/23 08:24, Florian Obser wrote:

On 2022-12-31 23:54 +01, Ingo Schwarze  wrote:

Hi Alejandro,

Alejandro Colomar wrote on Sat, Dec 31, 2022 at 05:56:27PM +0100:


I've started auditing the OpenBSD source code after the discussion on
arc4random_uniform(3) and my suggestion of arc4random_range() on the glibc
mailing list.

I found some cases where it seems like there's an off-by-one bug, which
would be solved by providing arc4random_range().  I'll show here one,
to confirm that it's a bug, and if you confirm it, I'll continue fixing
similar bugs around the OpenBSD tree.

Here's the first one I found, which I hope is fixed by my patch:


diff --git a/usr.sbin/rad/engine.c b/usr.sbin/rad/engine.c
index ceb11d574e3..a61ea3835a6 100644
--- a/usr.sbin/rad/engine.c
+++ b/usr.sbin/rad/engine.c
@@ -641,8 +641,7 @@ iface_timeout(int fd, short events, void *arg)
  struct imsg_send_ra  send_ra;
  struct timeval   tv;

-   tv.tv_sec = MIN_RTR_ADV_INTERVAL +
-   arc4random_uniform(MAX_RTR_ADV_INTERVAL - MIN_RTR_ADV_INTERVAL);
+   tv.tv_sec = arc4random_range(MIN_RTR_ADV_INTERVAL, 
MAX_RTR_ADV_INTERVAL);
  tv.tv_usec = arc4random_uniform(100);


Currently, the code puts a number in the range [200, 600) in tv_sec
and a random number of microseconds into tv_usec,
i.e. the timeout will be greater than or equal to 200 seconds
and strictly less than 600 seconds with a uniform distribution.

Isn't that exactly what is intended?


  log_debug("%s new timeout in %lld", __func__, tv.tv_sec);


If I'm correct, it should have been 'min + (max - min + 1)' instead
of 'min + (max - min)'.  Please confirm.


With your change, the timeout could go up to 600.99, i.e. almost 601
seconds.  I don't know the protocol and can't say whether the change
would matter, but naively, exceeding the MAX_ feels surprising to me.

Really, this doesn't look like a bug to me...


Unfortunately the OP did not explain why they think this is a bug.


Sorry; my bad; I should have explained it.

The thing that led me to believe that it was a bug is that variables or 
constants called *max* (normally) refer to the maximum value allowed in a range, 
for which there usually is a *min* counterpart (when it's not simply 0).


In this case, it seems MAX_* is really the maximum+1.  I don't know what the 
code is about, so 200 and 600 just look like magic numbers to me, and I don't 
know if the maximum is 600 or actually 599.






Yours,
   Ingo




Cheers,
Alex

--



OpenPGP_signature
Description: OpenPGP digital signature


Re: Possible off-by-one bug in usr.sbin/rad/engine.c

2023-01-01 Thread Alejandro Colomar



On 1/1/23 14:48, Alejandro Colomar wrote:

Hello Florian, Ingo,

On 1/1/23 08:24, Florian Obser wrote:

On 2022-12-31 23:54 +01, Ingo Schwarze  wrote:


[...]



With your change, the timeout could go up to 600.99, i.e. almost 601
seconds.  I don't know the protocol and can't say whether the change
would matter, but naively, exceeding the MAX_ feels surprising to me.


Oops, I missed this part.  That's where it makes sense. :)



Really, this doesn't look like a bug to me...


Unfortunately the OP did not explain why they think this is a bug.


Sorry; my bad; I should have explained it.

The thing that led me to believe that it was a bug is that variables or 
constants called *max* (normally) refer to the maximum value allowed in a range, 
for which there usually is a *min* counterpart (when it's not simply 0).


In this case, it seems MAX_* is really the maximum+1.  I don't know what the 
code is about, so 200 and 600 just look like magic numbers to me, and I don't 
know if the maximum is 600 or actually 599.






Yours,
   Ingo




Cheers,
Alex



--



OpenPGP_signature
Description: OpenPGP digital signature


Re: Some NFS clients won't mount

2023-01-01 Thread vitmau...@gmail.com
Theo helped solve my main concern. In my mind, I was kinda comparing
NFS and HTTP: no one requires a browser (which is essentially a
HTTPclient) to be run as root, so why require a NFS client to be run
this way? But the point is that we have securely written HTTP servers,
so we don't need to be so strict when it comes to who talks to these
servers. Unfortunately, the same cannot be said of NFS servers.

Thank you guys for the help and for making things clearer for me.

Best,
Vitor

Em dom., 1 de jan. de 2023 às 06:39, Theo de Raadt
 escreveu:
>
> vitmau...@gmail.com  wrote:
>
> > I did some tests and I'm now pretty sure the problem revolves around
> > the point naddy made: Kodi and VLC try to mount my NFS share through a
> > non-privileged port. As both Kodi and VLC use the same NFS client
> > library (libnfs), I tried to find out a bit more about how it works.
> > According to its readme, libnfs uses standard NFS ports when run as
> > root and non-privileged ports when run non-root. Here is the relevant
> > part of the readme file: "When running as root, libnfs tries to
> > allocate a system port for its connection to the NFS server. When
> > running as non-root it will use a normal ephemeral port". I find it
> > strange that a client library should be run as root in order to use a
> > privileged port. My (very poor, I confess) understanding was that only
> > server processes should be run as root in order to use privileged
> > ports. Anyway, as things stand I can only mount my OpenBSD NFS shares
> > if the client is run as root, since the usual way to circumvent this
> > problem on the server side (set the insecure flag on exports) is not
> > available on OpenBSD and, I hope, won't ever be. As I don't have root
> > access to my Fire Stick TV, there is no way to mount my OpenBSD NFS
> > shares on it. As I'm no expert on security though, I'd like an opinion
> > from you guys regarding this: is it reasonable to require an NFS
> > client to be run as root?
>
> Yes, it is very reasonable, because after five decades noone has written
> a bug-free NFS server, so this mechanism limiting who can talk to it
> makes sense.
>
> More general comment on this matter: The use of reserved ports for
> numerous protocols will continue to make sense FOREVER, no matter what
> the naysayers say, because there are protocols with insufficient
> alternative authentication methods or services you don't want generic
> users to startup on a shared system and behave as the cannonical
> offering of that service from such a shared system machine.  If anyone
> things we are wrong, go to IETF and convince then to move http and https
> to non-reserved well-known ports *by default*.  Anyone who tries this
> will get nowhere there, and will get nowhere here either... practical
> matters force this.
>
> Sorry.



Re: Possible off-by-one bug in usr.sbin/rad/engine.c

2023-01-01 Thread Alejandro Colomar

Hello Rudolf,

On 1/1/23 16:59, Rudolf Leitgeb wrote:

Coming from a C/C++ background, I would assume, that a range from
200 to 600 comprises numbers would start at 200 and reach as far
as 599. This would be in sync with all STL functions for iterating
through collections or for extracting ranges.

As long as you need two random numbers to craft seconds and
microseconds values, it will be anything but easy to create
a uniform distribution going from 200.000 all the way up
to and including 600.00. As others have already pointed
out, your initially proposed fix certainly does not achieve this.


Yes, I was wrong.  However, please check my patch 2/2 recently sent to the list. 
 There are a few other cases, and I think some of them may be actual bugs.


Thanks,

Alex

--



OpenPGP_signature
Description: OpenPGP digital signature


Re: Possible off-by-one bug in usr.sbin/rad/engine.c

2023-01-01 Thread Rudolf Leitgeb
Coming from a C/C++ background, I would assume, that a range from
200 to 600 comprises numbers would start at 200 and reach as far
as 599. This would be in sync with all STL functions for iterating
through collections or for extracting ranges.

As long as you need two random numbers to craft seconds and
microseconds values, it will be anything but easy to create
a uniform distribution going from 200.000 all the way up
to and including 600.00. As others have already pointed
out, your initially proposed fix certainly does not achieve this.

> Gesendet: Sonntag, 01. Januar 2023 um 15:33 Uhr
> Von: "Alejandro Colomar" 
> An: "Florian Obser" , "Ingo Schwarze" 
> Cc: misc@openbsd.org
> Betreff: Re: Possible off-by-one bug in usr.sbin/rad/engine.c
>
> 
> 
> On 1/1/23 14:48, Alejandro Colomar wrote:
> > Hello Florian, Ingo,
> > 
> > On 1/1/23 08:24, Florian Obser wrote:
> >> On 2022-12-31 23:54 +01, Ingo Schwarze  wrote:
> 
> [...]
> 
> >>>
> >>> With your change, the timeout could go up to 600.99, i.e. almost 601
> >>> seconds.  I don't know the protocol and can't say whether the change
> >>> would matter, but naively, exceeding the MAX_ feels surprising to me.
> 
> Oops, I missed this part.  That's where it makes sense. :)
> 
> >>>
> >>> Really, this doesn't look like a bug to me...
> >>
> >> Unfortunately the OP did not explain why they think this is a bug.
> > 
> > Sorry; my bad; I should have explained it.
> > 
> > The thing that led me to believe that it was a bug is that variables or 
> > constants called *max* (normally) refer to the maximum value allowed in a 
> > range, 
> > for which there usually is a *min* counterpart (when it's not simply 0).
> > 
> > In this case, it seems MAX_* is really the maximum+1.  I don't know what 
> > the 
> > code is about, so 200 and 600 just look like magic numbers to me, and I 
> > don't 
> > know if the maximum is 600 or actually 599.
> > 
> >>
> >>>
> >>> Yours,
> >>>    Ingo
> >>
> > 
> > Cheers,
> > Alex
> > 
> 
> -- 
> 
>



Re: [RFC v1 2/2] Use arc4random_range() instead of arc4random_uniform() when appropriate

2023-01-01 Thread Theo de Raadt
Your proposal is junk.  Not going to happen.

>From owner-misc+M195331=deraadt=cvs.openbsd@openbsd.org Sat Dec 31 
>11:19:48 2022
>Delivered-To: dera...@cvs.openbsd.org
>DKIM-Signature: v=1; a=rsa-sha256; c=simple/simple; s=selector1; bh=/JVUSEqVR3
>   /k8gFGm9V8QDDc/a7fMpZ1djd/RE+G3ho=; h=list-unsubscribe:list-subscribe:
>   list-post:list-owner:list-id:list-help:references:in-reply-to:date:
>   subject:cc:to:from; d=openbsd.org; b=9QbHsEP2Cs5GzwJFaRcKYtGgmTEdJNstk
>   WG0moKjzSa7hfEBKsJsJdfiisq/xmQ6cDJ22kKl4rA0btSsjHthLFkJhvak9A132n8Pyv0
>   wHmF/q53tJzAvwHHCPGNFmCMXdKVzN/k2IQUyrA/USls/x58VaRfuLYO2ooxbL3pPr9sSj
>   Vi8PjayN3X1XksgfEJZj38LMOv8l2knvBBYrubQmLM21+bY1ilB6kq7yPUs/BneF7lG0Gt
>   lgAGRJitijIAy8hZzgbgZEfs49Bx1Q5hT0Bw7T7OH6EkWtQ5ez59TiUzRWGXRkQSRAaTPy
>   JyU3jjcTVd/NV3BmkdVTjMBONmpuQ==
>X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed;
>d=1e100.net; s=20210112;
>h=content-transfer-encoding:mime-version:references:in-reply-to
> :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc
> :subject:date:message-id:reply-to;
>bh=2UYUeZysQqtf/KOzlblpSVrwo2fzTbMjuWtz4tikOe0=;
>b=URJZ9YXpK7xe7laZ33ubdezrA7RmZ3aI+F8YaDpyZ8BuETxr7k+J6TzjjXbjvzj+vm
> kxI3RFU0zRcggCh/Xt6lIRYxViBYS2SuA7JB1VsDDAOrOwpRSq5s3ryoTggMtlFj4zD/
> HpWkBBML0zkmoLQHYmdk8jSx2gKUGpDdJaIA8uUD6mIkmZlxKEFMRagUyisXR8O5RH6W
> P23r5w9dlLu7sPfHCHDkwR76xUTp97+yVgHsKzcAc9OlLsKr0+aacNq8uuXQhztxNY3W
> JgQc5MhOOahuEI97xN8xI3L0e6BKFZmNS5vvUCLXEiFdt7jbx2JbZ9czZSeZwa87p6Wn
> Ru1g==
>X-Gm-Message-State: AFqh2krJps2B1CZ8kRCvS5Dm15CqO5h8/P3AzqvbHRPKj9lPOh8DWcFW
>   ypZJNU0vfcYp+NudScdH0MILST5jvj8=
>X-Google-Smtp-Source: 
>AMrXdXuh8Ked2AQA4K81PjC0dktKBv52w3jykbg/aF2xwJVEjOAuMNvGippMa6MPtJ+V948C79+izw==
>X-Received: by 2002:a05:600c:500a:b0:3d3:5b56:b834 with SMTP id 
>n10-20020a05600c500a00b003d35b56b834mr25309242wmr.5.1672510739398;
>Sat, 31 Dec 2022 10:18:59 -0800 (PST)
>From: Alejandro Colomar 
>X-Google-Original-From: Alejandro Colomar 
>To: misc@openbsd.org
>Cc: Alejandro Colomar ,
>   schwa...@openbsd.org
>Subject: [RFC v1 2/2] Use arc4random_range() instead of arc4random_uniform() 
>when appropriate
>Date: Sat, 31 Dec 2022 19:18:56 +0100
>X-Mailer: git-send-email 2.39.0
>In-Reply-To: <20221231181856.13173-1-...@kernel.org>
>References: <20221231181856.13173-1-...@kernel.org>
>MIME-Version: 1.0
>Content-Transfer-Encoding: 8bit
>List-Help: 
>List-ID: 
>List-Owner: 
>List-Post: 
>List-Subscribe: 
>List-Unsubscribe: 
>X-Loop: misc@openbsd.org
>Precedence: list
>Sender: owner-m...@openbsd.org
>
>This makes the code much more readable and self-documented.  While doing
>this, I noticed a few bugs, and other cases which may be bugs or not.
>Switching to this specialized API makes it easier to spot such bugs, but
>since I'm not familiar with the code, I kept some bugs unfixed.  The
>most obvious ones (although I may be wrong) I fixed them.  And in some
>cases where it was very unclear, I didn't touch the old *_uniform() code.
>
>Below are the cases where I changed the behavior (I considered it a bug):
>
>*  usr.bin/ssh/auth.c:
>
>   -  *cp = hashchars[arc4random_uniform(sizeof(hashchars) - 1)];
>   +  *cp = hashchars[arc4random_range(0, sizeof(hashchars) - 1)];
>
>*  usr.sbin/ftp-proxy/ftp-proxy.c:
>
>   -  return (IPPORT_HIFIRSTAUTO +
>   -  arc4random_uniform(IPPORT_HILASTAUTO - IPPORT_HIFIRSTAUTO));
>   +  return arc4random_range(IPPORT_HIFIRSTAUTO, IPPORT_HILASTAUTO);
>
>*  usr.sbin/rad/engine.c:
>
>   -  tv.tv_sec = MIN_RTR_ADV_INTERVAL +
>   -  arc4random_uniform(MAX_RTR_ADV_INTERVAL - MIN_RTR_ADV_INTERVAL);
>   +  tv.tv_sec = arc4random_range(MIN_RTR_ADV_INTERVAL, MAX_RTR_ADV_INTERVAL);
>
>In the following change, I didn't use the temporary variable 'num3'.
>AFAICS, this doesn't affect other uses of the variable in other places,
>because they set it before use.  But please check carefully; I may have
>missed something:
>
>*  usr.sbin/cron/entry.c:
>
>   -  /* get a random number in the interval [num1, num2]
>   -   */
>   -  num3 = num1;
>   -  num1 = arc4random_uniform(num2 - num3 + 1) + num3;
>   +  num1 = arc4random_range(num1, num2);
>
>Signed-off-by: Alejandro Colomar 
>---
> games/boggle/boggle/bog.c   | 2 +-
> games/canfield/canfield/canfield.c  | 2 +-
> games/mille/init.c  | 2 +-
> gnu/gcc/gcc/cfgexpand.c | 2 +-
> lib/libevent/select.c   | 2 +-
> regress/lib/libc/malloc/malloc_general/malloc_general.c | 2 +-
> regress/sys/sys/tree/rb/rb-test.c   | 3 +--
> regress/sys/sys/tree/splay/splay-test.c | 3 +--
> sbin/ik