Add SCION HTTP Proxy to shttp lib

[martenwallewein]

Hi @ALL,

I've worked on setting up DVB-T video streaming over SCION and implemented a proxy component that can proxy requests from HTTP/1.1 to HTTP/3 over SCION and vice versa. We needed the proxy

1. To make the stream over SCION accessible by the vlc media player
2. To provide the DVB-T stream grabbed from the receiver via HTTP/3 oiver SCION

To better understant the setup, please consider the following example:

Considering the figure, the SCION HTTP Ingress Proxy was realised using the "toScion" direction and the Egress proxy using the "fromScion" direction as can be seen from the code.

It would be cool to have this implementation upstream, because we plan to integrate a demo into SCION apps that can play video streams using HTTP/3 over SCION considering a similar example as shown above, but play the stream in the browser instead of using vlc media player.

Please feel free to give me feedback on this PR :)

Best,
Marten

---

This change is 

[matzf]

Hi Marten, thanks a lot for your contribution!

I would like to propose an alternative approach to implement this. Have a look at httplib.ReverseProxy or more specifically httplib.NewSingleHostReverseProxy. This implements a proxy as a handler for a normal http server. This is really nice, because it allows an application to compose this, e.g. adding the setconfig handlers that you now added in the library.
So my proposal would be: add a function e.g. NewSingleSCIONHostReverseProxy to shttp that creates a ReverseProxy with the Transport configured with a the SCION/QUIC round tripper.
With this, you can then easily create a proxy application that covers all four cases of local and remote SCION or IP combinations:

mux := http.NewServeMux()
if isScionAddress(remote) {
   mux.Handle("/", shttp.NewSingleSCIONHostReverseProxy(remote))
} else {
   mux.Handle("/", http.NewSingleHostReverseProxy(remote))
}

if isScionAddress(local) { 
   shttp.ListenAndServe(mux,...)
} else {
   http.ListenAndServe(mux,...)
}

Reviewed 4 of 5 files at r1.
Reviewable status: 4 of 5 files reviewed, 1 unresolved discussion (waiting on @martenwallewein)

---

pkg/shttp/proxy.go, line 100 at r1 (raw file):

	log.Println("Perform config update")
	s.config = &newConfig

This seems unsafe; requests can probably be concurrent so updates to this config would have to be guarded against concurrent read (and writes).

[martenwallewein]

Hi Matthias,

thanks for the feedback, I think is a more cleaner way to implement this and furthermore, the setConfig part can be moved outside of the lib itself. I'll give it a try!

[martenwallewein]

I have three more questions, maybe you can help me with them:

