syz-manager: add http prefix to pages

[pbelskiy]

Hello,

I want to start syzkaller with some http prefix, because I use nginx as reverse proxy to it, so URL inside becomes invalid.

For example, set --prefix=/syzkaller to make syzkaller accessible at http://server:56741/syzkaller with valid URLs inside it, like http://server:56741/syzkaller/config instead of invalid http://server:56741/config when no prefix option and reverse proxy used.

As I see in master, there is no such feature HTTP endpoints maps directly. https://github.com/google/syzkaller/blob/master/syz-manager/html.go

[pbelskiy]

I can create PR for this feature if you don't mind, probably it's not so complicated.

[a-nogikh]

To be honest, I'm not sure if this feature is indeed essential. The reverse proxy must definitely let you specify rules for rewriting URLs, so that e.g. your queries like http://server:NGINX_PORT/syzkaller/config would be mapped to http://syz-server:SYZKALLER_PORT/config by nginx itself.

[pbelskiy]

To be honest, I'm not sure if this feature is indeed essential. The reverse proxy must definitely let you specify rules for rewriting URLs, so that e.g. your queries like http://server:NGINX_PORT/syzkaller/config would be mapped to http://syz-server:SYZKALLER_PORT/config by nginx itself.

No, because your software update, and NGINX admin cannot always update a lot of internal links for its rewriting, for example when on new release you change /config to full_config, my NGINX mappings will be broken, it's bad software design.

[a-nogikh]

But these rules don't have to be defined for each individual URL, do they? You would just specify that each access to /syzkaller/{XYZ} URL should be converted to /{XYZ} query to the syzkaller server. I think it might even be the default nginx proxy behaviour.

[pbelskiy]

So, who will rewrite all URLs in index.html of syzkaller? Nginx?

[a-nogikh]

Hmm, that's a good point, didn't think about that :)

Technically even that can be done by nginx (sub_filter), but yes, it turns out that syzkaller itself is not really compatible with being run from a non-root URL.

We have (at least) two options, then:

• Switch our urls from being root-relative (starting with /) to being just path relative (starting with ./). This would be the simplest solution, but potentially we might have problems if we later ever introduce some more deep http endpoints.
• Introduce an extra parameter, as you said, and adjust everything to that.

You can create a PR with this feature, if you want to. Just assign this issue to yourself in this case, so no one else picks it.

[pbelskiy]

Okay, second option is better, so I will prepare PR. Could you assign it to me? Because I cannot.

[pbelskiy]

@a-nogikh According to contribution docs: https://github.com/pbelskiy/syzkaller/blob/master/docs/contributing.md

I got a bunch of problems: 1) No src directory in $GOPATH, need to use export GO111MODULE=off for my gaoling 1.17 2) Using syz-env presubmit command fails on your master, it master okay?

~/go/src/github.com/google/syzkaller(my-branch) » syz-env make presubmit                                                                                            2 ↵ themis@live
make presubmit_smoke
make generate
make descriptions
go list -f '{{.Stale}}' ./sys/syz-sysgen | grep -q false || go install ./sys/syz-sysgen
make .descriptions
bin/syz-sysgen
make[4]: *** [Makefile:147: .descriptions] Killed
make[3]: *** [Makefile:144: descriptions] Error 2
make[2]: *** [Makefile:233: generate] Error 2
make[1]: *** [Makefile:289: presubmit_smoke] Error 2
make: *** [Makefile:284: presubmit] Error 2

3) Tests are also failed with syz-env

~/go/src/github.com/google/syzkaller(my-branch) » syz-env go test -short ./pkg/csource                                                                              2 ↵ themis@live
--- FAIL: TestExecutorMacros (0.00s)
panic: runtime error: invalid memory address or nil pointer dereference [recovered]
    panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x0 pc=0x5bb382]

goroutine 7 [running]:
testing.tRunner.func1.2(0x6266c0, 0x864000)
    /usr/local/go/src/testing/testing.go:1144 +0x332
