Re: Vulnerability in pcs or is it in more generic code?

2022-09-09 Thread Paul Wise
On Fri, 2022-09-09 at 22:41 +0200, Ola Lundqvist wrote:

> I see that I was not clear what I meant with "in general" :-)

Woops, sorry for the noise :)

> Here I found how the generic source code looks like:
> https://rubydoc.info/gems/thin/1.3.1/Thin%2FBackends%2FUnixServer:connect
> 
> You can see the umask(0) there.
> 
> That is what I think is insecure, not pcs itself.

Agreed.

> But I think Thin::Backends::UnixServer#connect is still insecure.

It looks like the issue was introduced in this pull request:

   https://github.com/macournoyer/thin/pull/28

It sounds like unicorn might have the same issue as thin.

Looking at the unicorn code, it sets the default umask to 0
when the umask option is not set, so not quite as bad but still.

The justification for the default umask of 0 in unicorn is:

   #   Typically UNIX domain sockets are created with more liberal
   #   file permissions than the rest of the application.  By default,
   #   we create UNIX domain sockets to be readable and writable by
   #   all local users to give them the same accessibility as
   #   locally-bound TCP listeners.

So the choice of umask 0 is deliberate in unicorn and the thin folks
copied that choice and dropped the possibility of overriding it,
since they did't have an options argument for the function.

Since UNIX domain sockets are often used for situations that are not
like localhost TCP sockets, that was probably the wrong choice, but
the unicorn/thin folks are likely from the web developer community,
which is mostly focused on HTTP and TCP and often use localhost for
development, so it was the right choice within their bubble.

I feel like the APIs of both thin and unicorn need redesigning so that
the choice of umask to use must be made by the caller. Then the web
developer community can use 0 and others will choose a safe value.

Really the web developer community shouldn't use 0 either but I expect
that convincing them of that might not be possible.

I'm not sure how palatable the API change will be to thin/unicorn.

-- 
bye,
pabs

https://wiki.debian.org/PaulWise


signature.asc
Description: This is a digitally signed message part


Re: Vulnerability in pcs or is it in more generic code?

2022-09-09 Thread Ola Lundqvist
Hi Paul

I see that I was not clear what I meant with "in general" :-)

In the fix for pcs
https://github.com/ClusterLabs/pcs/commit/de068e2066e377d1cc77edf25aed0198e4c77f7b
you can see a comment that there is a change from umask(0) to umask(0x077)

It was this umask(0) (in Thin::Backends::UnixServer#connect) I was
referring to as "in general".

I mean the fix is to override this more generic function that is obviously
not secure enough.

Here I found how the generic source code looks like:
https://rubydoc.info/gems/thin/1.3.1/Thin%2FBackends%2FUnixServer:connect

You can see the umask(0) there.

That is what I think is insecure, not pcs itself.

It looks like pcs code was not vulnerable because what I missed to check
was whether this source code was present in buster. It was not as someone
have concluded.

But I think Thin::Backends::UnixServer#connect is still insecure.

Cheers

// Ola

On Tue, 6 Sept 2022 at 03:09, Paul Wise  wrote:

> On Mon, 2022-09-05 at 21:38 +0200, Ola Lundqvist wrote:
>
> > I agree that it is good to fix the pcs package, but shouldn't we fix
> > the default umask in general?
> > I would argue that the default umask is insecure.
>
> bookworm login sets new user home directories to secure permissions:
>
>$ grep -E 'HOME_MODE\s*[0-9]' /etc/login.defs
>#HOME_MODE   0700
>
> This somewhat mitigates, but not completely, the umask being insecure:
>
>$ grep -E 'UMASK\s*[0-9]' /etc/login.defs
>UMASK022
>
> I can't find any bugs open about changing the default umask,
> but it was mentioned in replies to the recent adduser thread:
>
> https://lists.debian.org/msgid-search/yiejaly0ny0+0...@torres.zugschlus.de
>
> --
> bye,
> pabs
>
> https://wiki.debian.org/PaulWise
>


-- 
 --- Inguza Technology AB --- MSc in Information Technology 
|  o...@inguza.como...@debian.org|
|  http://inguza.com/Mobile: +46 (0)70-332 1551 |
 ---