• is there an existing isScionAddress function? If not, could something like parseUdpAddress from https://github.com/scionproto/scion/blob/master/go/lib/snet/udpaddr.go help?
• I would add *tls.Config parameters to shttp.ListenAndServe and shttp.NewSingleSCIONHostReverseProxy so custom tls configuration/certs can be used, is it okay to add those parameters (especially for the listenAndServe method, I don't want to break exisiting implementations)
• Should I squash my commits in the end?

[matzf]

• is there an existing isScionAddress function? If not, could something like parseUdpAddress from https://github.com/scionproto/scion/blob/master/go/lib/snet/udpaddr.go help?

I don't think there is a public function like that, but you can use snet.ParseUDPAddr and check for errors, as you suggested.

• I would add *tls.Config parameters to shttp.ListenAndServe and shttp.NewSingleSCIONHostReverseProxy so custom tls configuration/certs can be used, is it okay to add those parameters

Sure, that sounds good to me.

• Should I squash my commits in the end?

You can if you like, but don't worry about it. We squash merge all pull requests anyway, so whatever history you have will be fine.

[matzf]

Reviewed 3 of 3 files at r2.
Reviewable status: all files reviewed, 4 unresolved discussions (waiting on @martenwallewein)

---

_examples/shttp/proxy/main.go, line 27 at r2 (raw file):

	// which we can use to proxy to SCION
	if _, err := snet.ParseUDPAddr(*remote); err == nil {
		mux.Handle("/", shttp.NewSingleSCIONHostReverseProxy(*remote, nil))

I think you will want to pass &tls.Config{InsecureSkipVerify: true} here as this is no longer added by default (this has just changed on master).

---

pkg/shttp/proxy.go, line 19 at r2 (raw file):

	}

	handler := func(wr http.ResponseWriter, r *http.Request) {

This implements the proxy-ing manually, and I'm guess there are many subtle ways to do this wrong. Is there a reason to not use the ReverseProxy handler from httputil, e.g. like so?

func NewSingleSCIONHostReverseProxy(remote string, cliTLSCfg *tls.Config) *httputil.ReverseProxy {
   targetURL := url.Parse(MangleSCIONURL(remote))
   proxy := httputil.NewSingleHostReverseProxy(targetURL);
   proxy.Transport = NewRoundTripper(cliTLSCfg, nil)
   return proxy
}

---

pkg/shttp/README.md, line 71 at r2 (raw file):

The proxy can handle two directions: From HTTP/1.1 to SCION and from SCION to HTTP/1.1. It's idea is to make resources provided over HTTP accessible over the SCION network. 

To use the proxy, consider the proxy example in _examples. This implementation detects from the format of the `remote` and `local` argument if it should listen on SCION/HTTP/1.1 and proxy to SCION/HTTP/1.1.

Unimportant, but quic-go actually implements http/3.

---

pkg/shttp/README.md, line 73 at r2 (raw file):

To use the proxy, consider the proxy example in _examples. This implementation detects from the format of the `remote` and `local` argument if it should listen on SCION/HTTP/1.1 and proxy to SCION/HTTP/1.1.

```Go

Don't replicate the entire example here. You can add a link to the go file, like so: [_examples/shttp/proxy](../../_examples/shttp/proxy/main.go) (not completely sure about the relative path though)

[martenwallewein]

Hi Matthias, sorry for the late response. I added your feedback, compared to the initial implementation this one is now a lot smarter, thanks!

[matzf]

No worries, thank you for your contribution. Just one small comment left.

Reviewed 3 of 3 files at r3.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @martenwallewein)

---

_examples/shttp/proxy/main.go, line 27 at r2 (raw file):

Previously, matzf (Matthias Frei) wrote…

I think you will want to pass &tls.Config{InsecureSkipVerify: true} here as this is no longer added by default (this has just changed on master).

Done.

---

pkg/shttp/proxy.go, line 19 at r2 (raw file):

Previously, matzf (Matthias Frei) wrote…

This implements the proxy-ing manually, and I'm guess there are many subtle ways to do this wrong. Is there a reason to not use the ReverseProxy handler from httputil, e.g. like so?

func NewSingleSCIONHostReverseProxy(remote string, cliTLSCfg *tls.Config) *httputil.ReverseProxy {
   targetURL := url.Parse(MangleSCIONURL(remote))
   proxy := httputil.NewSingleHostReverseProxy(targetURL);
   proxy.Transport = NewRoundTripper(cliTLSCfg, nil)
   return proxy
}

Done.

---

pkg/shttp/proxy.go, line 18 at r3 (raw file):

	targetURL, err := url.Parse(sUrl)
	if err != nil {
		log.Fatalf("Failed to parse SCION URL %s", err)

Just return the error so the application code can decide how to continue.

---

pkg/shttp/README.md, line 73 at r2 (raw file):

Previously, matzf (Matthias Frei) wrote…

Don't replicate the entire example here. You can add a link to the go file, like so: [_examples/shttp/proxy](../../_examples/shttp/proxy/main.go) (not completely sure about the relative path though)

Done.

[matzf]

Reviewed 2 of 2 files at r4.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @martenwallewein)

---

pkg/shttp/proxy.go, line 18 at r3 (raw file):

Previously, matzf (Matthias Frei) wrote…

Just return the error so the application code can decide how to continue.

Done.

[matzf]

Thanks @martenwallewein. If you could quickly update your branch, we'll get this merged

[martenwallewein]

Thanks, I updated the branch.

[matzf]

Reviewed 1 of 1 files at r5, 2 of 2 files at r6.
Reviewable status: complete! all files reviewed, all discussions resolved