Caddy: Encoded slash in URL

[Cinerar]

1. What version of Caddy are you running (caddy -version)?

Caddy 0.9.5 (+7d15435 Sun Apr 16 00:21:33 UTC 2017)

2. What are you trying to do?

Proxy request

3. What is your entire Caddyfile?

localhost:8888 {
  # proxy to "nc -l localhost 9999"
  proxy /test http://localhost:9999
}

4. How did you run Caddy (give the full command and describe the execution environment)?

Just run caddy with this caddyfile

5. Please paste any relevant HTTP request(s) here.

 curl localhost:8888/test/%2F/foo

6. What did you expect to see?

nc -l localhost 9999
GET /test/%2F/foo HTTP/1.1
Host: localhost:9999
User-Agent: curl/7.47.0
Accept: */*
X-Forwarded-For: 127.0.0.1
Accept-Encoding: gzip

7. What did you see instead (give full error messages and/or log)?

nc -l localhost 9999
GET /test/foo HTTP/1.1
Host: localhost:9999
User-Agent: curl/7.47.0
Accept: */*
X-Forwarded-For: 127.0.0.1
Accept-Encoding: gzip

8. How can someone who is starting from scratch reproduce the bug as minimally as possible?

1. Caddyfile:

localhost:8888 {
  #proxy to "nc -l localhost 9999"
  proxy /test http://localhost:9999
}

1. Run

nc -l localhost 9999

1. Run

curl localhost:8888/test/%2F/foo

1. See

GET /test/foo HTTP/1.1
Host: localhost:9999
User-Agent: curl/7.47.0
Accept: */*
X-Forwarded-For: 127.0.0.1
Accept-Encoding: gzip

I believe this issue is #643 reintroduced with https://github.com/mholt/caddy/commit/c37481cc7b2e2476fe50e17f31421bf20c35cc75
and there are no slashes becouse of https://github.com/mholt/caddy/commit/04bee0f36d218edf0593b2c2254974f35d00276e #1298

@tw4452852 please check it out.

[tw4452852]

@Cinerar
Patch is ready, could you please check it again? Thanks!

[Cinerar]

@tw4452852 looks like everything ok now.

[mholt]

Unfortunately, the patch that fixed this broke proxying when rewrite or ext directives were used, so I reverted the change on master now. (We don't have tests that try all combinations of directives.) It looks like c684de9a887570c21959e0ba1d59fe6efbf4592c may also have had something to do to affect this issue.

We need a correct way to proxy the encoded slash, it must be done very carefully.

(Note: #1604 was affected by my revert.)

[tw4452852]

Hmm, I will try to work out a better solution.

[mholt]

@tw4452852 Would it be as easy as adding another mask to CleanMaskedPath for encoded slashes?

[tw4452852]

@mholt . You mean if the request is /%2f/test, then the URL.Path will be ///test and URL.RawPath will be /%2f/test after sanitizePath ?

BTW, do you think we should also handle the encoded dot in URL? Like /%2e%2e/%2e/test ?

[mholt]

@tw4452852 Hmm, I think the OP wants /%2f/test to go upstream, rather than ///test. And I think URL.Path has to be set to that for that to be the path, right?

(And let's not worry about encoded dots for now.)

[tw4452852]

@mholt . The problem is from sanitizePath not proxy plugin.
Let's suppose caddy receive the incoming request which is /%2f/test, and after go std lib parsing the URL, req.URL.Path = ///test and req.URL.RawPath = /%2f/test. and after sanitizePath, req.URL.Path will be changed to /test but req.URL.RawPath remains unchanged. According to the comment in net/url: "URL's String method uses RawPath if it is a valid encoding of Path, by calling the EscapedPath method.", now RawPath is a invalid, so it will be discarded...

[mholt]

Argh, this is complicated.

What if we stopped using path.Clean altogether, but manually stripped "../" and "./" from the path instead? Would that simplify any of this?

[tw4452852]

Hmm, I think this will not solve the problem entirely because Path isn't consistent with RawPath.
And could we just skip sanitizePath if the original URL contains encoded forms for now until we find a proper way?

[tw4452852]

Just a question @mholt , Is /%2f/test same as /test ? I think they are same but not sure...

[mholt]

@tw4452852

Hmm, I think this will not solve the problem entirely because Path isn't consistent with RawPath.

My hope is that if we avoid doing any extra stuff to the URL, we can just pass it through with less code and less transformations, simplifying a lot of these edge cases. But, maybe I don't understand: can you explain what you mean by this? I know that Path and RawPath are different, but they should still be related and consistent in that sense, even if they are not the exact same.

Also, wonder if we should be using u.EscapedPath() instead of accessing RawPath directly.

And could we just skip sanitizePath if the original URL contains encoded forms?

What do you mean by encoded forms? Like ?query=string? Or something else...

Is /%2f/test same as /test ? I think they are same but not sure...

I don't think so. I think they are different.

[mholt]

I'm basically wondering if we can get away with leaving the path un-touched except that we manually strip "../" and "./" from the path -- not doing any of the sanitizePath stuff we're doing.

[tw4452852]

What do you mean by encoded forms? Like ?query=string? Or something else...

I mean we could just skip sanitizePath if req.URL.RawPath != "" (there are some encoded forms in URL)

[mholt]

Ah, I see. I guess I had misunderstood how that works before.

url.URL.EscapedPath() should give us what we want: the RawPath if it's set, otherwise the Path. I think we should not mess with the path or rawpath except to trim the ../ and ./.

[mholt]

@Cinerar Hey, are you able to build from source and try out my changes in #1633? I removed path sanitizing completely (except when actually loading files from disk, which doesn't happen when proxying) so it should work for you. If you could double-check and verify in the next day or so, that'd be great!

[Cinerar]

@mholt looks like with #1633 and

curl localhost:8888/test/%2F/foo

i'm still recieveing

GET /test/foo HTTP/1.1

instead of

GET /test/%2F/foo HTTP/1.1

can you verify this?

[mholt]

@Cinerar Thanks for trying it! Hmm, no, I see:

GET /test/%2F/foo HTTP/1.1
Host: localhost:9999
User-Agent: curl/7.51.0
Accept: */*
X-Forwarded-For: ::1
Accept-Encoding: gzip