testing.tRunner.func1(0xc000001680)
    /usr/local/go/src/testing/testing.go:1147 +0x4b6
panic(0x6266c0, 0x864000)
    /usr/local/go/src/runtime/panic.go:965 +0x1b9
github.com/google/syzkaller/prog.(*Target).DefaultChoiceTable(0x0, 0x0)
    /syzkaller/gopath/src/github.com/google/syzkaller/prog/target.go:225 +0x22
github.com/google/syzkaller/pkg/csource.TestExecutorMacros(0xc000001680)
    /syzkaller/gopath/src/github.com/google/syzkaller/pkg/csource/csource_test.go:123 +0xaa
testing.tRunner(0xc000001680, 0x6defb8)
    /usr/local/go/src/testing/testing.go:1194 +0xef
created by testing.(*T).Run
    /usr/local/go/src/testing/testing.go:1239 +0x2b3
FAIL    github.com/google/syzkaller/pkg/csource 0.007s
FAIL

[a-nogikh]

I've just run syz-env make presubmit from my machine and it finished successfully. Is the output you provided complete or have you cut some parts of it?

I was also unable to reproduce the error you got when running syz-env go test -short ./pkg/csource. Do you get the same error if you execute go test -short ./pkg/csource (without syz-env)?

Not sure if this is the problem, but just in case try to run docker pull gcr.io/syzkaller/env. The image was updated not so long ago, and if you had already downloaded that image some time ago, it might just be out of date.

[dvyukov]

Switch our urls from being root-relative (starting with /) to being just path relative (starting with ./). This would be the simplest solution, but potentially we might have problems if we later ever introduce some more deep http endpoints.

This sounds like the right solution. What's the problem with "deep http endpoints"?

[a-nogikh]

What's the problem with "deep http endpoints"?

If we ever need to render some HTML output at /a/b, then links to other endpoints would have to look like ../c. And that might be a bit more painful to maintain than if we just directly generate each link.

[dvyukov]

What's the problem with "deep http endpoints"?

If we ever need to render some HTML output at /a/b, then links to other endpoints would have to look like ../c. And that might be a bit more painful to maintain than if we just directly generate each link.

I see. So there are no fundamental reasons why it won't work. If it turns out to be a problem, we need a dead-links test (parse main page with x/net/html and follow all links recursively). There is no guarantee we don't have dead links right now.

[pbelskiy]

@dvyukov So you think that using some new option like url-prefix is a bad idea and it would be better to use relative URLs?

I saw many projects which uses such approach with prefix eg - https://www.jenkins.io/doc/book/installing/initial-settings/

PS: I can prepare PR with relate URL too

[dvyukov]

Relative paths don't add code nor user burden. It's equivalent code/complexity that just supports more use cases. While prefix is more code/complexity. One can buy everything with more code, but code is liability not an advantage. So if relative paths work, I would prefer using them.

[pbelskiy]

Anyway I think that using some new option like http-prefix is a good idea because:

1. Introducing multilevel HTTP links in future will break relative addressing.
2. log.Logf(0, "serving http on http://%v", mgr.cfg.HTTP) in manager cannot print valid root url using relative links approach as @dvyukov suggested, because we don't know prefix.
3. I've found few multilevel endpoints in grep output below, should we also change URLs inside it code to relative approach? If yes we cannot because of multilevel links inside.
4. Cannot handle mux.HandleFunc("/", mgr.httpSummary) with relative links, so how to back to main manager page from config page for example?
5. Cannot handle favicon.ico :-)

