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

2017-05-22 Thread Dridi Boukelmoune
> OK, that should be documented though (there is other software that
> works with UDSen that does in fact unlink before bind).

There should be a limit to hand-holding. I have bitter memories of
tracking down processes holding unlinked files around and leaving a
partition with no space. The upside was that it introduced me to
lsof(1) at the time.

I would invoke POLA, at least it would feel surprising to me, not
being sure what service I'm connecting to when I pick a path.

>> 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.

Well, I was cautious enough to say probably. I'm basing my reasoning
on the fact that Varnish tends to trust the backend. It was recently-ish
reconfirmed [1] by phk.

>> 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.

I will confess I haven't researched this part thoroughly, that's why I
left it as an open question and linked directly to your original draft.

Dridi

[1] 
https://github.com/varnishcache/varnish-cache/issues/2197#issuecomment-275055034

___
varnish-dev mailing list
varnish-dev@varnish-cache.org
https://www.varnish-cache.org/lists/mailman/listinfo/varnish-dev


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

2017-05-22 Thread 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 

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

2017-05-19 Thread Dridi Boukelmoune
> +1 I just had a use case for this yesterday, which might also be a general use
> case: cross-container communication (in docker). Sharing a file system with a
> UDS (read only) between container is safe and easy, while configuring a shared
> network between containers is not.

The VIP now covers both -a -T and -M.

> I must say though that this use case calls for more finely grained access
> control for cli connections. Sounds like we could want a cli vcl?

The varnish-cli is already its own language so I don't see how a "cli
vcl" would fit in the picture.

I think that loopback+secret or uds[+secret] is already quite fine.
You need a local access somehow and local credentials with enough
privileges to even use the CLI.

Dridi

___
varnish-dev mailing list
varnish-dev@varnish-cache.org
https://www.varnish-cache.org/lists/mailman/listinfo/varnish-dev


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

2017-05-18 Thread Dridi Boukelmoune
On Mon, May 15, 2017 at 11:41 AM, Dridi Boukelmoune  wrote:


Hello,

Making slow progress on the draft, but getting there. While gathering
details from the first v6wash and the original draft, I found that
named listen addresses could once again solve one of the issues.

In this case, that would be ACLs containing paths. With named listen
address you could already implement name-based access control (with
strong typing) and I figured we could do path matching too (via string
conversion).

https://github.com/varnishcache/varnish-cache/wiki/VIP-17:-Enable-Unix-domain-sockets-for-listen-and-backend-addresses/_compare/5b5ca04148c0420789af4006f0f1b2bf1815f53c...caa8cf86af2ab1e8c37f2a80bee29f4a16333898

Thoughts?

Thanks,
Dridi

___
varnish-dev mailing list
varnish-dev@varnish-cache.org
https://www.varnish-cache.org/lists/mailman/listinfo/varnish-dev


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

2017-05-15 Thread Dridi Boukelmoune
> First of all (a lesser matter): OK, we can go with commas as a
> separator, with the consequence that UDS paths may not have a comma.
> There is a precedent for that with -sfile.

Yes, we can probably get away with no commas in file names.

Also see this comment from phk:

https://github.com/varnishcache/varnish-cache/issues/2325#issuecomment-301413244

> The more important matter is the mode. *If* we want to do this -- set
> access restrictions on a listen UDS for UID and/or GID --, then if I'm
> understanding the various platform quirks correctly, we can't have a
> mode option. Because if you want to restrict access to a user and/or
> group, then the management process will have to set permissions a
> certain way, and the user cannot have any options.
>
> 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.
>
> I believe that covers all the possibilities for the platforms that
> Varnish supports.

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?

>   - set read/write access on the path for uid and/or gid, no access
> for other
> - - otherwise (the UDS should be accessible to everyone)
>   - bind and set read/write access for everyone on the path
>
> That would mean that users don't have any options for setting the
> mode, the management process has to control that. Setting mode in -a,
> in the best case, would get it just right, but would most likely screw
> it up.
>
> If you don't do anything, then yes, the permissions on the path
> created by bind depend on umask. But if we want to implement access
> restrictions, then I believe we'd have to do something like this.
>
> Another possibility would leave the "otherwise" case to whatever
> results from umask. Or that we don't implement this feature at all, at
> least not in the next release. But in either case, I'm not sure if
> "leave it to the umask" is a good idea, it might result in other
> processes being unable to connect, forcing users to set a umask before
> starting Varnish. They would have to know to do that, for one thing,
> and they might have to set a wide-open umask that we wouldn't want
> otherwise.

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.

