gateway: clean up its surface, and remove BlockList
This patch is in preparation for the gateway's extraction. It's interesting to trace technical debt back to its origin, understanding the circumstances in which it was introduced and built up, and then cutting it back at exactly the right places. - Clean up the gateway's surface The option builder GatewayOption() now takes only arguments which are relevant for HTTP handler muxing, i.e. the paths where the gateway should be mounted. All other configuration happens through the GatewayConfig object. - Remove BlockList I know why this was introduced in the first place, but it never ended up fulfilling that purpose. Somehow it was only ever used by the API server, not the gateway, which really doesn't make sense. It was also never wired up with CLI nor fs-repo. Eventually @krl started punching holes into it to make the Web UI accessible. - Remove --unrestricted-api This was holes being punched into BlockList too, for accessing /ipfs and /ipn on the API server. With BlockList removed and /ipfs and /ipns freely accessible, putting this option out of action is safe. With the next major release, the option can be removed for good. License: MIT Signed-off-by: Lars Gierth <larsg@systemli.org>
Lars Gierth committed
Jun 17, 2016 at 16:33 UTC
1afebc21f324982141ca8a29710da0d6f83ca804
9 files changed
+37
-119
cmd/ipfs/daemon.go
+5
-28
@@ -8,7 +8,6 @@ import (
8
_ "net/http/pprof"
9
"os"
10
"sort"
11
- "strings"
11
"sync"
12
13
"gx/ipfs/QmPpRcbNUXauP3zWZ1NJMLWpe4QnmEHrd2ba2D3yqWznw7/go-multiaddr-net"
@@ -135,7 +134,7 @@ Headers.
134
cmds.BoolOption(writableKwd, "Enable writing objects (with POST, PUT and DELETE)").Default(false),
135
cmds.StringOption(ipfsMountKwd, "Path to the mountpoint for IPFS (if using --mount). Defaults to config setting."),
136
cmds.StringOption(ipnsMountKwd, "Path to the mountpoint for IPNS (if using --mount). Defaults to config setting."),
138
- cmds.BoolOption(unrestrictedApiAccessKwd, "Allow API access to unlisted hashes").Default(false),
137
+ cmds.BoolOption(unrestrictedApiAccessKwd, "This option has no effect since v0.4.3").Default(false),
138
cmds.BoolOption(unencryptTransportKwd, "Disable transport encryption (for debugging protocols)").Default(false),
139
cmds.BoolOption(enableGCKwd, "Enable automatic periodic repo garbage collection").Default(false),
140
cmds.BoolOption(adjustFDLimitKwd, "Check and raise file descriptor limits if needed").Default(false),
@@ -364,33 +363,11 @@ func serveHTTPApi(req cmds.Request) (error, <-chan error) {
363
apiMaddr = apiLis.Multiaddr()
364
fmt.Printf("API server listening on %s\n", apiMaddr)
365
367
- unrestricted, _, err := req.Option(unrestrictedApiAccessKwd).Bool()
368
- if err != nil {
369
- return fmt.Errorf("serveHTTPApi: Option(%s) failed: %s", unrestrictedApiAccessKwd, err), nil
370
- }
371
-
372
- apiGw := corehttp.NewGateway(corehttp.GatewayConfig{
373
- Writable: true,
374
- BlockList: &corehttp.BlockList{
375
- Decider: func(s string) bool {
376
- if unrestricted {
377
- return true
378
- }
379
- // for now, only allow paths in the WebUI path
380
- for _, webuipath := range corehttp.WebUIPaths {
381
- if strings.HasPrefix(s, webuipath) {
382
- return true
383
- }
384
- }
385
- return false
386
- },
387
- },
388
- })
366
var opts = []corehttp.ServeOption{
367
corehttp.MetricsCollectionOption("api"),
368
corehttp.CommandsOption(*req.InvocContext()),
369
corehttp.WebUIOption,
393
- apiGw.ServeOption(),
370
+ corehttp.GatewayOption("/ipfs", "/ipns"),
371
corehttp.VersionOption(),
372
defaultMux("/debug/vars"),
373
defaultMux("/debug/pprof/"),
@@ -452,8 +429,8 @@ func serveHTTPGateway(req cmds.Request) (error, <-chan error) {
429
if err != nil {
430
return fmt.Errorf("serveHTTPGateway: req.Option(%s) failed: %s", writableKwd, err), nil
431
}
455
- if !writableOptionFound {
456
- writable = cfg.Gateway.Writable
432
+ if writableOptionFound {
433
+ cfg.Gateway.Writable = writable
434
}
435
436
gwLis, err := manet.Listen(gatewayMaddr)
@@ -474,7 +451,7 @@ func serveHTTPGateway(req cmds.Request) (error, <-chan error) {
451
corehttp.CommandsROOption(*req.InvocContext()),
452
corehttp.VersionOption(),
453
corehttp.IPNSHostnameOption(),
477
- corehttp.GatewayOption(writable, cfg.Gateway.PathPrefixes),
454
+ corehttp.GatewayOption("/ipfs", "/ipns"),
455
}
456
457
if len(cfg.Gateway.RootRedirect) > 0 {
cmd/ipfswatch/main.go
+7
-1
@@ -81,10 +81,16 @@ func run(ipfsPath, watchPath string) error {
81
}
82
defer node.Close()
83
84
+ cfg, err := node.Repo.Config()
85
+ if err != nil {
86
+ return err
87
+ }
88
+ cfg.Gateway.Writable = true
89
+
90
if *http {
91
addr := "/ip4/127.0.0.1/tcp/5001"
92
var opts = []corehttp.ServeOption{
87
- corehttp.GatewayOption(true, nil),
93
+ corehttp.GatewayOption("/ipfs", "/ipns"),
94
corehttp.WebUIOption,
95
corehttp.CommandsOption(cmdCtx(node, ipfsPath)),
96
}
core/corehttp/gateway.go
+8
-60
@@ -4,60 +4,38 @@ import (
4
"fmt"
5
"net"
6
"net/http"
7
- "sync"
7
8
core "github.com/ipfs/go-ipfs/core"
9
config "github.com/ipfs/go-ipfs/repo/config"
10
id "gx/ipfs/QmdBpVuSYuTGDA8Kn66CbKvEThXqKUh2nTANZEhzSxqrmJ/go-libp2p/p2p/protocol/identify"
11
)
12
14
-// Gateway should be instantiated using NewGateway
15
-type Gateway struct {
16
- Config GatewayConfig
17
-}
18
-
13
type GatewayConfig struct {
14
Headers map[string][]string
21
- BlockList *BlockList
15
Writable bool
16
PathPrefixes []string
17
}
18
26
-func NewGateway(conf GatewayConfig) *Gateway {
27
- return &Gateway{
28
- Config: conf,
29
- }
30
-}
31
-
32
-func (g *Gateway) ServeOption() ServeOption {
19
+func GatewayOption(paths ...string) ServeOption {
20
return func(n *core.IpfsNode, _ net.Listener, mux *http.ServeMux) (*http.ServeMux, error) {
34
- // pass user's HTTP headers
21
cfg, err := n.Repo.Config()
22
if err != nil {
23
return nil, err
24
}
25
40
- g.Config.Headers = cfg.Gateway.HTTPHeaders
26
+ gateway := newGatewayHandler(n, GatewayConfig{
27
+ Headers: cfg.Gateway.HTTPHeaders,
28
+ Writable: cfg.Gateway.Writable,
29
+ PathPrefixes: cfg.Gateway.PathPrefixes,
30
+ })
31
42
- gateway, err := newGatewayHandler(n, g.Config)
43
- if err != nil {
44
- return nil, err
32
+ for _, p := range paths {
33
+ mux.Handle(p+"/", gateway)
34
}
46
- mux.Handle("/ipfs/", gateway)
47
- mux.Handle("/ipns/", gateway)
35
return mux, nil
36
}
37
}
38
52
-func GatewayOption(writable bool, prefixes []string) ServeOption {
53
- g := NewGateway(GatewayConfig{
54
- Writable: writable,
55
- BlockList: &BlockList{},
56
- PathPrefixes: prefixes,
57
- })
58
- return g.ServeOption()
59
-}
60
-
39
func VersionOption() ServeOption {
40
return func(n *core.IpfsNode, _ net.Listener, mux *http.ServeMux) (*http.ServeMux, error) {
41
mux.HandleFunc("/version", func(w http.ResponseWriter, r *http.Request) {
@@ -68,33 +46,3 @@ func VersionOption() ServeOption {
46
return mux, nil
47
}
48
}
71
-
72
-// Decider decides whether to Allow string
73
-type Decider func(string) bool
74
-
75
-type BlockList struct {
76
- mu sync.RWMutex
77
- Decider Decider
78
-}
79
-
80
-func (b *BlockList) ShouldAllow(s string) bool {
81
- b.mu.RLock()
82
- d := b.Decider
83
- b.mu.RUnlock()
84
- if d == nil {
85
- return true
86
- }
87
- return d(s)
88
-}
89
-
90
-// SetDecider atomically swaps the blocklist's decider. This method is
91
-// thread-safe.
92
-func (b *BlockList) SetDecider(d Decider) {
93
- b.mu.Lock()
94
- b.Decider = d
95
- b.mu.Unlock()
96
-}
97
-
98
-func (b *BlockList) ShouldBlock(s string) bool {
99
- return !b.ShouldAllow(s)
100
-}
core/corehttp/gateway_handler.go
+2
-8
@@ -36,12 +36,12 @@ type gatewayHandler struct {
36
config GatewayConfig
37
}
38
39
-func newGatewayHandler(node *core.IpfsNode, conf GatewayConfig) (*gatewayHandler, error) {
39
+func newGatewayHandler(node *core.IpfsNode, conf GatewayConfig) *gatewayHandler {
40
i := &gatewayHandler{
41
node: node,
42
config: conf,
43
}
44
- return i, nil
44
+ return i
45
}
46
47
// TODO(cryptix): find these helpers somewhere else
@@ -152,12 +152,6 @@ func (i *gatewayHandler) getOrHeadHandler(w http.ResponseWriter, r *http.Request
152
ipnsHostname = true
153
}
154
155
- if i.config.BlockList != nil && i.config.BlockList.ShouldBlock(urlPath) {
156
- w.WriteHeader(http.StatusForbidden)
157
- w.Write([]byte("403 - Forbidden"))
158
- return
159
- }
160
-
155
nd, err := core.Resolve(ctx, i.node, path.Path(urlPath))
156
// If node is in offline mode the error code and message should be different
157
if err == core.ErrNoNamesys && !i.node.OnlineMode() {
core/corehttp/gateway_test.go
+7
-1
@@ -89,6 +89,12 @@ func newTestServerAndNode(t *testing.T, ns mockNamesys) (*httptest.Server, *core
89
t.Fatal(err)
90
}
91
92
+ cfg, err := n.Repo.Config()
93
+ if err != nil {
94
+ t.Fatal(err)
95
+ }
96
+ cfg.Gateway.PathPrefixes = []string{"/good-prefix"}
97
+
98
// need this variable here since we need to construct handler with
99
// listener, and server with handler. yay cycles.
100
dh := &delegatedHandler{}
@@ -98,7 +104,7 @@ func newTestServerAndNode(t *testing.T, ns mockNamesys) (*httptest.Server, *core
104
ts.Listener,
105
VersionOption(),
106
IPNSHostnameOption(),
101
- GatewayOption(false, []string{"/good-prefix"}),
107
+ GatewayOption("/ipfs", "/ipns"),
108
)
109
if err != nil {
110
t.Fatal(err)
misc/completion/ipfs-completion.bash
+6
-6
@@ -104,7 +104,7 @@ _ipfs_config_show()
104
_ipfs_daemon()
105
{
106
_ipfs_comp "--init --routing= --mount --writable --mount-ipfs= \
107
- --mount-ipns= --unrestricted-api --disable-transport-encryption \
107
+ --mount-ipns= --disable-transport-encryption \
108
--help"
109
}
110
@@ -314,7 +314,7 @@ _ipfs_resolve()
314
315
_ipfs_stats()
316
{
317
- _ipfs_comp "bw --help"
317
+ _ipfs_comp "bw --help"
318
}
319
320
_ipfs_stats_bw()
@@ -401,17 +401,17 @@ _ipfs()
401
{
402
COMPREPLY=()
403
local word="${COMP_WORDS[COMP_CWORD]}"
404
-
404
+
405
case "${COMP_CWORD}" in
406
- 1)
406
+ 1)
407
local opts="add bitswap block bootstrap cat commands config daemon dht \
408
diag dns file get id init log ls mount name object pin ping \
409
refs repo stats swarm tour update version"
410
COMPREPLY=( $(compgen -W "${opts}" -- ${word}) );;
411
- 2)
411
+ 2)
412
local command="${COMP_WORDS[1]}"
413
eval "_ipfs_$command" 2> /dev/null ;;
414
- *)
414
+ *)
415
local command="${COMP_WORDS[1]}"
416
local subcommand="${COMP_WORDS[2]}"
417
eval "_ipfs_${command}_${subcommand}" 2> /dev/null && return
test/sharness/t0061-daemon-opts.sh
+1
-10
@@ -11,20 +11,11 @@ test_description="Test daemon command"
11
12
test_init_ipfs
13
14
-test_launch_ipfs_daemon --unrestricted-api --disable-transport-encryption
14
+test_launch_ipfs_daemon --disable-transport-encryption
15
16
gwyaddr=$GWAY_ADDR
17
apiaddr=$API_ADDR
18
19
-test_expect_success 'api gateway should be unrestricted' '
20
- echo "hello mars :$gwyaddr :$apiaddr" >expected &&
21
- HASH=$(ipfs add -q expected) &&
22
- curl -sfo actual1 "http://$gwyaddr/ipfs/$HASH" &&
23
- curl -sfo actual2 "http://$apiaddr/ipfs/$HASH" &&
24
- test_cmp expected actual1 &&
25
- test_cmp expected actual2
26
-'
27
-
19
# Odd. this fails here, but the inverse works on t0060-daemon.
20
test_expect_success 'transport should be unencrypted' '
21
nc -w 1 localhost $SWARM_PORT > swarmnc < ../t0060-data/mss-ls &&
test/sharness/t0110-gateway.sh
-4
@@ -32,10 +32,6 @@ test_expect_success "GET IPFS path output looks good" '
32
rm actual
33
'
34
35
-test_expect_success "GET IPFS path on API forbidden" '
36
- test_curl_resp_http_code "http://127.0.0.1:$apiport/ipfs/$HASH" "HTTP/1.1 403 Forbidden"
37
-'
38
-
35
test_expect_success "GET IPFS directory path succeeds" '
36
mkdir dir &&
37
echo "12345" >dir/test &&
test/supernode_client/main.go
+1
-1
@@ -109,7 +109,7 @@ func run() error {
109
110
opts := []corehttp.ServeOption{
111
corehttp.CommandsOption(cmdCtx(node, repoPath)),
112
- corehttp.GatewayOption(false, nil),
112
+ corehttp.GatewayOption(),
113
}
114
115
if *cat {