Fix issue with Sign/VerifyRequestHeader proto.Clone

proto.Clone couldn't makes copy for custom fields.
We should reset and restore MetaHeader before/after Sign/Verify.
Add test coverage to check that all works like expected.

service/meta.go

@@ -8,10 +8,11 @@ import (
 
 type (
 	// MetaHeader contains meta information of request.
-	// It provides methods to get or set meta information
-	// and reset meta header.
+	// It provides methods to get or set meta information meta header.
+	// Also contains methods to reset and restore meta header.
 	MetaHeader interface {
-		ResetMeta()
+		ResetMeta() RequestMetaHeader
+		RestoreMeta(RequestMetaHeader)
 
 		// TTLRequest to verify and update ttl requests.
 		GetTTL() uint32
@@ -22,7 +23,7 @@ type (
 		SetEpoch(v uint64)
 	}
 
-	// TTLCondition is closure, that allows to validate request with ttl
+	// TTLCondition is closure, that allows to validate request with ttl.
 	TTLCondition func(ttl uint32) error
 )
 
@@ -45,14 +46,21 @@ const (
 	ErrIncorrectTTL = internal.Error("incorrect ttl")
 )
 
-// SetTTL sets TTL to RequestMetaHeader
+// SetTTL sets TTL to RequestMetaHeader.
 func (m *RequestMetaHeader) SetTTL(v uint32) { m.TTL = v }
 
-// SetEpoch sets Epoch to RequestMetaHeader
+// SetEpoch sets Epoch to RequestMetaHeader.
 func (m *RequestMetaHeader) SetEpoch(v uint64) { m.Epoch = v }
 
-// ResetMeta sets RequestMetaHeader to empty value
-func (m *RequestMetaHeader) ResetMeta() { m.Reset() }
+// ResetMeta returns current value and sets RequestMetaHeader to empty value.
+func (m *RequestMetaHeader) ResetMeta() RequestMetaHeader {
+	cp := *m
+	m.Reset()
+	return cp
+}
+
+// RestoreMeta sets current RequestMetaHeader to passed value.
+func (m *RequestMetaHeader) RestoreMeta(v RequestMetaHeader) { *m = v }
 
 // IRNonForwarding condition that allows NonForwardingTTL only for IR
 func IRNonForwarding(role NodeRole) TTLCondition {

service/verify.go

@@ -10,7 +10,7 @@ import (
 )
 
 type (
-	// VerifiableRequest adds possibility to sign and verify request header
+	// VerifiableRequest adds possibility to sign and verify request header.
 	VerifiableRequest interface {
 		proto.Message
 		Marshal() ([]byte, error)
@@ -30,19 +30,19 @@ type (
 )
 
 const (
-	// ErrCannotLoadPublicKey is raised when cannot unmarshal public key from RequestVerificationHeader_Sign
+	// ErrCannotLoadPublicKey is raised when cannot unmarshal public key from RequestVerificationHeader_Sign.
 	ErrCannotLoadPublicKey = internal.Error("cannot load public key")
 
-	// ErrCannotFindOwner is raised when signatures empty in GetOwner
+	// ErrCannotFindOwner is raised when signatures empty in GetOwner.
 	ErrCannotFindOwner = internal.Error("cannot find owner public key")
 )
 
-// SetSignatures replaces signatures stored in RequestVerificationHeader
+// SetSignatures replaces signatures stored in RequestVerificationHeader.
 func (m *RequestVerificationHeader) SetSignatures(signatures []*RequestVerificationHeader_Signature) {
 	m.Signatures = signatures
 }
 
-// AddSignature adds new Signature into RequestVerificationHeader
+// AddSignature adds new Signature into RequestVerificationHeader.
 func (m *RequestVerificationHeader) AddSignature(sig *RequestVerificationHeader_Signature) {
 	if sig == nil {
 		return
@@ -123,12 +123,14 @@ func newSignature(key *ecdsa.PrivateKey, data []byte) (*RequestVerificationHeade
 // SignRequestHeader receives private key and request with RequestVerificationHeader,
 // tries to marshal and sign request with passed PrivateKey, after that adds
 // new signature to headers. If something went wrong, returns error.
-func SignRequestHeader(key *ecdsa.PrivateKey, req VerifiableRequest) error {
-	msg := proto.Clone(req).(VerifiableRequest)
-
+func SignRequestHeader(key *ecdsa.PrivateKey, msg VerifiableRequest) error {
 	// ignore meta header
 	if meta, ok := msg.(MetaHeader); ok {
-		meta.ResetMeta()
+		h := meta.ResetMeta()
+
+		defer func() {
+			meta.RestoreMeta(h)
+		}()
 	}
 
 	data, err := msg.Marshal()
@@ -141,22 +143,28 @@ func SignRequestHeader(key *ecdsa.PrivateKey, req VerifiableRequest) error {
 		return err
 	}
 
-	req.AddSignature(signature)
+	msg.AddSignature(signature)
 
 	return nil
 }
 
 // VerifyRequestHeader receives request with RequestVerificationHeader,
-// tries to marshal and verify each signature from request
+// tries to marshal and verify each signature from request.
 // If something went wrong, returns error.
-func VerifyRequestHeader(req VerifiableRequest) error {
-	msg := proto.Clone(req).(VerifiableRequest)
+func VerifyRequestHeader(msg VerifiableRequest) error {
 	// ignore meta header
 	if meta, ok := msg.(MetaHeader); ok {
-		meta.ResetMeta()
+		h := meta.ResetMeta()
+
+		defer func() {
+			meta.RestoreMeta(h)
+		}()
 	}
 
 	signatures := msg.GetSignatures()
+	defer func() {
+		msg.SetSignatures(signatures)
+	}()
 
 	for i := range signatures {
 		msg.SetSignatures(signatures[:i])
@@ -177,3 +185,35 @@ func VerifyRequestHeader(req VerifiableRequest) error {
 
 	return nil
 }
+
+// testCustomField for test usage only.
+type testCustomField [8]uint32
+
+var _ internal.Custom = (*testCustomField)(nil)
+
+// Reset skip, it's for test usage only.
+func (t testCustomField) Reset() {}
+
+// ProtoMessage skip, it's for test usage only.
+func (t testCustomField) ProtoMessage() {}
+
+// Size skip, it's for test usage only.
+func (t testCustomField) Size() int { return 32 }
+
+// String skip, it's for test usage only.
+func (t testCustomField) String() string { return "" }
+
+// Bytes skip, it's for test usage only.
+func (t testCustomField) Bytes() []byte { return nil }
+
+// Unmarshal skip, it's for test usage only.
+func (t testCustomField) Unmarshal(data []byte) error { return nil }
+
+// Empty skip, it's for test usage only.
+func (t testCustomField) Empty() bool { return false }
+
+// UnmarshalTo skip, it's for test usage only.
+func (t testCustomField) MarshalTo(data []byte) (int, error) { return 0, nil }
+
+// Marshal skip, it's for test usage only.
+func (t testCustomField) Marshal() ([]byte, error) { return nil, nil }

service/verify_test.go

@@ -1,9 +1,12 @@
 package service
 
 import (
+	"bytes"
+	"log"
 	"math"
 	"testing"
 
+	"github.com/gogo/protobuf/proto"
 	crypto "github.com/nspcc-dev/neofs-crypto"
 	"github.com/nspcc-dev/neofs-crypto/test"
 	"github.com/pkg/errors"
@@ -104,3 +107,37 @@ func TestMaintainableRequest(t *testing.T) {
 		require.EqualError(t, errors.Cause(err), crypto.ErrInvalidSignature.Error())
 	}
 }
+
+func TestVerifyAndSignRequestHeaderWithoutCloning(t *testing.T) {
+	key := test.DecodeKey(0)
+
+	custom := testCustomField{1, 2, 3, 4, 5, 6, 7, 8}
+
+	b := &TestRequest{
+		IntField:    math.MaxInt32,
+		StringField: "TestRequestStringField",
+		BytesField:  []byte("TestRequestBytesField"),
+		CustomField: &custom,
+		RequestMetaHeader: RequestMetaHeader{
+			TTL:   math.MaxInt32 - 8,
+			Epoch: math.MaxInt64 - 12,
+		},
+	}
+
+	require.NoError(t, SignRequestHeader(key, b))
+	require.NoError(t, VerifyRequestHeader(b))
+
+	require.Len(t, b.Signatures, 1)
+	require.Equal(t, custom, *b.CustomField)
+	require.Equal(t, uint32(math.MaxInt32-8), b.GetTTL())
+	require.Equal(t, uint64(math.MaxInt64-12), b.GetEpoch())
+
+	buf := bytes.NewBuffer(nil)
+	log.SetOutput(buf)
+
+	cp, ok := proto.Clone(b).(*TestRequest)
+	require.True(t, ok)
+	require.NotEqual(t, b, cp)
+
+	require.Contains(t, buf.String(), "proto: don't know how to copy")
+}

service/verify_test.proto

@@ -12,6 +12,7 @@ message TestRequest {
     int32 IntField                   = 1;
     string StringField               = 2;
     bytes BytesField                 = 3;
+    bytes CustomField                = 4 [(gogoproto.customtype) = "testCustomField"];
     RequestMetaHeader Meta           = 98 [(gogoproto.embed) = true, (gogoproto.nullable) = false];
     RequestVerificationHeader Header = 99 [(gogoproto.embed) = true, (gogoproto.nullable) = false];
 }