> (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.

Cheers,
Dridi

___
varnish-dev mailing list
varnish-dev@varnish-cache.org
https://www.varnish-cache.org/lists/mailman/listinfo/varnish-dev


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

2017-05-15 Thread Geoff Simmons
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA256

On 04/25/2017 04:42 PM, Dridi Boukelmoune wrote:
> 
>> ## Access permissions on the listen address For the ``-a``
>> address, I suggest an optional means of specifying who can access
>> the UDS: ``` varnishd -a unix:/path/to/uds:uid=foo,gid=bar ```
> 
>> If no access restrictions were requested, then don't manipulate 
>> ownership, let bind create the UDS, and set its permissions to
>> 0666.
> 
> Wouldn't it be based on umask instead?

This is something that I unfortunately never responded to, but I want
to get back to it as a comment on the current draft of VIP17, since
the VIP now has this as a kind of example:

varnishd -a /path/to/http.sock,proto=,uid=varnish,mode=0600

First of all (a lesser matter): OK, we can go with commas as a
separator, with the consequence that UDS paths may not have a comma.
There is a precedent for that with -sfile.

The more important matter is the mode. *If* we want to do this -- set
access restrictions on a listen UDS for UID and/or GID --, then if I'm
understanding the various platform quirks correctly, we can't have a
mode option. Because if you want to restrict access to a user and/or
group, then the management process will have to set permissions a
certain way, and the user cannot have any options.

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.

I believe that covers all the possibilities for the platforms that
Varnish supports.

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
  - set read/write access on the path for uid and/or gid, no access
for other
- - otherwise (the UDS should be accessible to everyone)
  - bind and set read/write access for everyone on the path

That would mean that users don't have any options for setting the
mode, the management process has to control that. Setting mode in -a,
in the best case, would get it just right, but would most likely screw
it up.

If you don't do anything, then yes, the permissions on the path
created by bind depend on umask. But if we want to implement access
restrictions, then I believe we'd have to do something like this.

Another possibility would leave the "otherwise" case to whatever
results from umask. Or that we don't implement this feature at all, at
least not in the next release. But in either case, I'm not sure if
"leave it to the umask" is a good idea, it might result in other
processes being unable to connect, forcing users to set a umask before
starting Varnish. They would have to know to do that, for one thing,
and they might have to set a wide-open umask that we wouldn't want
otherwise.

(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?)


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

iQIcBAEBCAAGBQJZGW9nAAoJEOUwvh9pJNURaxQQAIlpTRr7/oxcytIQRxXd3WfS
q/L3OvhuZ2cm0RhylAVscKvse951rFL0yHx1oUJdzxnkCQ7ZRvGwjixBvD8mzgaa
R2IWWfQwO/cT4FKB4sYLJOxJMUS1zeWa8Kh9WDa/UOijH67RWsoasNfewvLLjWfX
qJjzIoc7YN5WzAXf1j4BJqvsbH5Q8xN38TMMxqVQ6XhyL+QieSxGK3y2hEYTHz27
jgWGxR1iTOknawYYQPXRyRZqnzkgMLjdIQuNTZWZtUKoLqu1x+UuJ/Rs9Sn31YrN
Uc6Q00Dz6dYf3WyeoKc2SGZmCmVYcYZf7E0y+KLbJQSBQK0uBafCvU8teXAotLDL
PqitZjvgh2EkFn8dVzKubbg0zQesWXMU5I8BHf5zau2NN4nKD+PPs8Fb1sTsiAL2
JZ2MyJfvr47NMUoto4jcgduNupAdqglPuzzir3hmyl45pIgD2PMTprwmcyOYq1j4
05ystoyHWan0KXHO8aWBKF38dIhh1DJjYnF6b1ZU83RlXnhfQEeZBufoN9nl+BzO
CojUxCnzIoHF08XJrQp+y8cGBJ6u4bfcfeqdY5CjeTWRhKiJrOZh7/47d2EpOZ7Y
ctZ6pQM3/EwdBlMLCb2q6hs2J2agyQ76izDAs+T8pOy08XN1ppc2Poh6Y6zxI9TH
1jNDph4vSKCuP6JlZwpG
=b2M/
-END PGP SIGNATURE-

___
varnish-dev mailing list
varnish-dev@varnish-cache.org
https://www.varnish-cache.org/lists/mailman/listinfo/varnish-dev

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

2017-05-10 Thread Dridi Boukelmoune
>> So I say do it right from the beginning this time, and make it
>> possible to use relative paths and just change the uds_path parameter
>> when you have to.
>
> Define "when you have to", as I said earlier listen addresses are
> immutable once the manager is started. That's a condition that
> we must hold since we are to expose them in VCL.

Just when I decide to move to something else for the rest of the day I
get hit hard with this thought:

A uds_path would actually make sense for backend definitions, so
actually I'm not against the idea. For the reasons exposed previously
I still think we should stick to absolute paths on the client/listen
side.

Cheers,
Dridi

___
varnish-dev mailing list
varnish-dev@varnish-cache.org
https://www.varnish-cache.org/lists/mailman/listinfo/varnish-dev


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

2017-05-08 Thread Geoff Simmons
On 04/25/2017 04:42 PM, Dridi Boukelmoune wrote:
>> 
>> # How? ## Address notation I suggest that we require a prefix
>> such as ``unix:`` to identify UDS addresses (nginx uses
>> ``unix:``, haproxy uses ``unix@``): ``` varnishd -a
>> unix:/path/to/uds
> 
> This should be enough:
> 
> varnishd -a /path/to/uds

Dridi, I'm sorry I never answered all of this, after you put the
effort into responding.

On a tight schedule today (and I unfortunately can't make it to
bugwash today, May 8th), but I'd like to elaborate on this part a bit.

What I forgot to say in WIP17: I would suggest a varnishd parameter
uds_path, paralleling vcl_path and vmod_path, which specifies a path
in which to search for relative UDS paths in a -a argument and in
backend definitions.

From that it follows that we couldn't identify a UDS address
unambiguously as having a '/' as the first character, which is why I
think we should require the 'unix:' prefix (or some other prefix).

I think experience has shown that absolute paths in the Varnish
configuration, which used to be necessary for "include", for example,
leads to awkward problems, and the two *_path parameters have been a
relief. And I think we'll find that requiring absolute paths
everywhere for UDS addresses will lead to the same kinds of problems
-- say, you're running a test instance of your Varnish deployment in
an environment where files and directories a laid out differently from
the production environment. So then you'd have to get sed or something
replace all of the absolute paths, just like we used to have to do
with absolute include paths.

So I say do it right from the beginning this time, and make it
possible to use relative paths and just change the uds_path parameter
when you have to.

Notice that if we do have relative paths, it's not impossible to have
a file named "127.0.0.1", or anything else that looks just like an IP
address, as the file that's meant to be a UDS address. Of course
that's very unlikely and just asking for trouble. But the point is
that, strictly speaking, we *couldn't* have an unambiguous distinction
between IP and UDS addresses *unless* we require all UDS paths to be
absolute (and begin with a '/').

I say let's do everyone a favor by having uds_path and the 'unix:' prefix.


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



signature.asc
Description: OpenPGP digital signature
___
varnish-dev mailing list
varnish-dev@varnish-cache.org
https://www.varnish-cache.org/lists/mailman/listinfo/varnish-dev

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

2017-04-25 Thread Dridi Boukelmoune
> # Synopsis
> Allow unix domain sockets as a listen address for Varnish (``-a``
> option) and as addresses for backends, and for use in ACLs. Obtain
> credentials of the peer process connected on a UDS, such as uid and
> gid, for use in VCL.

Except for ACLs, I find the idea compelling so far. I would even like
to see UDS support for admin sockets (-T option).

> # Why?
> * Eliminate the overhead of TCP/loopback for connections with peers
> that are colocated on a host with Varnish.

Yes.

> * Restrict who can send requests to Varnish by setting permissions on
> the UDS path of the listen address.

The whole point of UDSs IMO.

>   * (But see the discussion below about getting this right portably.)
> * Make it possible for a backend peer to require restricted
> credentials for the Varnish process by setting permissions on the UDS
> path on which it listens.

It is technically possible to implement a UDS backend if one is brave
enough to re-implement all the VBE logic. So I'm strongly in favor of
having this capability in varnishd.

> * Peer credentials make it possible to:
>   * Make information about the peer available in VCL and the log.

Why not, no opinion.

>   * Extend ACLs to make it possible to place further restrictions on
> peers connecting to the listen address.

I would use a regex instead of messing with ACLs.

<...snip...>

> I would like to make this contribution for the September 2017 release.
> With the VIP I'd like to clarify:
>
> * Are there any changes planned for VTCP and VSA in the September
> release that would make adding UDS to those interfaces less trivial
> than it is now?

I wouldn't mix UDS with VSA, there's probably room for a different solution.

> * Every platform has a way to get peer credentials from a UDS, but
> there's no standard and it's highly platform-dependent. So how do we
> want to handle that?

Maybe we could start by not having them, being able to use UDSs is
already a huge win IMO.

> * Additions/changes to VCL and other changes in naming, such as the
> ``-a`` option and backend definitions.

I don't think we need to change -a or -T, as long as we force absolute
names we should be able to get away with the current syntax. An address
starting with a slash (/) would denote a UDS. [1]

> * If someone knows a reason why we shouldn't do this at all, this is
> the place to say so.
>
> # How?
> ## Address notation
> I suggest that we require a prefix such as ``unix:`` to identify UDS
> addresses (nginx uses ``unix:``, haproxy uses ``unix@``):
> ```
> varnishd -a unix:/path/to/uds

This should be enough:

varnishd -a /path/to/uds

> backend uds { .host = "unix:/path/to/uds"; }

I would instead go for a .path field mutually exclusive with .host and
.port, removing ambiguity at vcl.load-time (error messages etc).

> ```
> That makes the interpretation unambiguous. We could simply interpret
> paths as UDS addresses when they appear in those places, but then we
> would need logic like: if the argument cannot be resolved as a host or
> parsed as an IP address, then assume it's a path for UDS, but if the
> path does not exist or cannot be accessed, then fail. So better to
> just make it unambiguous.

As I said earlier, I think a slash [1] is enough to remove ambiguity.

> Parsing UDS addresses would be an extension of ``VSS_Resolver``.

Not if we don't mix paths with IPs/domains

> The name ``.host`` in a backend definition becomes a bit peculiar if
> its value can also be a UDS (we will see a number or examples like
> this). We could:
>
> * stay with the name ``.host``, and document the fact that it might
> not identify a host in some cases
> * replace ``.host`` with a name like ``.peer``, sacrificing backward
> compatibility
> * introduce ``.peer``, retain ``.host`` as a deprecated alias, and
> remove ``.host`` in a future release
>
> I suggest the last option, comments welcome.

Once again, I suggest we don't mix them up so that we don't need to
break anything. I also find .peer ambiguous.

> ``.port`` in a backend definition is already optional, and is
> unnecessary for a UDS. Should it be an error to specify a port when a
> UDS is specified, or should it be ignored? Comments welcome.

As stated above, mutually exclusive with the .path field.

> ## Access permissions on the listen address
> For the ``-a`` address, I suggest an optional means of specifying who
> can access the UDS:
> ```
> varnishd -a unix:/path/to/uds:uid=foo,gid=bar
> ```
> There's an issue here in that the separator (``:`` in the example)
> could not appear in any UDS path. We might just have to forbid a
> certain character in UDS paths. Fortunately we don't have a such a
> problem with backend addresses (which are generated by another server,
> so we have less freedom to impose restrictions on the path names).

I would use the comma separator like -j and -s options for jails and
storage backends. Possibly named parameters like in -j so that order
doesn't matter. But that means breaking the