Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RFC: aux_path parameter #3181

Closed
nigoroll opened this issue Jan 6, 2020 · 12 comments
Closed

RFC: aux_path parameter #3181

nigoroll opened this issue Jan 6, 2020 · 12 comments

Comments

@nigoroll
Copy link
Member

nigoroll commented Jan 6, 2020

This ticket is to ask for opinions about whether or not an aux_path parameter (other names suggested so far: cache_path, search_path) would be considered worth the implementation effort.

  • aux_path would define a search path paralleling vcl_path and vmod_path, but for auxiliary use cases like files with synthetic response pages.
  • a default aux_path could be $PREFIX/etc/varnish/aux:$PREFIX/share/varnish/aux
  • aux_path would be available to vmods, but with the currently envisioned simplified implementation, only during vcl events and vmod initialization (iow from CLI context)
    • this would imply that changes to aux_path would only become effective for newly loaded VCLs
  • the primary use case for aux_path would be a default search path for vmods handling files, see for example vmod_file

(subjective) summary of previous discussion:

  • vcl_path cannot easily be made available in the worker process because it gets changed in the master process and the respective memory region is not shared. (see RFC: vmod read-access to parameters #3172 (comment) )
  • PRO: a new parameter may simplify configuration of multiple instances using the same VCL
  • CON: instead of a parameter, a per-instance include can be used
  • CON: could be problematic if we were to confine the worker process to some subtree (think: chroot(), namespaces)

Please provide feedback how important/useful such a parameter would be for you.

@slimhazard
Copy link
Contributor

std.fileread() could also take advantage of aux_path. (VMOD file can be thought of as "std.fileread() except you can change the file".)

IMO Varnish can and should provide the tools that allow admins to decide how they can restrict the worker process and also provide access to the file system. For example, aux_path could be set so that it is accessible within a chroot jail.

@dridi
Copy link
Member

dridi commented Jan 6, 2020

What should std.fileread() do with a relative path today? I don't think accessing the working directory makes sense. As per the bugwash discussion, I'd rather have something like a VRT_SearchPath() function for relative paths lookups. I don't know yet what default value we would want or how we would manage the "auxiliary" patch, I need to ponder this.

But if this is hidden behind the VRT curtain, can't we make it update-able via the CLI? The master already forwards CLI commands to the child, we can maybe have a syntax for "private" commands that would never be forwarded?

@aux_path <string>

The manager could know NOT to forward such a command (starting with @):

$ varnishadm @aux_path "/something"
Unknown request.
Type 'help' for more info.

Command failed with error code 101

But it could then use it internally:

mgt_private_askchild(cli, "@aux_path %s", aux_path);

With the appropriate tweak_aux_path function, with all the bells and whistles to sanitize the input and forward to the cache if started.

Then we need to figure how to make this update-proof, but if this is hidden behind VRT it should be possible to come up with something safe.

a default aux_path could be $PREFIX/etc/varnish/aux:$PREFIX/share/varnish/aux

I have been thinking about this for a while, I think we should also fix the default vcl_path:

-$PREFIX/etc/varnish:$PREFIX/share/varnish/vcl
+$PREFIX/etc/varnish/vcl:$PREFIX/share/varnish/vcl

The reason being that /etc/varnish is where we would put files such as a CLI secret, a CLI -I script, etc which is unrelated to VCL. Such files should not be in the default vcl_path.

The moment we do this there will be an outcry for the dearly beloved /etc/varnish/main.vcl but I think our linux packaging should go with -b by default instead of an arbitrary VCL file (similar to the default configuration of the FreeBSD port).

I know you will consider this off topic, but I want to make sure to bring this up here if we add a new path with a default value in $PREFIX/etc. That would be the appropriate cue to revisit the default vcl_path and as such our packaging.

@rezan
Copy link
Member

rezan commented Jan 6, 2020

Should this feature be implemented within the vmod? Or maybe make a way for vmods to define params? Making a core feature for a 3rd party vmod seems like we are glazing over a real need.

@nigoroll
Copy link
Member Author

nigoroll commented Jan 7, 2020

@rezan my question is if such a parameter would be considered generally useful - if there is any demand for it. If by saying for a 3rd party vmod you imply that you do not consider it generally useful, I am happy to take this as your -1 vote. But there would be at least std.fileread which could make use of it.

@dridi :

  • Thank you for the update idea, but this does not look like much of a simplification to me. At any rate, I think this could better be discussed based on a PR

  • +1 on making the default vcl location .../etc/varnish/vcl. And maybe even move the secret and other internals to .../etc/varnish/private ?

@slimhazard
Copy link
Contributor

A hard -1 (-Inf if could) for changing the default value of vcl_path. /etc/varnish/vcl may have its advantages, but it would mean that almost everyone would have to change their VCL deployments. Users would be coming with pitchforks, and I wouldn't blame them. (I might join them.)

I don't see why a secret can't be generated in the Varnish home directory, as Varnish does by itself in the default case. Or put the secret in some other location entirely, not on the vcl_path and only readable by a group that includes the management process owner and admin users. That would mean using -n or -S for everything, but it's probably the most secure option, if that's the priority.

Disrupting everyone's VCL deployments because of a problem that can easily be solved in other ways strikes me as sheer madness.

@dridi
Copy link
Member

dridi commented Jan 7, 2020

this does not look like much of a simplification to me.

Like I said I would need to ponder this, but this is the idea I got after the string parameters discussion. A way to safely pass a string parameter to the child, via the CLI.

and maybe even move the secret and other internals to .../etc/varnish/private ?

My idea was rather to align what's in $PREFIX/etc/varnish with what's in $PREFIX/share/varnish. I think anything non-VCL would be fine staying in $PREFIX/etc/varnish, and stick to a vcl/ sub-directory for both default entries in vcl_path.

For example in an environment with multiple -n instances:

$PREFIX/etc/varnish/foo_secret
$PREFIX/etc/varnish/foo.cli
$PREFIX/etc/varnish/bar_secret
$PREFIX/etc/varnish/vcl/foo/main.vcl
$PREFIX/etc/varnish/vcl/foo/label_x.vcl
$PREFIX/etc/varnish/vcl/foo/label_y.vcl
$PREFIX/etc/varnish/vcl/foo/label_z.vcl
$PREFIX/etc/varnish/vcl/bar/main.vcl
$PREFIX/etc/varnish/aux/foo/404.html
$PREFIX/etc/varnish/aux/foo/503.html
$PREFIX/share/varnish/vcl/my_vmod/inc.vcl
$PREFIX/share/varnish/aux/my_vmod/some.file
...

That would be compatible with the current SELinux varnishd module, and something end-users could opt into using. And more importantly to me, we'd stop mixing everything up in $PREFIX/etc/varnish.

But there would be at least std.fileread which could make use of it.

And also std.blobread pending a resolution :)

