[http_proxy_over_p2p] #5526

cboddy · 2018-09-26T18:46:05Z

This implements an http-proxy over p2p-streams, for context see #5341.

This script is a useful test of the functionality. In case it causes portability issues I've not included it as a sharness test (since it uses python to serve HTTP content, although happy to add it since python be ~ as available as bash).

(inline since GH doesn't support *.sh as an attachment)

#!/bin/bash                                                                                                                                                   
                                                                                                                                                              
#                                                                                                                                                             
# clean up all the things started in this script                                                                                                              
#                                                                                                                                                             
function teardown() {                                                                                                                                         
    jobs -p | xargs kill -9 ;                                                                                                                                 
}                                                                                                                                                             
trap teardown INT EXIT                                                                                                                                        
                                                                                                                                                              
#                                                                                                                                                             
# serve the thing over HTTP                                                                                                                                   
#                                                                                                                                                             
SERVE_PATH=$(mktemp -d)                                                                                                                                       
echo "YOU ARE THE CHAMPION MY FRIEND" > $SERVE_PATH/index.txt                                                                                                 
cd $SERVE_PATH                                                                                                                                                
# serve this on port 8000
python -m SimpleHTTPServer 8000 &


cd -

IPFS=cmd/ipfs/ipfs

PATH1=$(mktemp -d)
PATH2=$(mktemp -d)

RECEIVER_LOG=$PATH1/log.log
SENDER_LOG=$PATH2/log.log

export IPFS_PATH=$PATH1

#
# start RECEIVER IPFS daemon
#
$IPFS init >> $RECEIVER_LOG 2>&1
$IPFS config --json Experimental.Libp2pStreamMounting true >> $RECEIVER_LOG 2>&1
$IPFS config --json Addresses.API "\"/ip4/127.0.0.1/tcp/6001\"" >> $RECEIVER_LOG 2>&1
$IPFS config --json Addresses.Gateway "\"/ip4/127.0.0.1/tcp/8081\"" >> $RECEIVER_LOG 2>&1
$IPFS config --json Addresses.Swarm "[\"/ip4/0.0.0.0/tcp/7001\", \"/ip6/::/tcp/7001\"]" >> $RECEIVER_LOG 2>&1
$IPFS daemon >> $RECEIVER_LOG 2>&1 &
# wait for daemon to start.. maybe?
# ipfs id returns empty string if we don't wait here..
sleep 5

#
# start a p2p listener on RECIVER to the HTTP server with our content
#
$IPFS p2p listen /x/test /ip4/127.0.0.1/tcp/8000 >> $RECEIVER_LOG 2>&1
FIRST_ID=$($IPFS id -f "<id>")

export IPFS_PATH=$PATH2
$IPFS init >> $SENDER_LOG 2>&1
$IPFS config --json Experimental.Libp2pStreamMounting true >> $SENDER_LOG 2>&1
$IPFS daemon >> $SENDER_LOG 2>&1 &
# wait for daemon to start.. maybe?
sleep 5



# send a http request to SENDER via proxy to RECIEVER that will proxy to web-server

echo "******************"
echo proxy response
echo "******************"
curl http://localhost:5001/proxy/http/$FIRST_ID/test/index.txt



echo "******************"
echo link http://localhost:5001/proxy/http/$FIRST_ID/test/index.txt
echo "******************"
echo "RECEIVER IPFS LOG " $RECEIVER_LOG
echo "******************"
cat $RECEIVER_LOG

echo "******************"
echo "SENDER IPFS LOG " $SENDER_LOG
echo "******************"
cat $SENDER_LOG

cboddy · 2018-09-26T18:52:31Z

FYI this link appears to be broken
https://github.com/ipfs/community/blob/master/contributing.md#contribution-guidelines

magik6k · 2018-09-26T18:57:29Z

Thanks! You should be able to add a sharness test which hijacks the request the request using netcat as is done in https://github.com/ipfs/go-ipfs/blob/master/test/sharness/t0235-cli-request.sh#L14.

magik6k

Looks quite good, it would be nice to get some better tests though.

p2p/proxy.go

p2p/proxy_test.go

p2p/proxy.go

cboddy · 2018-09-26T21:37:27Z

@magik6k thanks for the swift feedback; have started and will munge that script into an sharness test as suggested.

Stebalien · 2018-10-01T20:47:21Z

So, I believe this should actually be using the ReverseProxy helper. That will copy and modify all the right headers and handle all the edge-cases. However, that may just make it more complicated so take this as a suggestion. (You won't be able to use the NewSingleHostReverseProxy helper, you'll have to implement a custom RoundTripper.)

I believe we also need to read while writing. That is, the current version expects to write the entire request before it reads any response. We should start the request and then read the response as some worker finishes writing the request.

Note: We can probably fix these later if they turn out to be difficult to get right.

Stebalien · 2018-10-01T20:52:09Z

core/corehttp/proxy.go

+			s := bufio.NewReader(stream)
+			proxyResponse, err := http.ReadResponse(s, proxyReq)
+
+			defer func() { proxyResponse.Body.Close() }()


Do we need this? I believe proxyResponse.Write will close the body when done.

Also, I'm not sure if this is valid if err != nil. Will it not panic?

core/corehttp/proxy.go

lanzafame · 2018-10-02T00:48:37Z

@cboddy @Stebalien this might be a little late in the PR cycle to suggest but, if I understand correctly, @hsanjuan has built go-libp2p-http for this use case. It should simplify the implementation.

hsanjuan · 2018-10-02T12:58:15Z

Hello, yes, https://github.com/hsanjuan/go-libp2p-http will probably simplify this (along with httputil.ReverseProxy). go-libp2p-http provides the libp2p transport (RoundTripper implementation) that you are looking for the ReverseProxy. If I'm correct you just need to initialize it with libp2p://peerid as target URL and manually set the transport. At least, it would not add another http RoundTripper to the libp2p world.

The RoundTrip in go-libp2p-http is simplified if you compare it to all the default transport's implementation (https://golang.org/src/net/http/transport.go?s=3628:10127#L385) but it's well tested and I think it should be solid enough (used by cluster for a while). It supports reading responses while sending the requests (for streaming requests).

The only problem would be to attach a custom transport tag to every Roundtripper. We can easily enable this upstream (currently a single tag is used for all), if you are willing to go down this path.

cboddy · 2018-10-02T13:16:26Z

@Stebalien @lanzafame thanks for the feedback.

Will look into using ReverseProxy as suggested (most of the go stdlib still a bit new to me) and try to push something today/ in the next few days.

Stebalien

Slightly more complicated but I think this'll have better header handling.

Stebalien · 2018-10-02T21:42:23Z

core/corehttp/proxy.go

+			}
+
+			// open connect to peer
+			stream, err := ipfsNode.P2P.PeerHost.NewStream(ipfsNode.Context(), parsedRequest.target, protocol.ID("/x/"+parsedRequest.name))


