From 29b79db998912e276531ff836420636bc8130270 Mon Sep 17 00:00:00 2001 From: Roman Khimov Date: Tue, 6 Apr 2021 16:45:06 +0300 Subject: [PATCH 1/5] oracle: we only work with https, forget http --- pkg/core/oracle_test.go | 44 +++++++++++++++++----------------- pkg/services/oracle/request.go | 2 +- 2 files changed, 23 insertions(+), 23 deletions(-) diff --git a/pkg/core/oracle_test.go b/pkg/core/oracle_test.go index 81fa264a5..fb2bbb0bf 100644 --- a/pkg/core/oracle_test.go +++ b/pkg/core/oracle_test.go @@ -133,19 +133,19 @@ func TestOracle(t *testing.T) { cs := getOracleContractState(bc.contracts.Oracle.Hash, bc.contracts.Std.Hash) require.NoError(t, bc.contracts.Management.PutContractState(bc.dao, cs)) - putOracleRequest(t, cs.Hash, bc, "http://get.1234", nil, "handle", []byte{}, 10_000_000) - putOracleRequest(t, cs.Hash, bc, "http://get.1234", nil, "handle", []byte{}, 10_000_000) - putOracleRequest(t, cs.Hash, bc, "http://get.timeout", nil, "handle", []byte{}, 10_000_000) - putOracleRequest(t, cs.Hash, bc, "http://get.notfound", nil, "handle", []byte{}, 10_000_000) - putOracleRequest(t, cs.Hash, bc, "http://get.forbidden", nil, "handle", []byte{}, 10_000_000) - putOracleRequest(t, cs.Hash, bc, "http://private.url", nil, "handle", []byte{}, 10_000_000) - putOracleRequest(t, cs.Hash, bc, "http://get.big", nil, "handle", []byte{}, 10_000_000) - putOracleRequest(t, cs.Hash, bc, "http://get.maxallowed", nil, "handle", []byte{}, 10_000_000) - putOracleRequest(t, cs.Hash, bc, "http://get.maxallowed", nil, "handle", []byte{}, 100_000_000) + putOracleRequest(t, cs.Hash, bc, "https://get.1234", nil, "handle", []byte{}, 10_000_000) + putOracleRequest(t, cs.Hash, bc, "https://get.1234", nil, "handle", []byte{}, 10_000_000) + putOracleRequest(t, cs.Hash, bc, "https://get.timeout", nil, "handle", []byte{}, 10_000_000) + putOracleRequest(t, cs.Hash, bc, "https://get.notfound", nil, "handle", []byte{}, 10_000_000) + putOracleRequest(t, cs.Hash, bc, "https://get.forbidden", nil, "handle", []byte{}, 10_000_000) + putOracleRequest(t, cs.Hash, bc, "https://private.url", nil, "handle", []byte{}, 10_000_000) + putOracleRequest(t, cs.Hash, bc, "https://get.big", nil, "handle", []byte{}, 10_000_000) + putOracleRequest(t, cs.Hash, bc, "https://get.maxallowed", nil, "handle", []byte{}, 10_000_000) + putOracleRequest(t, cs.Hash, bc, "https://get.maxallowed", nil, "handle", []byte{}, 100_000_000) flt := "Values[1]" - putOracleRequest(t, cs.Hash, bc, "http://get.filter", &flt, "handle", []byte{}, 10_000_000) - putOracleRequest(t, cs.Hash, bc, "http://get.filterinv", &flt, "handle", []byte{}, 10_000_000) + putOracleRequest(t, cs.Hash, bc, "https://get.filter", &flt, "handle", []byte{}, 10_000_000) + putOracleRequest(t, cs.Hash, bc, "https://get.filterinv", &flt, "handle", []byte{}, 10_000_000) checkResp := func(t *testing.T, id uint64, resp *transaction.OracleResponse) *state.OracleRequest { req, err := oracleCtr.GetRequestInternal(bc.dao, id) @@ -279,7 +279,7 @@ func TestOracleFull(t *testing.T) { t.Cleanup(orc.Shutdown) bc.setNodesByRole(t, true, noderoles.Oracle, keys.PublicKeys{acc.PrivateKey().PublicKey()}) - putOracleRequest(t, cs.Hash, bc, "http://get.1234", new(string), "handle", []byte{}, 10_000_000) + putOracleRequest(t, cs.Hash, bc, "https://get.1234", new(string), "handle", []byte{}, 10_000_000) require.Eventually(t, func() bool { return mp.Count() == 1 }, time.Second*3, time.Millisecond*200) @@ -341,43 +341,43 @@ func (c *httpClient) Get(url string) (*http.Response, error) { func newDefaultHTTPClient() oracle.HTTPClient { return &httpClient{ responses: map[string]testResponse{ - "http://get.1234": { + "https://get.1234": { code: http.StatusOK, body: []byte{1, 2, 3, 4}, }, - "http://get.4321": { + "https://get.4321": { code: http.StatusOK, body: []byte{4, 3, 2, 1}, }, - "http://get.timeout": { + "https://get.timeout": { code: http.StatusRequestTimeout, body: []byte{}, }, - "http://get.notfound": { + "https://get.notfound": { code: http.StatusNotFound, body: []byte{}, }, - "http://get.forbidden": { + "https://get.forbidden": { code: http.StatusForbidden, body: []byte{}, }, - "http://private.url": { + "https://private.url": { code: http.StatusOK, body: []byte("passwords"), }, - "http://get.big": { + "https://get.big": { code: http.StatusOK, body: make([]byte, transaction.MaxOracleResultSize+1), }, - "http://get.maxallowed": { + "https://get.maxallowed": { code: http.StatusOK, body: make([]byte, transaction.MaxOracleResultSize), }, - "http://get.filter": { + "https://get.filter": { code: http.StatusOK, body: []byte(`{"Values":["one", 2, 3],"Another":null}`), }, - "http://get.filterinv": { + "https://get.filterinv": { code: http.StatusOK, body: []byte{0xFF}, }, diff --git a/pkg/services/oracle/request.go b/pkg/services/oracle/request.go index 067b3a574..f0d5f48ee 100644 --- a/pkg/services/oracle/request.go +++ b/pkg/services/oracle/request.go @@ -102,7 +102,7 @@ func (o *Oracle) processRequest(priv *keys.PrivateKey, req request) error { } if err != nil { resp.Code = transaction.Forbidden - } else if u.Scheme == "http" { + } else if u.Scheme == "https" { r, err := o.Client.Get(req.Req.URL) switch { case err != nil: From bd9a303e2974d9933b865404f7ed52cba9c5723c Mon Sep 17 00:00:00 2001 From: Roman Khimov Date: Tue, 6 Apr 2021 16:53:32 +0300 Subject: [PATCH 2/5] oracle: URI host validation is only relevant for https NeoFS doesn't have hosts. --- pkg/services/oracle/request.go | 53 +++++++++++++++++++--------------- 1 file changed, 29 insertions(+), 24 deletions(-) diff --git a/pkg/services/oracle/request.go b/pkg/services/oracle/request.go index f0d5f48ee..df7cc82dd 100644 --- a/pkg/services/oracle/request.go +++ b/pkg/services/oracle/request.go @@ -97,35 +97,40 @@ func (o *Oracle) processRequest(priv *keys.PrivateKey, req request) error { } resp := &transaction.OracleResponse{ID: req.ID} u, err := url.ParseRequestURI(req.Req.URL) - if err == nil && !o.MainCfg.AllowPrivateHost { - err = o.URIValidator(u) - } if err != nil { resp.Code = transaction.Forbidden } else if u.Scheme == "https" { - r, err := o.Client.Get(req.Req.URL) - switch { - case err != nil: - resp.Code = transaction.Error - case r.StatusCode == http.StatusOK: - result, err := readResponse(r.Body, transaction.MaxOracleResultSize) + if !o.MainCfg.AllowPrivateHost { + err = o.URIValidator(u) if err != nil { - if errors.Is(err, ErrResponseTooLarge) { - resp.Code = transaction.ResponseTooLarge - } else { - resp.Code = transaction.Error - } - break + resp.Code = transaction.Forbidden + } + } + if err == nil { + r, err := o.Client.Get(req.Req.URL) + switch { + case err != nil: + resp.Code = transaction.Error + case r.StatusCode == http.StatusOK: + result, err := readResponse(r.Body, transaction.MaxOracleResultSize) + if err != nil { + if errors.Is(err, ErrResponseTooLarge) { + resp.Code = transaction.ResponseTooLarge + } else { + resp.Code = transaction.Error + } + break + } + resp.Code, resp.Result = filterRequest(result, req.Req) + case r.StatusCode == http.StatusForbidden: + resp.Code = transaction.Forbidden + case r.StatusCode == http.StatusNotFound: + resp.Code = transaction.NotFound + case r.StatusCode == http.StatusRequestTimeout: + resp.Code = transaction.Timeout + default: + resp.Code = transaction.Error } - resp.Code, resp.Result = filterRequest(result, req.Req) - case r.StatusCode == http.StatusForbidden: - resp.Code = transaction.Forbidden - case r.StatusCode == http.StatusNotFound: - resp.Code = transaction.NotFound - case r.StatusCode == http.StatusRequestTimeout: - resp.Code = transaction.Timeout - default: - resp.Code = transaction.Error } } else if err == nil && u.Scheme == neofs.URIScheme { ctx, cancel := context.WithTimeout(context.Background(), time.Duration(o.MainCfg.NeoFS.Timeout)*time.Millisecond) From d2f7f00997e141965f4f6e6a832f3f2b4877119d Mon Sep 17 00:00:00 2001 From: Roman Khimov Date: Tue, 6 Apr 2021 16:56:19 +0300 Subject: [PATCH 3/5] oracle: change neofs URI scheme Strip '//', see neo-project/neo-modules#518. --- pkg/services/oracle/neofs/neofs.go | 12 ++++++------ pkg/services/oracle/neofs/neofs_test.go | 10 +++++----- 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/pkg/services/oracle/neofs/neofs.go b/pkg/services/oracle/neofs/neofs.go index a767e5467..ee09ae424 100644 --- a/pkg/services/oracle/neofs/neofs.go +++ b/pkg/services/oracle/neofs/neofs.go @@ -47,7 +47,7 @@ var ( ) // Get returns neofs object from the provided url. -// URI scheme is "neofs:////". +// URI scheme is "neofs://". // If Command is not provided, full object is requested. func Get(ctx context.Context, priv *keys.PrivateKey, u *url.URL, addr string) ([]byte, error) { objectAddr, ps, err := parseNeoFSURL(u) @@ -80,25 +80,25 @@ func parseNeoFSURL(u *url.URL) (*object.Address, []string, error) { return nil, nil, ErrInvalidScheme } - ps := strings.Split(strings.TrimPrefix(u.Path, "/"), "/") - if len(ps) == 0 || ps[0] == "" { + ps := strings.Split(u.Opaque, "/") + if len(ps) < 2 { return nil, nil, ErrMissingObject } containerID := container.NewID() - if err := containerID.Parse(u.Hostname()); err != nil { + if err := containerID.Parse(ps[0]); err != nil { return nil, nil, fmt.Errorf("%w: %v", ErrInvalidContainer, err) } objectID := object.NewID() - if err := objectID.Parse(ps[0]); err != nil { + if err := objectID.Parse(ps[1]); err != nil { return nil, nil, fmt.Errorf("%w: %v", ErrInvalidObject, err) } objectAddr := object.NewAddress() objectAddr.SetContainerID(containerID) objectAddr.SetObjectID(objectID) - return objectAddr, ps[1:], nil + return objectAddr, ps[2:], nil } func getPayload(ctx context.Context, c *client.Client, addr *object.Address) ([]byte, error) { diff --git a/pkg/services/oracle/neofs/neofs_test.go b/pkg/services/oracle/neofs/neofs_test.go index 8f13cead5..6483875b9 100644 --- a/pkg/services/oracle/neofs/neofs_test.go +++ b/pkg/services/oracle/neofs/neofs_test.go @@ -44,7 +44,7 @@ func TestParseNeoFSURL(t *testing.T) { oid := object.NewID() require.NoError(t, oid.Parse(oStr)) - validPrefix := "neofs://" + cStr + "/" + oStr + validPrefix := "neofs:" + cStr + "/" + oStr objectAddr := object.NewAddress() objectAddr.SetContainerID(cid) objectAddr.SetObjectID(oid) @@ -57,10 +57,10 @@ func TestParseNeoFSURL(t *testing.T) { {validPrefix, nil, nil}, {validPrefix + "/", []string{""}, nil}, {validPrefix + "/range/1|2", []string{"range", "1|2"}, nil}, - {"neoffs://" + cStr + "/" + oStr, nil, ErrInvalidScheme}, - {"neofs://" + cStr, nil, ErrMissingObject}, - {"neofs://" + cStr + "ooo/" + oStr, nil, ErrInvalidContainer}, - {"neofs://" + cStr + "/ooo" + oStr, nil, ErrInvalidObject}, + {"neoffs:" + cStr + "/" + oStr, nil, ErrInvalidScheme}, + {"neofs:" + cStr, nil, ErrMissingObject}, + {"neofs:" + cStr + "ooo/" + oStr, nil, ErrInvalidContainer}, + {"neofs:" + cStr + "/ooo" + oStr, nil, ErrInvalidObject}, } for _, tc := range testCases { t.Run(tc.url, func(t *testing.T) { From f69fd342206e6716c48767118665cf57a6c000df Mon Sep 17 00:00:00 2001 From: Roman Khimov Date: Tue, 6 Apr 2021 17:02:00 +0300 Subject: [PATCH 4/5] oracle: add logging, refactor request processing a bit And return more appropriate errors in some cases. --- pkg/services/oracle/request.go | 61 +++++++++++++++++++++------------- 1 file changed, 37 insertions(+), 24 deletions(-) diff --git a/pkg/services/oracle/request.go b/pkg/services/oracle/request.go index df7cc82dd..9a354b330 100644 --- a/pkg/services/oracle/request.go +++ b/pkg/services/oracle/request.go @@ -98,20 +98,27 @@ func (o *Oracle) processRequest(priv *keys.PrivateKey, req request) error { resp := &transaction.OracleResponse{ID: req.ID} u, err := url.ParseRequestURI(req.Req.URL) if err != nil { - resp.Code = transaction.Forbidden - } else if u.Scheme == "https" { - if !o.MainCfg.AllowPrivateHost { - err = o.URIValidator(u) - if err != nil { - resp.Code = transaction.Forbidden + o.Log.Warn("malformed oracle request", zap.String("url", req.Req.URL), zap.Error(err)) + resp.Code = transaction.ProtocolNotSupported + } else { + switch u.Scheme { + case "https": + if !o.MainCfg.AllowPrivateHost { + err = o.URIValidator(u) + if err != nil { + o.Log.Warn("forbidden oracle request", zap.String("url", req.Req.URL)) + resp.Code = transaction.Forbidden + break + } } - } - if err == nil { r, err := o.Client.Get(req.Req.URL) - switch { - case err != nil: + if err != nil { + o.Log.Warn("oracle request failed", zap.String("url", req.Req.URL), zap.Error(err)) resp.Code = transaction.Error - case r.StatusCode == http.StatusOK: + break + } + switch r.StatusCode { + case http.StatusOK: result, err := readResponse(r.Body, transaction.MaxOracleResultSize) if err != nil { if errors.Is(err, ErrResponseTooLarge) { @@ -119,30 +126,36 @@ func (o *Oracle) processRequest(priv *keys.PrivateKey, req request) error { } else { resp.Code = transaction.Error } + o.Log.Warn("failed to read data for oracle request", zap.String("url", req.Req.URL), zap.Error(err)) break } resp.Code, resp.Result = filterRequest(result, req.Req) - case r.StatusCode == http.StatusForbidden: + case http.StatusForbidden: resp.Code = transaction.Forbidden - case r.StatusCode == http.StatusNotFound: + case http.StatusNotFound: resp.Code = transaction.NotFound - case r.StatusCode == http.StatusRequestTimeout: + case http.StatusRequestTimeout: resp.Code = transaction.Timeout default: resp.Code = transaction.Error } - } - } else if err == nil && u.Scheme == neofs.URIScheme { - ctx, cancel := context.WithTimeout(context.Background(), time.Duration(o.MainCfg.NeoFS.Timeout)*time.Millisecond) - defer cancel() - index := (int(req.ID) + incTx.attempts) % len(o.MainCfg.NeoFS.Nodes) - res, err := neofs.Get(ctx, priv, u, o.MainCfg.NeoFS.Nodes[index]) - if err != nil { - resp.Code = transaction.Error - } else { - resp.Code, resp.Result = filterRequest(res, req.Req) + case neofs.URIScheme: + ctx, cancel := context.WithTimeout(context.Background(), time.Duration(o.MainCfg.NeoFS.Timeout)*time.Millisecond) + defer cancel() + index := (int(req.ID) + incTx.attempts) % len(o.MainCfg.NeoFS.Nodes) + res, err := neofs.Get(ctx, priv, u, o.MainCfg.NeoFS.Nodes[index]) + if err != nil { + o.Log.Warn("oracle request failed", zap.String("url", req.Req.URL), zap.Error(err)) + resp.Code = transaction.Error + } else { + resp.Code, resp.Result = filterRequest(res, req.Req) + } + default: + resp.Code = transaction.ProtocolNotSupported + o.Log.Warn("unknown oracle request scheme", zap.String("url", req.Req.URL)) } } + o.Log.Debug("oracle request processed", zap.String("url", req.Req.URL), zap.Int("code", int(resp.Code)), zap.String("result", string(resp.Result))) currentHeight := o.Chain.BlockHeight() _, h, err := o.Chain.GetTransaction(req.Req.OriginalTxID) From 7a1c1638e4c3b091c1163f26d68921c837178962 Mon Sep 17 00:00:00 2001 From: Roman Khimov Date: Tue, 6 Apr 2021 17:08:04 +0300 Subject: [PATCH 5/5] oracle: specify neofs timeout as timeout, set default if 0 There has to be some sane default and we'd like configuration to be consistent with other timeout values. --- pkg/config/oracle_config.go | 4 ++-- pkg/services/oracle/oracle.go | 3 +++ pkg/services/oracle/request.go | 2 +- 3 files changed, 6 insertions(+), 3 deletions(-) diff --git a/pkg/config/oracle_config.go b/pkg/config/oracle_config.go index 479abd0ba..a12454f70 100644 --- a/pkg/config/oracle_config.go +++ b/pkg/config/oracle_config.go @@ -18,6 +18,6 @@ type OracleConfiguration struct { // NeoFSConfiguration is a config for the NeoFS service. type NeoFSConfiguration struct { - Nodes []string `yaml:"Nodes"` - Timeout int `yaml:"Timeout"` + Nodes []string `yaml:"Nodes"` + Timeout time.Duration `yaml:"Timeout"` } diff --git a/pkg/services/oracle/oracle.go b/pkg/services/oracle/oracle.go index 8cfb3b424..94f5084f1 100644 --- a/pkg/services/oracle/oracle.go +++ b/pkg/services/oracle/oracle.go @@ -108,6 +108,9 @@ func NewOracle(cfg Config) (*Oracle, error) { if o.MainCfg.RequestTimeout == 0 { o.MainCfg.RequestTimeout = defaultRequestTimeout } + if o.MainCfg.NeoFS.Timeout == 0 { + o.MainCfg.NeoFS.Timeout = defaultRequestTimeout + } if o.MainCfg.MaxConcurrentRequests == 0 { o.MainCfg.MaxConcurrentRequests = defaultMaxConcurrentRequests } diff --git a/pkg/services/oracle/request.go b/pkg/services/oracle/request.go index 9a354b330..e61f47ba6 100644 --- a/pkg/services/oracle/request.go +++ b/pkg/services/oracle/request.go @@ -140,7 +140,7 @@ func (o *Oracle) processRequest(priv *keys.PrivateKey, req request) error { resp.Code = transaction.Error } case neofs.URIScheme: - ctx, cancel := context.WithTimeout(context.Background(), time.Duration(o.MainCfg.NeoFS.Timeout)*time.Millisecond) + ctx, cancel := context.WithTimeout(context.Background(), o.MainCfg.NeoFS.Timeout) defer cancel() index := (int(req.ID) + incTx.attempts) % len(o.MainCfg.NeoFS.Nodes) res, err := neofs.Get(ctx, priv, u, o.MainCfg.NeoFS.Nodes[index])