RFC for VIP17: unix domain sockets for listen and backend addresses

[Geoff Simmons]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256

This will be comments on this part of the thread so far, and on the
current state of the draft in Wiki.

On 05/15/2017 11:41 AM, Dridi Boukelmoune wrote:
>> 
>> For connect(2), BSD-derived systems don't care about permissions
>> on the UDS path at all. That is, the permissions can look as if
>> you couldn't access it, if it were a regular file, but
>> nevertheless connect will succeed. However, you can restrict
>> access by restricting the directory to which the UDS belongs.
>> (This is what I read in the docs, haven't tried it myself.)
>> 
>> With Linux, however, the process that calls connect(2) must have
>> both read and write permissions on the path.
> 
> Well, we can always set a mode and let users refer to the UDS docs
> of their platforms.
> 
>> So it seems to me that this would have to happen (remember that
>> bind creates the path):
>> 
>> - - If uid and/or gid are specified in -a - bind and set read
>> access on the containing directory for uid and/or gid, no access
>> for other
> 
> I'm not fond of fiddling with directories we don't own, for
> example, what if the user picks /tmp/varnish.sock?
> 
> My take on it is do nothing unless a mode was specified when the 
> socket is created. Simple for us, leaving the users in control.

(BTW the same discussion applies to UDSen for the -T admin address.)

/tmp is a good example, and that's probably enough to shoot this idea
down. If we do what I was thinking of, we'd have to document loudly:
"don't do that with a listen path if you're unwilling to set access
restrictions this way". Thing is, a lot of people are likely to use
/tmp just for testing (some might do it in production).

At the risk of getting long-winded and philosophical again, here's
what's been bothering me about the way this has been going.

I've had security in mind with respect to UDSen, and I've been looking
for ways to express security restrictions in the configuration -- VCL
and the command line -- so that Varnish is able to enforce them. That
way the config and Varnish could do some things to ensure that the
admin set things up the way they were intended.

An obvious use case will be connecting Varnish to SSL on- and
offloading components, although that isn't the only context in which
it's relevant for an admin to be sure that Varnish is talking to whom
it's supposed to be talking to.

If someone finds away to sandwich a man-in-the-middle between Varnish
and a peer component, then as things are now just talking to IP
addresses, there's no way for Varnish to know and nothing we can say
in the config to stop it. You can't know if the peer at the loopback
address, or any other address, is the "right" component.

UDSen with access restrictions in the file system and credentials
present ways to impose limitations on who Varnish talks to. (Of course
those can be undermined as well, but we can make it a little harder.)

Dridi, your position appears to me to be: the admin can make use of
those means to do tighten things up, and Varnish doesn't have to do a
lot of checking if they got it right. But admins famously set things
up incorrectly sometimes, and don't always notice until it's too late.
Even if they get it right the first time, a change may silently
undermine what had been working.

So I've been looking for ways to express restrictions, for example in
VCL and in this case for the listen address, so that Varnish can
enforce them, for example by failing to start, or making it possible
to write VCL that sets restrictions. That was my motivation behind
expanding ACLs to cover UDS paths.

We've dropped the ACL idea, and if we drop the idea of Varnish
*enforcing* restrictions on the listen and admin addresses -- instead
saying "I'll set uid, gid and mode as you say, admin, you have to know
what you're doing" -- well then, if there's no other way. But I think
it's worth it to remain open for ways to state restrictions in the
config that Varnish can enforce. The same thought motivates some
comments further down.

At the least we should document some of these considerations. The
User's Guide might a good place for that -- something like "for
example, on BSD-derived systems it's like this, on Linux like this ..."

>> (Incidentally, we'll have to decide what Varnish does if the UDS
>> path already exists before the bind, which causes bind to throw
>> EADDRINUSE. unlink and then bind, or throw the error, requiring
>> users to make sure it doesn't exist before starting Varnish?)
> 
> If we can't bind, we should just fail the startup. Varnish
> shouldn't do the sysadmin's job.

OK, that should be documented though (there is other software that
works with UDSen that does in fact unlink before bind).

Now on the VIP17 draft:

About std.uid, std.gid etc.:

> If local.address is not a UDS, numeric variants could also return
> -1 and name variants could return NULL. The functions could also
> take fallback parameters, possibly with a default value to the ones
> suggested (-1 and NULL).

+1, and this would be the solution (the only option) on a platform on
which peer credentials cannot be obtained.

> The question here is more whether we need something like 
> beresp.backend.path in addition to the ip field. Same question for
> peer credentials, they probably don't make sense for backends (and
> that would keep the new std functions limited to the listen
> addresses type).

For the long-winded reasons stated above, I disagree that it doesn't
make sense. %^) Especially since connect(2) doesn't create the path as
bind(2) does, getting peer credentials on a backend address is about
the only thing that could be expressed in VCL about who the peer is
intended to be, that goes beyond assuming that the admin got it right.

> Is the question of naming from the original draft still relevant?

That was about VTCP_name, _hisname and _myname, so one way or another
we'll have to decide what happens with those. We could drop them and
have them do what they do some other way, and introduce something else
again where they're currently called, when it's a UDS. git grep on
those function names currently gets over 30 hits, so that would be a
bit of a chore. The suggestion in the original draft was to avoid the
pain -- leave them as they are, and have them create names for UDSen
in way that doesn't change their interfaces.


Best,
Geoff
- -- 
** * * UPLEX - Nils Goroll Systemoptimierung

Scheffelstraße 32
22301 Hamburg

Tel +49 40 2880 5731
Mob +49 176 636 90917
Fax +49 40 42949753

http://uplex.de
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2

iQIcBAEBCAAGBQJZIrlxAAoJEOUwvh9pJNUREGYP/RZUqcO77vKt0BYVI3bj6/Xp
Cp/WNt2H1euu73OLllEDiPlGuw39VoTKD8UBmiiC9TX6ASCYwB2PKtp8mxYIqOzD
1JqyAcjz5FI/cbEAB4b5MtC1keTDqlWTkjK/0HACIhP8hdFAAksf3CIBebm+GzMH
PQvY5yqW/78eknpq7CpnGpJY3nyXwVnd5RF2MmO6tmXI0VDk6Z7uqlK1v5VV1i7A
F2LWgIam1Oqcsqx02Xqdt8pCYrJT23qmthIDAxBQ9VzUTgK25CNj6EdpeDicfLaC
rIDCSKGka5J+6WwDcUpct375D1CZGuQl1qEM+0xul33Z67L5bKijaPgEWYsMjnqW
GfZCONf759YvwEj4FOWnO9vYKrD8OUJkLAW2zl4JTz5S9HcxUEcu3nBvqY8FCEJx
DqFKfc94fEy/8U2QAhS5ZVaaMqLEexSI65taaRMQqSWka69P0twfoQy/NrN0hgaC
CxYwynvgRWrgkhI8vNLNw5H1CKWZD+BaM2w+qmIyWYGWqEygtqbIjne7ToEf9JTr
VMLo8ivhCeBtrzYDES40NBDDS0/uRNWnxx21TGrkCtaqkepjt4Vi6t3ZLhto+ZLg
bdKr/sDtBWhcRdGdpODBie2HreXDNqyYd0LrGruRHRgEtFLoRIzGLCFsc9iCWPny
aR6F0fkrUADgqbn3yUcN
=rF98
-----END PGP SIGNATURE-----