Note, on Mac I had to run nc -l -p 9999 localhost to get it to connect properly.

This is good news though, it's working for me. I probably should have included build instructions, basically do a git checkout to the preserve-uris branch, then run build.bash (in the caddy subfolder) then run ./caddy.

[Cinerar]

@mholt
Hmm thats strange ... is it right version that i should check?
Caddy 0.10.0 (+40c7f6d Mon May 01 05:30:58 UTC 2017)
i build with CGO_ENABLED=0 ./build.bash
and still recieving

GET /test/foo HTTP/1.1

Testing on Ubuntu 16.04 if this could help.

[mholt]

That version looks right. I'm not sure why the discrepancy.

[Cinerar]

Build with 1.8.1 still same result

go version go1.8.1 linux/amd64

Testing on

 cat /etc/*-release
DISTRIB_ID=Ubuntu
DISTRIB_RELEASE=16.04
DISTRIB_CODENAME=xenial
DISTRIB_DESCRIPTION="Ubuntu 16.04 LTS"
NAME="Ubuntu"

Caddy Version

./caddy --version
Caddy 0.10.0 (+40c7f6d Mon May 01 05:56:58 UTC 2017)

Caddyfile

localhost:8888 {
  # proxy to "nc -l localhost 9999"
  proxy / http://localhost:9999
}

curl localhost:8888/test/%2F/foo



md5-d34f9cda9690207db3bc52521bbe9220



nc -l localhost 9999
GET /test/foo HTTP/1.1
Host: localhost:9999
User-Agent: curl/7.47.0
Accept: */*
X-Forwarded-For: 127.0.0.1
Accept-Encoding: gzip

[mholt]

@Cinerar I spun up a Ubuntu 16.04 VM just for you. :smile: Here's my result doing your steps exactly:

ubuntu@ubuntu-xenial:/vagrant/caddy$ nc -l localhost 9999
GET /test/%2F/foo HTTP/1.1
Host: localhost:9999
User-Agent: curl/7.47.0
Accept: */*
X-Forwarded-For: 127.0.0.1
Accept-Encoding: gzip

(Your nc command works as intended on Linux, but it's different on Mac. Oh well.)

My Caddyfile:

localhost:8888
#proxy to "nc -l localhost 9999"
proxy / http://localhost:9999
log stdout
errors stderr

Caddy log:

127.0.0.1 - - [01/May/2017:14:00:19 +0000] "GET /test/%2F/foo HTTP/1.1" 502 16

Seems good here. I honestly have no idea why you're not seeing the same result.

[Cinerar]

i'm out of ideas also. Will give it a try again next day or so.

[mholt]

@Cinerar Okay. I will still go forward with the release and assume this is fixed, because I definitely see it as resolved on my end. Sorry for the trouble, whatever it is. If you ever figure it out or have new findings, feel free to continue commenting!

[thedodd]

I am seeing this same issue running 0.10.10.

:80/admin/mq-management/ {
    # Log everything to stdout. Pattern is as follows:
    # {remote} - [{when}] "{method} {uri} {proto}" {status} {size} "{>Referer}" "{>User-Agent}"
    log / stdout "{combined}"

    proxy / rabbitmq:15672 {
        transparent
    }
}

Please note:

• the proxy is successful. I can see the rabbitmq control panel.
• the problem is with any URL involving an escaped forward slash (%2F) the request fails (rabbit returns a 405, believes the request is for a different resource due to slashes being merged).
• when I visit the same rabbitmq control panel outside of the reverse proxy (I have this all replicated on my localhost), the problem does not exist. So the problem is coming from the proxy.