I believe this should be using the request context.

Stebalien · 2018-10-02T21:45:48Z

core/corehttp/proxy.go

+	sendRequest := func() {
+		err := req.Write(*rt.stream)
+		if err != nil {
+			(*(rt.stream)).Close()


This should probably be rt.stream.Reset().

Stebalien · 2018-10-02T21:46:14Z

core/corehttp/proxy.go

+}
+
+type roundTripper struct {
+	stream *inet.Stream


No real need for the pointer. This is already an interface (class-like thing) so we can just say inet.Stream.

hsanjuan · 2018-10-02T22:01:27Z

fyi, protocol tags per Roundtripper coming about: libp2p/go-libp2p-http#12

Stebalien · 2018-10-02T22:01:51Z

One more thing we'll need to do before merging this: put it behind an experimental feature-gate. Take a look at how the ipfs p2p command does this (using config.Experimental.Libp2pStreamMounting).

I don't really see this needing a long stabilization period, but we should probably leave it experimental at last until we stabilize the ipfs p2p feature...

ianopolous · 2018-10-02T23:00:16Z

I've tested out all our api calls for Peergos through this, which includes gets, posts, and multiparts. And everything works!

hsanjuan

This is a re-implementation of the logic of go-libp2p-http but it doesn't close the streams. And it if did, it probably would miss handling connection closing correctly (like this: https://github.com/hsanjuan/go-libp2p-gostream/blob/master/conn.go#L37).

This is all handled in go-libp2p-http because we have already hit a number of edge cases. I really don't like that we are cloning the logic here.

hsanjuan · 2018-10-03T08:16:16Z

core/corehttp/proxy.go

+				return
+			}
+			//send proxy request and response to client
+			newReverseHTTPProxy(parsedRequest, &stream).ServeHTTP(w, request)


Something is fishy here. For every request a new proxy is created but I think the stream is never closed ?

Good catch. That's fixed now.

ianopolous · 2018-10-03T18:28:47Z

@hsanjuan We can't use the RoundTripper in go-libp2p-http because it assumes a different incoming request format, and we don't want to do an extra socket+http round trip. We used your technique to close the stream though.

ianopolous · 2018-10-03T20:45:47Z

@Stebalien Does that mean we need a PR to go-ipfs-config to add a new config option and then gx publish that before this can be merged, or can we use the existing Libp2pStreamMounting option?

hsanjuan · 2018-10-03T21:19:29Z

@hsanjuan We can't use the RoundTripper in go-libp2p-http because it assumes a different incoming request format, and we don't want to do an extra socket+http round trip. We used your technique to close the stream though.

That's not true. You just need to rewrite the path, which you are already doing anyway.

We don't need to do this for every test and our tests are slow enough. License: MIT Signed-off-by: Steven Allen <steven@stebalien.com>

License: MIT Signed-off-by: Ian Preston <ianopolous@protonmail.com>

License: MIT Signed-off-by: Steven Allen <steven@stebalien.com>

License: MIT Signed-off-by: Ian Preston <ianopolous@protonmail.com>

License: MIT Signed-off-by: Steven Allen <steven@stebalien.com>

License: MIT Signed-off-by: Ian Preston <ianopolous@protonmail.com> Co-Authored-By: ianopolous <ianopolous@protonmail.com>

License: MIT Signed-off-by: Ian Preston <ianopolous@protonmail.com>

License: MIT Signed-off-by: Ian Preston <ianopolous@protonmail.com> Co-Authored-By: ianopolous <ianopolous@protonmail.com>

License: MIT Signed-off-by: Ian Preston <ianopolous@protonmail.com>

Instead of repeatedly starting the netcat server, start it once and wait for it to fully start. Then, feed responses in using a fifo. License: MIT Signed-off-by: Steven Allen <steven@stebalien.com>

Stebalien · 2018-11-30T04:15:24Z

OK, tests pass and @hsanjuan has given a ✔️ 🚂 (this train may be a bit slow...).

ianopolous · 2018-11-30T09:48:47Z

Hooray! The last critical feature we need for Peergos!

@Stebalien is the plan to enable this on ipfs.io?

cboddy · 2018-12-02T10:18:41Z

@Stebalien @magik6k @hsanjuan thanks for your help with this one!

Stebalien · 2018-12-03T18:46:56Z

@Stebalien is the plan to enable this on ipfs.io?

Probably not for a while, if ever. We'll have to seriously consider the security and performance implications and, really, the public gateway's more of a crutch (we'd prefer if users would just load js-libp2p from their DAPP).

ianopolous · 2018-12-03T20:36:46Z

Probably not for a while, if ever. We'll have to seriously consider the security and performance implications and, really, the public gateway's more of a crutch (we'd prefer if users would just load js-libp2p from their DAPP).

:-( That's a shame, because can do some really cool demos if that is enabled. Loading js-libp2p will never be an option for us for security reasons.

Stebalien · 2018-12-03T21:05:59Z

Are you forbidding javascript entirely? Can you not spin up a js-ipfs node in a sandboxed worker?

I do agree this would allow for some pretty cool demos but I'd like to sit on it at least until it's stabilized.

ianopolous · 2018-12-03T21:21:31Z

@Stebalien In general, we veto anything that uses npm which is anathema to basic security practices. We're also trying to keep the JS down to a small easily auditable amount (as much as possible) for the same reasons. As well as being much harder to audit for attacks, having a complex P2P daemon running in the same process as that which has the secret keys is also asking for trouble. Browsers explicitly don't defend against spectre attacks within the same page to my knowledge (they'd have to run every worker in an independent OS process, not just different OS threads). We also have reproducible builds for the web-ui already.

For now we can just preface all ipfs-only demos with, install ipfs and enable these two flags, then browse to this local url.

Stebalien · 2018-12-03T22:35:03Z

In general, we veto anything that uses npm which is anathema to basic security practices.

Well, I can't really blame you there...

Browsers explicitly don't defend against spectre attacks within the same page to my knowledge

Sandboxing has more to do with origins/contexts than pages. I'm not familiar with browser spectre mitigations but, a page in a sandboxed iframe/worker should have equivalent security to a page running in a separate tab.

ianopolous · 2018-12-03T22:57:30Z

Sandboxing has more to do with origins/contexts than pages. I'm not familiar with browser spectre mitigations but, a page in a sandboxed iframe/worker should have equivalent security to a page running in a separate tab.

If the p2p daemon was running from a separate domain in an iframe, I believe this might be safe in some browsers (depending on their site isolation properties), but it's much harder to prove it safe, and it requires you to have two separate origins. We don't have any 3rd party hosted content in our web-ui - it's all self hosted (partially to enable easy offline and localhost access).

How about we continue this on irc to stop spamming all the other contributors to this pr? :-)

cboddy requested a review from Kubuxu as a code owner September 26, 2018 18:46

ianopolous requested a review from Stebalien September 26, 2018 18:46

cboddy force-pushed the feat/http_proxy_over_p2p branch 2 times, most recently from 2cb7199 to f984368 Compare September 26, 2018 18:55

magik6k self-requested a review September 26, 2018 18:57

magik6k reviewed Sep 26, 2018

View reviewed changes

p2p/proxy.go Outdated Show resolved Hide resolved

p2p/proxy.go Outdated Show resolved Hide resolved

p2p/proxy_test.go Outdated Show resolved Hide resolved

p2p/proxy.go Outdated Show resolved Hide resolved

cboddy force-pushed the feat/http_proxy_over_p2p branch from 3dbdeb4 to 2c91cb6 Compare September 29, 2018 16:35

Stebalien reviewed Oct 1, 2018

View reviewed changes

ghost assigned ianopolous Oct 1, 2018

ghost added the status/in-progress In progress label Oct 1, 2018

ianopolous force-pushed the feat/http_proxy_over_p2p branch 2 times, most recently from 60092b8 to 5d20453 Compare October 2, 2018 00:14

cboddy force-pushed the feat/http_proxy_over_p2p branch from 9f6e044 to 4400212 Compare October 2, 2018 18:41

Stebalien reviewed Oct 2, 2018

View reviewed changes

hsanjuan requested changes Oct 3, 2018

View reviewed changes

ianopolous force-pushed the feat/http_proxy_over_p2p branch 2 times, most recently from 199d25e to 2fd1c26 Compare October 3, 2018 20:24

Stebalien and others added 15 commits November 28, 2018 23:17

setup and teardown nodes only once

83369b5

We don't need to do this for every test and our tests are slow enough. License: MIT Signed-off-by: Steven Allen <steven@stebalien.com>

Don't url decode protocol name. It won't work.

13b0483

License: MIT Signed-off-by: Ian Preston <ianopolous@protonmail.com>

fix tests for iptb update

2865275

License: MIT Signed-off-by: Steven Allen <steven@stebalien.com>

move p2p http proxy from api to gateway

78c43fe

License: MIT Signed-off-by: Ian Preston <ianopolous@protonmail.com>

switch to new path format in p2p http proxy

fd43f47

License: MIT Signed-off-by: Ian Preston <ianopolous@protonmail.com>

fix tests

47d45c7

License: MIT Signed-off-by: Ian Preston <ianopolous@protonmail.com>

update p2p http proxy docs

19d4d66

License: MIT Signed-off-by: Ian Preston <ianopolous@protonmail.com>

update tests

d8cab79

License: MIT Signed-off-by: Steven Allen <steven@stebalien.com>

add some additional tests for custom protocols

fe8ffde

License: MIT Signed-off-by: Steven Allen <steven@stebalien.com>

uses the global PeerHost and don't expose the P2P one

b720d1f

License: MIT Signed-off-by: Steven Allen <steven@stebalien.com>

p2p http proxy: go fmt

bf18437

License: MIT Signed-off-by: Steven Allen <steven@stebalien.com>

Apply suggestions from code review

3b2ce4a

License: MIT Signed-off-by: Ian Preston <ianopolous@protonmail.com> Co-Authored-By: ianopolous <ianopolous@protonmail.com>

add more p2p http proxy tests

9956630

License: MIT Signed-off-by: Ian Preston <ianopolous@protonmail.com>

Update test/sharness/t0184-http-proxy-over-p2p.sh

7e54107

License: MIT Signed-off-by: Ian Preston <ianopolous@protonmail.com> Co-Authored-By: ianopolous <ianopolous@protonmail.com>

Add another test case for invalid p2p http proxy path

8694984

License: MIT Signed-off-by: Ian Preston <ianopolous@protonmail.com>

Stebalien force-pushed the feat/http_proxy_over_p2p branch from 181aa64 to 6c35fbc Compare November 29, 2018 07:17

p2p proxy tests: make robust against timing

9a443ad

Instead of repeatedly starting the netcat server, start it once and wait for it to fully start. Then, feed responses in using a fifo. License: MIT Signed-off-by: Steven Allen <steven@stebalien.com>

Stebalien force-pushed the feat/http_proxy_over_p2p branch from 6c35fbc to 9a443ad Compare November 30, 2018 02:01

Stebalien merged commit 2d94a3f into ipfs:master Nov 30, 2018

ghost removed the status/in-progress In progress label Nov 30, 2018

ianopolous mentioned this pull request Nov 30, 2018

Feature: Transparent HTTP proxy over p2p streams #5341

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[http_proxy_over_p2p] #5526

[http_proxy_over_p2p] #5526

cboddy commented Sep 26, 2018

cboddy commented Sep 26, 2018

magik6k commented Sep 26, 2018

magik6k left a comment

cboddy commented Sep 26, 2018 •

edited

Loading

Stebalien commented Oct 1, 2018 •

edited

Loading

Stebalien Oct 1, 2018

lanzafame commented Oct 2, 2018 •

edited

Loading

hsanjuan commented Oct 2, 2018

cboddy commented Oct 2, 2018

Stebalien left a comment

Stebalien Oct 2, 2018

ianopolous Oct 3, 2018

Stebalien Oct 2, 2018

Stebalien Oct 2, 2018

hsanjuan commented Oct 2, 2018

Stebalien commented Oct 2, 2018

ianopolous commented Oct 2, 2018

hsanjuan left a comment

hsanjuan Oct 3, 2018

ianopolous Oct 3, 2018

ianopolous commented Oct 3, 2018

ianopolous commented Oct 3, 2018

hsanjuan commented Oct 3, 2018

Stebalien commented Nov 30, 2018

ianopolous commented Nov 30, 2018

cboddy commented Dec 2, 2018

Stebalien commented Dec 3, 2018

ianopolous commented Dec 3, 2018

Stebalien commented Dec 3, 2018 •

edited

Loading

ianopolous commented Dec 3, 2018

Stebalien commented Dec 3, 2018

ianopolous commented Dec 3, 2018

[http_proxy_over_p2p] #5526

[http_proxy_over_p2p] #5526

Conversation

cboddy commented Sep 26, 2018

cboddy commented Sep 26, 2018

magik6k commented Sep 26, 2018

magik6k left a comment

Choose a reason for hiding this comment

cboddy commented Sep 26, 2018 • edited Loading

Stebalien commented Oct 1, 2018 • edited Loading

Stebalien Oct 1, 2018

Choose a reason for hiding this comment

lanzafame commented Oct 2, 2018 • edited Loading

hsanjuan commented Oct 2, 2018

cboddy commented Oct 2, 2018

Stebalien left a comment

Choose a reason for hiding this comment

Stebalien Oct 2, 2018

Choose a reason for hiding this comment

ianopolous Oct 3, 2018

Choose a reason for hiding this comment

Stebalien Oct 2, 2018

Choose a reason for hiding this comment

Stebalien Oct 2, 2018

Choose a reason for hiding this comment

hsanjuan commented Oct 2, 2018

Stebalien commented Oct 2, 2018

ianopolous commented Oct 2, 2018

hsanjuan left a comment

Choose a reason for hiding this comment

hsanjuan Oct 3, 2018

Choose a reason for hiding this comment

ianopolous Oct 3, 2018

Choose a reason for hiding this comment

ianopolous commented Oct 3, 2018

ianopolous commented Oct 3, 2018

hsanjuan commented Oct 3, 2018

Stebalien commented Nov 30, 2018

ianopolous commented Nov 30, 2018

cboddy commented Dec 2, 2018

Stebalien commented Dec 3, 2018

ianopolous commented Dec 3, 2018

Stebalien commented Dec 3, 2018 • edited Loading

ianopolous commented Dec 3, 2018

Stebalien commented Dec 3, 2018

ianopolous commented Dec 3, 2018

cboddy commented Sep 26, 2018 •

edited

Loading

Stebalien commented Oct 1, 2018 •

edited

Loading

lanzafame commented Oct 2, 2018 •

edited

Loading

Stebalien commented Dec 3, 2018 •

edited

Loading