relative imports and duplicate includes

[Kacper Wysocki]

Hi.
On Tue, Nov 17, 2015 at 11:16 AM, Poul-Henning Kamp <phk at phk.freebsd.dk> wrote:
> --------
> In message <CABaBnj4AVVgkkc6d2fjdu8FTrxmWAGZDV7JcUdopsCSPK0_duA at mail.gmail.com>, Kacper Wysocki writes:
>
>>attached is a patch to make 'import foo' relative to the importing source file,
>
> We thought a lot about this originally, and the conclusion back
> then was to not do it for reasons of predictability and security.

Main motivator for me hammering out this patch was to make vcl
packages possible with a minimal amount of impact on existing code out
there.

I thought there might be reasons for the current behavior. I am
proposing a predictable way to solving this, but regarding security it
might make sense to talk threat models.

> First:  What is "relative to the importing source file" when you
> load with vcl.inline ?

Relative to vcl_dir.
Looking over all the different ways a VCL could enter, if you're
inlining or f_arg'ing or if it's builtin and there is an include
statement, it'll have to resolve somehow. Without the patch, that is
vcl_dir. With patch, that is still vcl_dir.

> There are also a host of security/trust issues.

I am wondering about those trust issues. If you include untrusted vcl,
isn't the game over anyway, includes or nay?
Are we talking about insecure paths? I have poignantly avoided any
parsing or validating of paths in my code past the existing "does the
filename foo actually open"

> What if you include "foo", which comes from vcl_dir, and foo includes
> "bar" ?

> Do you look for "bar" in the original "relative to the importing
> source file" dir first or do you look only in vcl_dir ?

Assuming you mean "foo.vcl" and "bar.vcl" then they are in the same
directory and nothing changes. If foo and bar are in subdirectories,
ie "path/to/foo.vcl" which includes "relative/path/to/bar.vcl" then
bar will be loaded at path/to/relative/path/to/bar.vcl".

As a rule, I only look in vcl_dir for includes that can't be resolved
relative to the including file.

There has got to be a topdir for includes anyway. f_arg default-vcl
gets loaded as either an absolute path or from this directory, and
includes are loaded from this directory.

Check the testcase provided in the patch for how this should work for
the not-so-general cases.

> What if you include "/foo/bar/barf.vcl" and it includes "murphy.vcl"
> which lives in vcl_dir, and murphy includes "heisenberg.vcl" ?

Under the new behavior, we error out because /foo/barf/murphy.vcl does
not exist, unless vcl_dir is set to /foo/bar.

> Where do you look for heisenberg.vcl ?
If /foo/bar/murphy.vcl did exist, we'd look for /foo/barf/heisenberg.vcl, too.

> If we want to reopen this issue, I think the much more
> predictable and sensible way to go about it is to turn
> vcl_dir (and vmod_dir?) into lists of paths to search,
> (just like $PATH).

It's quite possible that there are more elegant solutions, but I'd
like to agree on the problem domain at least.
How does the "just like $PATH" behavior solve the original problem of
"I wanted to release complete, self contained vcl packages"?

An instance of the problem I have out there is
a multi-site varnish has vcl snippets site1/main.vcl which includes
site1/foo.vcl and site1/bar.vcl and site2/main.vcl which includes
site2/foo.vcl site2/zool/baz.vcl. Today these includes have to either
be absolute, or relative to vcl_dir.

Tomorrow, site1 and site2 could be movable package-like things where I
could rename the directory, put them on a different varnish server,
make vcl_dir=path/to/site2 and even let site1 include site2 if I so
wished.

>>and a patch to remove the error upon duplicate 'include'.
>
> You mean "import" right ?

yes. those early-morning emails..

> What if one is:
>
>         import "foo";
>
> and the other is:
>
>         import "foo" from /somewhere/libvmod_foo.so";
>
> Isn't that a problem ?

I don't know. In either case you're assuming something about what the
user wants. I would like to do

import "foo";
include "some/other/vcl";

in my default vcl and not have some/other/vcl break because it too imports foo.
That's behavior which is in line with the "first come first serve"
behavior of ie multiple vcl_recv subs.

It is a bonus that I can override where some/other/vcl loads its vmod
from. Saves us modifying all the vcl to match every operational
environment out there.

-Kacper