Re: relayd does not delete control socket on shutdown

2023-10-22 Thread Theo de Raadt
Otto Moerbeek  wrote:

> On Sat, Oct 21, 2023 at 10:40:45PM +0300, Kapetanakis Giannis wrote:
> 
> > On 21/10/2023 20:39, Florian Obser wrote:
> > > Which was 8 years ago. I don't understand why you see a change in 7.4.
> > > 
> > > Anyway, we decided to not clean up control sockets in any of our
> > > privsep daemons because leaving them behind does not cause any issues.
> > 
> > I just noticed it today when I tried to use the socket in a script and
> > noticed that it stayed there even after shutdown and though it was after 7.4
> > but I was wrong about that.
> > 
> > Your commit made it that clear.
> > 
> > Agree it's not a big case if it stays there.
> > 
> > Would the unlink succeed if the socket was owned by _relayd?
> > 
> > G
> 
> Unlinking somthing requires write permissions to the directory it is
> in.

Which means an attacker who gains control, but otherwise can't do a bunch
of other things becuase of the privsep design -- could still fill the directory
and filesystem.

So a few years ago we asked ourselves -- is the tradeoff worth it?



Re: relayd does not delete control socket on shutdown

2023-10-22 Thread Otto Moerbeek
On Sat, Oct 21, 2023 at 10:40:45PM +0300, Kapetanakis Giannis wrote:

> On 21/10/2023 20:39, Florian Obser wrote:
> > Which was 8 years ago. I don't understand why you see a change in 7.4.
> > 
> > Anyway, we decided to not clean up control sockets in any of our
> > privsep daemons because leaving them behind does not cause any issues.
> 
> I just noticed it today when I tried to use the socket in a script and
> noticed that it stayed there even after shutdown and though it was after 7.4
> but I was wrong about that.
> 
> Your commit made it that clear.
> 
> Agree it's not a big case if it stays there.
> 
> Would the unlink succeed if the socket was owned by _relayd?
> 
> G

Unlinking somthing requires write permissions to the directory it is
in.

-Otto



Re: relayd does not delete control socket on shutdown

2023-10-21 Thread Kapetanakis Giannis

On 21/10/2023 20:39, Florian Obser wrote:

Which was 8 years ago. I don't understand why you see a change in 7.4.

Anyway, we decided to not clean up control sockets in any of our
privsep daemons because leaving them behind does not cause any issues.


I just noticed it today when I tried to use the socket in a script and 
noticed that it stayed there even after shutdown and though it was after 
7.4 but I was wrong about that.


Your commit made it that clear.

Agree it's not a big case if it stays there.

Would the unlink succeed if the socket was owned by _relayd?

G




Re: relayd does not delete control socket on shutdown

2023-10-21 Thread Florian Obser
On 2023-10-21 14:49 +03, Kapetanakis Giannis  wrote:
> Rev 1.140 by florian@ seems to have changed that.
>
> Do not try to unlink the control socket in an unprivileged child
> process on shutdown.
> Found while working ontame(2)  .
> OK benno@
>

Which was 8 years ago. I don't understand why you see a change in 7.4.

Anyway, we decided to not clean up control sockets in any of our
privsep daemons because leaving them behind does not cause any issues.

> G
>
>
> On 21/10/2023 14:41, Kapetanakis Giannis wrote:
>> After 7.4 relayd does not unlink it's socket
>>
>> I've added the following but it's probably not enough. unveil?
>>
>> G
>>
>> Index: relayd.c
>> ===
>> RCS file: /cvs/src/usr.sbin/relayd/relayd.c,v
>> retrieving revision 1.191
>> diff -u -p -r1.191 relayd.c
>> --- relayd.c    25 Jun 2023 08:07:38 -    1.191
>> +++ relayd.c    21 Oct 2023 11:39:44 -
>> @@ -382,6 +382,8 @@ parent_shutdown(struct relayd *env)
>>  free(env->sc_ps);
>>  free(env);
>>
>> +    unlink(env->sc_ps->ps_csock.cs_name);
>> +
>>  log_info("parent terminating, pid %d", getpid());
>>
>>  exit(0);
>>
>

-- 
In my defence, I have been left unsupervised.



Re: relayd does not delete control socket on shutdown

2023-10-21 Thread Kapetanakis Giannis

Rev 1.140 by florian@ seems to have changed that.

Do not try to unlink the control socket in an unprivileged child
process on shutdown.
Found while working ontame(2)  .
OK benno@

G


On 21/10/2023 14:41, Kapetanakis Giannis wrote:

After 7.4 relayd does not unlink it's socket

I've added the following but it's probably not enough. unveil?

G

Index: relayd.c
===
RCS file: /cvs/src/usr.sbin/relayd/relayd.c,v
retrieving revision 1.191
diff -u -p -r1.191 relayd.c
--- relayd.c    25 Jun 2023 08:07:38 -    1.191
+++ relayd.c    21 Oct 2023 11:39:44 -
@@ -382,6 +382,8 @@ parent_shutdown(struct relayd *env)
 free(env->sc_ps);
 free(env);

+    unlink(env->sc_ps->ps_csock.cs_name);
+
 log_info("parent terminating, pid %d", getpid());

 exit(0);



relayd does not delete control socket on shutdown

2023-10-21 Thread Kapetanakis Giannis

After 7.4 relayd does not unlink it's socket

I've added the following but it's probably not enough. unveil?

G

Index: relayd.c
===
RCS file: /cvs/src/usr.sbin/relayd/relayd.c,v
retrieving revision 1.191
diff -u -p -r1.191 relayd.c
--- relayd.c    25 Jun 2023 08:07:38 -    1.191
+++ relayd.c    21 Oct 2023 11:39:44 -
@@ -382,6 +382,8 @@ parent_shutdown(struct relayd *env)
 free(env->sc_ps);
 free(env);

+    unlink(env->sc_ps->ps_csock.cs_name);
+
 log_info("parent terminating, pid %d", getpid());

 exit(0);