~/go/src/github.com/google/syzkaller(master) » grep -r 'HandleFunc("' .
./syz-manager/html.go:  mux.HandleFunc("/", mgr.httpSummary)
./syz-manager/html.go:  mux.HandleFunc("/config", mgr.httpConfig)
./syz-manager/html.go:  mux.HandleFunc("/metrics", promhttp.HandlerFor(prometheus.DefaultGatherer, promhttp.HandlerOpts{}).ServeHTTP)
./syz-manager/html.go:  mux.HandleFunc("/syscalls", mgr.httpSyscalls)
./syz-manager/html.go:  mux.HandleFunc("/corpus", mgr.httpCorpus)
./syz-manager/html.go:  mux.HandleFunc("/corpus.db", mgr.httpDownloadCorpus)
./syz-manager/html.go:  mux.HandleFunc("/crash", mgr.httpCrash)
./syz-manager/html.go:  mux.HandleFunc("/cover", mgr.httpCover)
./syz-manager/html.go:  mux.HandleFunc("/subsystemcover", mgr.httpSubsystemCover)
./syz-manager/html.go:  mux.HandleFunc("/modulecover", mgr.httpModuleCover)
./syz-manager/html.go:  mux.HandleFunc("/prio", mgr.httpPrio)
./syz-manager/html.go:  mux.HandleFunc("/file", mgr.httpFile)
./syz-manager/html.go:  mux.HandleFunc("/report", mgr.httpReport)
./syz-manager/html.go:  mux.HandleFunc("/rawcover", mgr.httpRawCover)
./syz-manager/html.go:  mux.HandleFunc("/rawcoverfiles", mgr.httpRawCoverFiles)
./syz-manager/html.go:  mux.HandleFunc("/filterpcs", mgr.httpFilterPCs)
./syz-manager/html.go:  mux.HandleFunc("/funccover", mgr.httpFuncCover)
./syz-manager/html.go:  mux.HandleFunc("/filecover", mgr.httpFileCover)
./syz-manager/html.go:  mux.HandleFunc("/input", mgr.httpInput)
./syz-manager/html.go:  mux.HandleFunc("/favicon.ico", func(w http.ResponseWriter, r *http.Request) {})
./syz-ci/syz-ci.go: http.HandleFunc("/upload_cover", func(w http.ResponseWriter, r *http.Request) {
./dashboard/app/kcidb.go:   http.HandleFunc("/kcidb_poll", handleKcidbPoll)
./dashboard/app/reporting_email.go: http.HandleFunc("/email_poll", handleEmailPoll)
./dashboard/app/reporting_email.go: http.HandleFunc("/_ah/mail/", handleIncomingMail)
./dashboard/app/reporting_email.go: http.HandleFunc("/_ah/bounce", handleEmailBounce)
./dashboard/app/main.go:    http.HandleFunc("/cache_update", cacheUpdate)
./syz-hub/http.go:  http.HandleFunc("/", hub.httpSummary)
./vendor/google.golang.org/appengine/README.md:http.HandleFunc("/_ah/start", func(w http.ResponseWriter, r *http.Request) {
./vendor/github.com/gorilla/handlers/cors.go://      r.HandleFunc("/users", UserEndpoint)
./vendor/github.com/gorilla/handlers/cors.go://      r.HandleFunc("/projects", ProjectEndpoint)
./vendor/github.com/gorilla/handlers/recovery.go://  r.HandleFunc("/", func(w http.ResponseWriter, r *http.Request) {
./vendor/github.com/gorilla/handlers/logging.go://  r.HandleFunc("/", func(w http.ResponseWriter, r *http.Request) {
./vendor/github.com/gorilla/handlers/README.md:    r.HandleFunc("/", ShowIndex)
./vendor/github.com/gorilla/handlers/canonical.go://  r.HandleFunc("/route", YourHandler)

[dvyukov]

Anyway I think that using some new option like http-prefix is a good idea because:

1. Introducing multilevel HTTP links in future will break relative addressing.

What's the problem with multi-level links? They should work with relative paths.

2. log.Logf(0, "serving http on http://%v", mgr.cfg.HTTP) in manager cannot print valid root url using relative links approach as @dvyukov suggested, because we don't know prefix.

This is the correct url. That's where we serve http. It's not changing if we use different way to express links in html pages.

3. I've found few multilevel endpoints in grep output below, should we also change URLs inside it code to relative approach? If yes we cannot because of multilevel links inside.
4. Cannot handle mux.HandleFunc("/", mgr.httpSummary) with relative links, so how to back to main manager page from config page for example?

Do we need to change this if we use relative links in html pages? Why?

5. Cannot handle favicon.ico :-)

Why

[pbelskiy]

What's the problem with multi-level links? They should work with relative paths.

Let's imagine we have syzkaller manager home page, it is on address for example http://test/syzkaller, so we just use nginx reverse proxy with prefix 'syzkaller' for our convenience.

location /syzkaller {
    proxy_pass http://127.0.0.1:56741;
}

When we click on config/verbose relative url on home page, we go to http://test/syzkaller/config/verbose, so how can you generate on this HTML page url to http://test/syzkaller to get homepage back?

Probably you will suggest something like <a href="./"> yes it will be relative url to prevous level, but it's not work for example with trailing slash in url, it will address to the same page as we are currently on. And also it's would be so inconvenient to support correct ur's during add\remove\rename endpoints.

This is the correct url. That's where we serve http. It's not changing if we use different way to express links in html pages.

Yes, this is correct root url without reverse proxy, so we probably can ignore this.

Cannot handle favicon.ico. Why?

Because browsers requests root favicon /favicon.ico, so if we want to put browser correct favicon, we should put special code in HTML which contain absolute address to our favicon, which we cannot put because we don't know absolute address, because we don't know which URL prefix is used behind reverse proxy.

https://stackoverflow.com/questions/9943771/adding-a-favicon-to-a-static-html-page

---

Anyway both approaches means changing code

1) Bad and incomplete relative url approaches will need to introduce new url router like gorilla mux or something custom just like regexp table with handlers which will be ignore any prefix by reverse proxy. 2) Better url-prefix option approach will suppress new url router implementation, and fully cover all the problems we discussed in this topic.

[dvyukov]

When we click on config/verbose relative url on home page, we go to http://test/syzkaller/config/verbose, so how can you generate on this HTML page url to http://test/syzkaller to get homepage back?

Looking at examples here: https://stackoverflow.com/questions/4810927/how-to-go-up-a-level-in-the-src-path-of-a-url-in-html I assume it should be "../..". Won't it work?

Because browsers requests root favicon /favicon.ico

I see. If the favicon.ico doesn't work with reverse proxies, it does not look too critical. We don't have any in syz-manager anyway.

[pbelskiy]

Okay, but anyway in both cases we will add code for url-prefix or new http router with ugly relative urls inside, why do you think that second variant better then?

[dvyukov]

Okay, but anyway in both cases we will add code for url-prefix or new http router

Why do we need to do anything with http router? We will have normal URLs pointing to the same places as now. I don't see that anything changes wrt routing.

with ugly relative urls inside

I don't see how "corpus" is uglier than "/corpus".

why do you think that second variant better then?

No additional parameters to configure, no additional code to maintain, no additional tests needed. So far this looks like a feature that can and should be supported externally. It's like if we would try to bolt FTP support into the unix cp utility because the destination can be an FTP server. There is almost infinite set of features cp could potentially support, but it does not mean all of that code needs to be stuffed into cp itself.

[pbelskiy]

Why do we need to do anything with http router? We will have normal URLs pointing to the same places as now. I don't see that anything changes wrt routing.

Because here https://github.com/google/syzkaller/blob/master/syz-manager/html.go for example mux.HandleFunc("/config", mgr.httpConfig) will not handle address like 'http://test/syzkaller/config' so we will need to rewrite it in some regexp state or using some third party routers with prefix ignoring.

[dvyukov]

But isn't it the job of the reverse proxy to transparently handle path changes? It should add/remove the URL prefix. If it's only adding the prefix w/o removing it, I don't see how it's any useful...

[pbelskiy]

Any updates?