From 300654b045dbd5ebcb05e601aa51c76321f2c30c Mon Sep 17 00:00:00 2001 From: Evgenii Stratonikov Date: Wed, 15 May 2024 11:51:38 +0300 Subject: [PATCH 1/3] [#1083] objsvc/v2: Properly check response status after forwarding Previously we had cryptic error: ``` debug get/remote.go:38 remote call failed {"component": "Object.Get service", "request": "HEAD", "address": "9sTxoVrhJ7WBtXQfK2NJ7zDV5yCF7BPLKK1XTxYPdGsP/BbHV4KZZ8y2BPqAT5kyjdHRLkfbtY2xf5uYoMVqxACn1", "raw": false, "local": false, "with session": false, "with bearer": false, "error": "unexpected header type "} ``` Now we have and expected error: ``` debug get/remote.go:38 remote call failed {"component": "Object.Get service", "request": "HEAD", "address": "D2rqaMG4D2VHdv3HKky8UYSYmwQFH2v9oXXqtyRZPTMy/BbHV4KZZ8y2BPqAT5kyjdHRLkfbtY2xf5uYoMVqxACn1", "raw": false, "local": false, "with session": false, "with bearer": false, "error": "status: code = 2049 message = object not found"} ``` Signed-off-by: Evgenii Stratonikov --- pkg/services/object/get/v2/head_forwarder.go | 3 +-- pkg/services/object/get/v2/util.go | 1 - 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/pkg/services/object/get/v2/head_forwarder.go b/pkg/services/object/get/v2/head_forwarder.go index 11286321a..0c2925859 100644 --- a/pkg/services/object/get/v2/head_forwarder.go +++ b/pkg/services/object/get/v2/head_forwarder.go @@ -24,7 +24,6 @@ import ( type headRequestForwarder struct { Request *objectV2.HeadRequest - Response *objectV2.HeadResponse OnceResign sync.Once ObjectAddr oid.Address Key *ecdsa.PrivateKey @@ -172,5 +171,5 @@ func (f *headRequestForwarder) verifyResponse(headResp *objectV2.HeadResponse, p return errResponseVerificationFailed(err) } - return checkStatus(f.Response.GetMetaHeader().GetStatus()) + return checkStatus(headResp.GetMetaHeader().GetStatus()) } diff --git a/pkg/services/object/get/v2/util.go b/pkg/services/object/get/v2/util.go index 610076c7a..d71b381e7 100644 --- a/pkg/services/object/get/v2/util.go +++ b/pkg/services/object/get/v2/util.go @@ -249,7 +249,6 @@ func (s *Service) toHeadPrm(req *objectV2.HeadRequest, resp *objectV2.HeadRespon forwarder := &headRequestForwarder{ Request: req, - Response: resp, ObjectAddr: objAddr, Key: key, } -- 2.45.3 From 0924b62a95889e731890ff5725fc38871a2c3601 Mon Sep 17 00:00:00 2001 From: Evgenii Stratonikov Date: Wed, 15 May 2024 11:56:17 +0300 Subject: [PATCH 2/3] [#1083] objsvc/v2: Unify response verification after forwarding 1. Use the same routine for HEAD/GET_RANGE methods. 2. Make error message similar. Signed-off-by: Evgenii Stratonikov --- pkg/services/object/get/v2/errors.go | 4 ---- .../object/get/v2/get_range_forwarder.go | 17 +---------------- pkg/services/object/get/v2/head_forwarder.go | 17 +---------------- pkg/services/object/get/v2/util.go | 19 +++++++++++++++++++ 4 files changed, 21 insertions(+), 36 deletions(-) diff --git a/pkg/services/object/get/v2/errors.go b/pkg/services/object/get/v2/errors.go index 01b57f1f2..213455e10 100644 --- a/pkg/services/object/get/v2/errors.go +++ b/pkg/services/object/get/v2/errors.go @@ -63,10 +63,6 @@ func errCouldNotWriteObjChunk(forwarder string, err error) error { return fmt.Errorf("could not write object chunk in %s forwarder: %w", forwarder, err) } -func errCouldNotVerifyRangeResponse(resp *objectV2.GetRangeResponse, err error) error { - return fmt.Errorf("could not verify %T: %w", resp, err) -} - func errCouldNotCreateGetRangeStream(err error) error { return fmt.Errorf("could not create Get payload range stream: %w", err) } diff --git a/pkg/services/object/get/v2/get_range_forwarder.go b/pkg/services/object/get/v2/get_range_forwarder.go index 5b05ec370..10ecfc4a3 100644 --- a/pkg/services/object/get/v2/get_range_forwarder.go +++ b/pkg/services/object/get/v2/get_range_forwarder.go @@ -14,7 +14,6 @@ import ( "git.frostfs.info/TrueCloudLab/frostfs-api-go/v2/signature" "git.frostfs.info/TrueCloudLab/frostfs-node/pkg/core/client" "git.frostfs.info/TrueCloudLab/frostfs-node/pkg/network" - "git.frostfs.info/TrueCloudLab/frostfs-node/pkg/services/object/internal" internalclient "git.frostfs.info/TrueCloudLab/frostfs-node/pkg/services/object/internal/client" "git.frostfs.info/TrueCloudLab/frostfs-observability/tracing" objectSDK "git.frostfs.info/TrueCloudLab/frostfs-sdk-go/object" @@ -64,20 +63,6 @@ func (f *getRangeRequestForwarder) forwardRequestToNode(ctx context.Context, add return nil, f.readStream(ctx, rangeStream, c, pubkey) } -func (f *getRangeRequestForwarder) verifyResponse(resp *objectV2.GetRangeResponse, pubkey []byte) error { - // verify response key - if err := internal.VerifyResponseKeyV2(pubkey, resp); err != nil { - return err - } - - // verify response structure - if err := signature.VerifyServiceMessage(resp); err != nil { - return errCouldNotVerifyRangeResponse(resp, err) - } - - return checkStatus(resp.GetMetaHeader().GetStatus()) -} - func (f *getRangeRequestForwarder) openStream(ctx context.Context, addr network.Address, c client.MultiAddressClient) (*rpc.ObjectRangeResponseReader, error) { // open stream var rangeStream *rpc.ObjectRangeResponseReader @@ -107,7 +92,7 @@ func (f *getRangeRequestForwarder) readStream(ctx context.Context, rangeStream * return errReadingResponseFailed(err) } - if err := f.verifyResponse(resp, pubkey); err != nil { + if err := verifyResponse(resp, pubkey); err != nil { return err } diff --git a/pkg/services/object/get/v2/head_forwarder.go b/pkg/services/object/get/v2/head_forwarder.go index 0c2925859..5e16008b8 100644 --- a/pkg/services/object/get/v2/head_forwarder.go +++ b/pkg/services/object/get/v2/head_forwarder.go @@ -13,7 +13,6 @@ import ( "git.frostfs.info/TrueCloudLab/frostfs-api-go/v2/signature" "git.frostfs.info/TrueCloudLab/frostfs-node/pkg/core/client" "git.frostfs.info/TrueCloudLab/frostfs-node/pkg/network" - "git.frostfs.info/TrueCloudLab/frostfs-node/pkg/services/object/internal" "git.frostfs.info/TrueCloudLab/frostfs-observability/tracing" frostfscrypto "git.frostfs.info/TrueCloudLab/frostfs-sdk-go/crypto" objectSDK "git.frostfs.info/TrueCloudLab/frostfs-sdk-go/object" @@ -60,7 +59,7 @@ func (f *headRequestForwarder) forwardRequestToNode(ctx context.Context, addr ne return nil, err } - if err := f.verifyResponse(headResp, pubkey); err != nil { + if err := verifyResponse(headResp, pubkey); err != nil { return nil, err } @@ -159,17 +158,3 @@ func (f *headRequestForwarder) sendHeadRequest(ctx context.Context, addr network } return headResp, nil } - -func (f *headRequestForwarder) verifyResponse(headResp *objectV2.HeadResponse, pubkey []byte) error { - // verify response key - if err := internal.VerifyResponseKeyV2(pubkey, headResp); err != nil { - return err - } - - // verify response structure - if err := signature.VerifyServiceMessage(headResp); err != nil { - return errResponseVerificationFailed(err) - } - - return checkStatus(headResp.GetMetaHeader().GetStatus()) -} diff --git a/pkg/services/object/get/v2/util.go b/pkg/services/object/get/v2/util.go index d71b381e7..852c2aec3 100644 --- a/pkg/services/object/get/v2/util.go +++ b/pkg/services/object/get/v2/util.go @@ -8,11 +8,13 @@ import ( objectV2 "git.frostfs.info/TrueCloudLab/frostfs-api-go/v2/object" "git.frostfs.info/TrueCloudLab/frostfs-api-go/v2/refs" "git.frostfs.info/TrueCloudLab/frostfs-api-go/v2/session" + "git.frostfs.info/TrueCloudLab/frostfs-api-go/v2/signature" "git.frostfs.info/TrueCloudLab/frostfs-api-go/v2/status" "git.frostfs.info/TrueCloudLab/frostfs-node/pkg/core/client" "git.frostfs.info/TrueCloudLab/frostfs-node/pkg/network" objectSvc "git.frostfs.info/TrueCloudLab/frostfs-node/pkg/services/object" getsvc "git.frostfs.info/TrueCloudLab/frostfs-node/pkg/services/object/get" + "git.frostfs.info/TrueCloudLab/frostfs-node/pkg/services/object/internal" "git.frostfs.info/TrueCloudLab/frostfs-node/pkg/services/object/util" clientSDK "git.frostfs.info/TrueCloudLab/frostfs-sdk-go/client" apistatus "git.frostfs.info/TrueCloudLab/frostfs-sdk-go/client/status" @@ -408,3 +410,20 @@ func chunkToSend(global, local int, chunk []byte) []byte { return chunk[global-local:] } + +type apiResponse interface { + GetMetaHeader() *session.ResponseMetaHeader + GetVerificationHeader() *session.ResponseVerificationHeader +} + +func verifyResponse(resp apiResponse, pubkey []byte) error { + if err := internal.VerifyResponseKeyV2(pubkey, resp); err != nil { + return err + } + + if err := signature.VerifyServiceMessage(resp); err != nil { + return errResponseVerificationFailed(err) + } + + return checkStatus(resp.GetMetaHeader().GetStatus()) +} -- 2.45.3 From b3eaa8a9bcf0933f511adbc7bfea6a8d67b2c679 Mon Sep 17 00:00:00 2001 From: Evgenii Stratonikov Date: Wed, 15 May 2024 12:03:32 +0300 Subject: [PATCH 3/3] [#1083] objsvc/v2: Check response status in RANGE_HASH forwarder Fixes #1083 Signed-off-by: Evgenii Stratonikov --- pkg/services/object/get/v2/get_range_hash.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/pkg/services/object/get/v2/get_range_hash.go b/pkg/services/object/get/v2/get_range_hash.go index 0054f0e9f..e97b60f66 100644 --- a/pkg/services/object/get/v2/get_range_hash.go +++ b/pkg/services/object/get/v2/get_range_hash.go @@ -142,6 +142,9 @@ func (s *Service) forwardGetRangeHashRequest(ctx context.Context, req *objectV2. resp, err := s.performGetRangeHashOnNode(ctx, req, info) if err == nil { + if err := verifyResponse(resp, info.PublicKey()); err != nil { + return nil, err + } return resp, nil } if firstErr == nil { -- 2.45.3