@nigoroll
Copy link
Member Author

nigoroll commented Jan 7, 2020

@slimhazard do you have that strong an opinion even if -p vcl_path=... and adding symlinks are simple transition methods?

@dridi
Copy link
Member

dridi commented Jan 7, 2020

That would mean using -n or -S for everything, but it's probably the most secure option, if that's the priority.

Using an explicit -n is already something you have to actively do, we have a sensible default value for that.

And regarding -S, unless you want remote CLI access you are much better off not using it for security reasons since a random secret will be generated instead. This means that in order to talk to varnishd with no default -S you would need local access with enough privileges.

This is already the case for the 6.x packaging:

https://github.com/varnishcache/pkg-varnish-cache/blob/6.x/systemd/varnish.service

I also had a proposal to get rid of further arbitrary defaults in our packaging:

varnishcache/pkg-varnish-cache#106

Unsurprisingly it was met with pitchforks but I'm still convinced that we should:

  • drop the arbitrary -f in favor of -b :8080
  • drop the arbitrary -a in favor of the sensible default HTTP port on all network interfaces
  • drop the arbitrary -s since we already have a similar default storage backend
  • package an empty /etc/varnish directory (and empty aux/ and vcl/ sub-directories)

The defaults I'm suggesting are actually closer to what one would usually want for production (listening to HTTP traffic and forwarding to a private 8080 HTTP backend) but this is however a topic for pkg-varnish-cache.

On the topic of aux_path, I think aligning vcl_path in a similar fashion should be the way to go.

All of the above is either trivial to cope with during a _major_ upgrade, or something you probably _already_ have to cope with (-a and -s) for a production environment. So in my book that kind of change is OK for a 7.x series with the appropriate headlines in the upgrading/what's new documentation.

@rezan
Copy link
Member

rezan commented Jan 7, 2020

@rezan my question is if such a parameter would be considered generally useful - if there is any demand for it.

I would find it very useful if there was a framework where vmods could define command line parameters on their own, without having to modify core Varnish code.

We wrote a vmod_file a few years ago and solved this problem by just exposing an ACL API within the VMOD itself and allowing an optional root directory. So there are solutions to this problem outside of adding core parameters.

@nigoroll
Copy link
Member Author

nigoroll commented Jan 8, 2020

@rezan No question that we can set a search path in vcl_init {} (and we already do that in the other vmod_file). The question that is being asked here is if we should have a parameter to be used as a default. The use case would be to make it easier to re-use the same vcl in different environments.

command line parameters for vmods are problematic because vmods are loaded long after the command line is parsed. But extending your idea a bit might make sense: We could have arbitrary custom parameters, e.g. with some marker prefix like custom_. Obviously they would all be string typed.

Another idea that had been discussed previously was a tell <vcl>.<vmod> ... CLI command to trigger some runtime action/change in vmods.

@rezan
Copy link
Member

rezan commented Jan 8, 2020

Ya, didn't think about the timing, vmods get loaded too late for this to be possible.

Anyway, I have been thinking about this for security and sandboxing filesystem reads and writes. Its just going to be a challenge to enforce this across the entire vmod ecosystem. But it would be useful to have this from a policy perspective. If this is for convenience for your potential vmod_file, then I dont have much else to say.

@nigoroll
Copy link
Member Author

it's dead.

the idea of a simple fixed length buffer shared between mgmt and worker process was refused and anything else would overly complicate the cli dance.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants