From 7d10b432d1392551d0602d4edf2ae00afb37cd34 Mon Sep 17 00:00:00 2001 From: Leonard Lyubich Date: Thu, 7 Jul 2022 13:51:05 +0300 Subject: [PATCH] [#284] *: Return error from all `ReadFromV2` methods Return `error` from all `ReadFromV2` methods in order to support backward compatibility if message will be extended with some formatted field. Signed-off-by: Leonard Lyubich --- accounting/decimal.go | 6 ++- accounting/decimal_test.go | 2 +- bearer/bearer.go | 3 +- checksum/checksum.go | 18 +++++++- client/accounting.go | 21 +++++++--- client/netmap.go | 6 ++- container/container_test.go | 4 +- crypto/crypto_test.go | 2 +- crypto/signature.go | 23 ++++++++++- object/fmt.go | 3 +- object/object.go | 8 ++-- reputation/trust.go | 3 +- session/common.go | 3 +- session/container.go | 3 +- storagegroup/storagegroup.go | 68 +++++++++++++++++++++++-------- storagegroup/storagegroup_test.go | 14 ++----- version/version.go | 6 ++- 17 files changed, 133 insertions(+), 60 deletions(-) diff --git a/accounting/decimal.go b/accounting/decimal.go index e4dae06..7a4eaeb 100644 --- a/accounting/decimal.go +++ b/accounting/decimal.go @@ -13,11 +13,13 @@ import "github.com/nspcc-dev/neofs-api-go/v2/accounting" // _ = Decimal(accounting.Decimal{}) // not recommended type Decimal accounting.Decimal -// ReadFromV2 reads Decimal from the accounting.Decimal message. +// ReadFromV2 reads Decimal from the accounting.Decimal message. Checks if the +// message conforms to NeoFS API V2 protocol. // // See also WriteToV2. -func (d *Decimal) ReadFromV2(m accounting.Decimal) { +func (d *Decimal) ReadFromV2(m accounting.Decimal) error { *d = Decimal(m) + return nil } // WriteToV2 writes Decimal to the accounting.Decimal message. diff --git a/accounting/decimal_test.go b/accounting/decimal_test.go index 7252b56..97a5468 100644 --- a/accounting/decimal_test.go +++ b/accounting/decimal_test.go @@ -32,7 +32,7 @@ func TestDecimalMessageV2(t *testing.T) { m.SetValue(7) m.SetPrecision(8) - d.ReadFromV2(m) + require.NoError(t, d.ReadFromV2(m)) require.EqualValues(t, m.GetValue(), d.Value()) require.EqualValues(t, m.GetPrecision(), d.Precision()) diff --git a/bearer/bearer.go b/bearer/bearer.go index fa395a8..27b266b 100644 --- a/bearer/bearer.go +++ b/bearer/bearer.go @@ -279,10 +279,9 @@ func (b Token) VerifySignature() bool { } var sig neofscrypto.Signature - sig.ReadFromV2(b.sig) // TODO: (#233) check owner<->key relation - return sig.Verify(b.signedData()) + return sig.ReadFromV2(b.sig) == nil && sig.Verify(b.signedData()) } // Marshal encodes Token into a binary format of the NeoFS API protocol diff --git a/checksum/checksum.go b/checksum/checksum.go index a381c57..59a0711 100644 --- a/checksum/checksum.go +++ b/checksum/checksum.go @@ -3,6 +3,7 @@ package checksum import ( "crypto/sha256" "encoding/hex" + "errors" "fmt" "github.com/nspcc-dev/neofs-api-go/v2/refs" @@ -35,11 +36,24 @@ const ( TZ ) -// ReadFromV2 reads Checksum from the refs.Checksum message. +// ReadFromV2 reads Checksum from the refs.Checksum message. Checks if the +// message conforms to NeoFS API V2 protocol. // // See also WriteToV2. -func (c *Checksum) ReadFromV2(m refs.Checksum) { +func (c *Checksum) ReadFromV2(m refs.Checksum) error { + if len(m.GetSum()) == 0 { + return errors.New("missing value") + } + + switch m.GetType() { + default: + return fmt.Errorf("unsupported type %v", m.GetType()) + case refs.SHA256, refs.TillichZemor: + } + *c = Checksum(m) + + return nil } // WriteToV2 writes Checksum to the refs.Checksum message. diff --git a/client/accounting.go b/client/accounting.go index 7f77b1f..f05b314 100644 --- a/client/accounting.go +++ b/client/accounting.go @@ -2,6 +2,8 @@ package client import ( "context" + "errors" + "fmt" v2accounting "github.com/nspcc-dev/neofs-api-go/v2/accounting" "github.com/nspcc-dev/neofs-api-go/v2/refs" @@ -94,12 +96,21 @@ func (c *Client) BalanceGet(ctx context.Context, prm PrmBalanceGet) (*ResBalance cc.result = func(r responseV2) { resp := r.(*v2accounting.BalanceResponse) - if bal := resp.GetBody().GetBalance(); bal != nil { - var d accounting.Decimal - d.ReadFromV2(*bal) - - res.setAmount(&d) + bal := resp.GetBody().GetBalance() + if bal == nil { + cc.err = errors.New("missing balance field") + return } + + var d accounting.Decimal + + cc.err = d.ReadFromV2(*bal) + if cc.err != nil { + cc.err = fmt.Errorf("invalid balance: %w", cc.err) + return + } + + res.setAmount(&d) } // process call diff --git a/client/netmap.go b/client/netmap.go index 096ee0e..02f9023 100644 --- a/client/netmap.go +++ b/client/netmap.go @@ -95,7 +95,11 @@ func (c *Client) EndpointInfo(ctx context.Context, prm PrmEndpointInfo) (*ResEnd var ver version.Version if v2ver := body.GetVersion(); v2ver != nil { - ver.ReadFromV2(*v2ver) + cc.err = ver.ReadFromV2(*v2ver) + if cc.err != nil { + cc.err = fmt.Errorf("invalid version: %w", cc.err) + return + } } res.setLatestVersion(&ver) diff --git a/container/container_test.go b/container/container_test.go index c5ed84e..06b8f7e 100644 --- a/container/container_test.go +++ b/container/container_test.go @@ -63,7 +63,7 @@ func TestContainer_Init(t *testing.T) { require.NotNil(t, verV2) var ver version.Version - ver.ReadFromV2(*verV2) + require.NoError(t, ver.ReadFromV2(*verV2)) require.Equal(t, version.Current(), ver) @@ -345,7 +345,7 @@ func TestCalculateSignature(t *testing.T) { sig.WriteToV2(&msg) var sig2 neofscrypto.Signature - sig2.ReadFromV2(msg) + require.NoError(t, sig2.ReadFromV2(msg)) require.True(t, container.VerifySignature(sig2, val)) } diff --git a/crypto/crypto_test.go b/crypto/crypto_test.go index 5759517..90b36bf 100644 --- a/crypto/crypto_test.go +++ b/crypto/crypto_test.go @@ -39,7 +39,7 @@ func TestSignature(t *testing.T) { s.WriteToV2(&m) - s.ReadFromV2(m) + require.NoError(t, s.ReadFromV2(m)) valid := s.Verify(data) require.True(t, valid, "type %T", signer) diff --git a/crypto/signature.go b/crypto/signature.go index 237176b..bb21867 100644 --- a/crypto/signature.go +++ b/crypto/signature.go @@ -1,6 +1,7 @@ package neofscrypto import ( + "errors" "fmt" "github.com/nspcc-dev/neofs-api-go/v2/refs" @@ -16,11 +17,29 @@ import ( // _ = Signature(refs.Signature{}) // not recommended type Signature refs.Signature -// ReadFromV2 reads Signature from the refs.Signature message. +// ReadFromV2 reads Signature from the refs.Signature message. Checks if the +// message conforms to NeoFS API V2 protocol. // // See also WriteToV2. -func (x *Signature) ReadFromV2(m refs.Signature) { +func (x *Signature) ReadFromV2(m refs.Signature) error { + if len(m.GetKey()) == 0 { + return errors.New("missing public key") + } else if len(m.GetSign()) == 0 { + return errors.New("missing signature") + } + + switch m.GetScheme() { + default: + return fmt.Errorf("unsupported scheme %v", m.GetSign()) + case + refs.ECDSA_SHA512, + refs.ECDSA_RFC6979_SHA256, + refs.ECDSA_RFC6979_SHA256_WALLET_CONNECT: + } + *x = Signature(m) + + return nil } // WriteToV2 writes Signature to the refs.Signature message. diff --git a/object/fmt.go b/object/fmt.go index 4339a04..6d99ec6 100644 --- a/object/fmt.go +++ b/object/fmt.go @@ -127,9 +127,8 @@ func (o *Object) VerifyIDSignature() bool { } var sig neofscrypto.Signature - sig.ReadFromV2(*sigV2) - return sig.Verify(idV2.StableMarshal(nil)) + return sig.ReadFromV2(*sigV2) == nil && sig.Verify(idV2.StableMarshal(nil)) } // SetIDWithSignature sets object identifier and signature. diff --git a/object/object.go b/object/object.go index ada5ec7..0475ded 100644 --- a/object/object.go +++ b/object/object.go @@ -117,7 +117,7 @@ func (o *Object) Signature() *neofscrypto.Signature { } var sig neofscrypto.Signature - sig.ReadFromV2(*sigv2) + sig.ReadFromV2(*sigv2) // FIXME(@cthulhu-rider): #226 handle error return &sig } @@ -149,7 +149,7 @@ func (o *Object) SetPayload(v []byte) { func (o *Object) Version() *version.Version { var ver version.Version if verV2 := (*object.Object)(o).GetHeader().GetVersion(); verV2 != nil { - ver.ReadFromV2(*verV2) + ver.ReadFromV2(*verV2) // FIXME(@cthulhu-rider): #226 handle error } return &ver } @@ -248,7 +248,7 @@ func (o *Object) PayloadChecksum() (checksum.Checksum, bool) { v2 := (*object.Object)(o) if hash := v2.GetHeader().GetPayloadHash(); hash != nil { - v.ReadFromV2(*hash) + v.ReadFromV2(*hash) // FIXME(@cthulhu-rider): #226 handle error return v, true } @@ -278,7 +278,7 @@ func (o *Object) PayloadHomomorphicHash() (checksum.Checksum, bool) { v2 := (*object.Object)(o) if hash := v2.GetHeader().GetHomomorphicHash(); hash != nil { - v.ReadFromV2(*hash) + v.ReadFromV2(*hash) // FIXME(@cthulhu-rider): #226 handle error return v, true } diff --git a/reputation/trust.go b/reputation/trust.go index 428ce71..09c045e 100644 --- a/reputation/trust.go +++ b/reputation/trust.go @@ -394,9 +394,8 @@ func (x GlobalTrust) VerifySignature() bool { } var sig neofscrypto.Signature - sig.ReadFromV2(*sigV2) - return sig.Verify(x.m.GetBody().StableMarshal(nil)) + return sig.ReadFromV2(*sigV2) == nil && sig.Verify(x.m.GetBody().StableMarshal(nil)) } // Marshal encodes GlobalTrust into a binary format of the NeoFS API protocol diff --git a/session/common.go b/session/common.go index 0eb6254..7346cd0 100644 --- a/session/common.go +++ b/session/common.go @@ -178,10 +178,9 @@ func (x commonData) verifySignature(w contextWriter) bool { } var sig neofscrypto.Signature - sig.ReadFromV2(x.sig) // TODO: (#233) check owner<->key relation - return sig.Verify(x.signedData(w)) + return sig.ReadFromV2(x.sig) == nil && sig.Verify(x.signedData(w)) } func (x commonData) marshal(w contextWriter) []byte { diff --git a/session/container.go b/session/container.go index 9a245a4..29773a7 100644 --- a/session/container.go +++ b/session/container.go @@ -210,7 +210,6 @@ func (x Container) VerifySessionDataSignature(data, signature []byte) bool { sigV2.SetSign(signature) var sig neofscrypto.Signature - sig.ReadFromV2(sigV2) - return sig.Verify(data) + return sig.ReadFromV2(sigV2) == nil && sig.Verify(data) } diff --git a/storagegroup/storagegroup.go b/storagegroup/storagegroup.go index a0d3ff1..76bce38 100644 --- a/storagegroup/storagegroup.go +++ b/storagegroup/storagegroup.go @@ -1,6 +1,7 @@ package storagegroup import ( + "errors" "fmt" "strconv" @@ -23,11 +24,55 @@ import ( // _ = StorageGroup(storagegroup.StorageGroup) // not recommended type StorageGroup storagegroup.StorageGroup +// reads StorageGroup from the storagegroup.StorageGroup message. If checkFieldPresence is set, +// returns an error on absence of any protocol-required field. +func (sg *StorageGroup) readFromV2(m storagegroup.StorageGroup, checkFieldPresence bool) error { + var err error + + h := m.GetValidationHash() + if h != nil { + err = new(checksum.Checksum).ReadFromV2(*h) + if err != nil { + return fmt.Errorf("invalid hash: %w", err) + } + } else if checkFieldPresence { + return errors.New("missing hash") + } + + members := m.GetMembers() + if len(members) > 0 { + var member oid.ID + mMembers := make(map[oid.ID]struct{}, len(members)) + var exits bool + + for i := range members { + err = member.ReadFromV2(members[i]) + if err != nil { + return fmt.Errorf("invalid member: %w", err) + } + + _, exits = mMembers[member] + if exits { + return fmt.Errorf("duplicated member %s", member) + } + + mMembers[member] = struct{}{} + } + } else if checkFieldPresence { + return errors.New("missing members") + } + + *sg = StorageGroup(m) + + return nil +} + // ReadFromV2 reads StorageGroup from the storagegroup.StorageGroup message. +// Checks if the message conforms to NeoFS API V2 protocol. // // See also WriteToV2. -func (sg *StorageGroup) ReadFromV2(m storagegroup.StorageGroup) { - *sg = StorageGroup(m) +func (sg *StorageGroup) ReadFromV2(m storagegroup.StorageGroup) error { + return sg.readFromV2(m, true) } // WriteToV2 writes StorageGroup to the storagegroup.StorageGroup message. @@ -68,7 +113,7 @@ func (sg *StorageGroup) SetValidationDataSize(epoch uint64) { func (sg StorageGroup) ValidationDataHash() (v checksum.Checksum, isSet bool) { v2 := (storagegroup.StorageGroup)(sg) if checksumV2 := v2.GetValidationHash(); checksumV2 != nil { - v.ReadFromV2(*checksumV2) + v.ReadFromV2(*checksumV2) // FIXME(@cthulhu-rider): #226 handle error isSet = true } @@ -174,7 +219,7 @@ func (sg *StorageGroup) Unmarshal(data []byte) error { return err } - return formatCheck(v2) + return sg.readFromV2(*v2, false) } // MarshalJSON encodes StorageGroup to protobuf JSON format. @@ -195,20 +240,7 @@ func (sg *StorageGroup) UnmarshalJSON(data []byte) error { return err } - return formatCheck(v2) -} - -func formatCheck(v2 *storagegroup.StorageGroup) error { - var oID oid.ID - - for _, m := range v2.GetMembers() { - err := oID.ReadFromV2(m) - if err != nil { - return err - } - } - - return nil + return sg.readFromV2(*v2, false) } // ReadFromObject assemble StorageGroup from a regular diff --git a/storagegroup/storagegroup_test.go b/storagegroup/storagegroup_test.go index 8c67c73..8e65718 100644 --- a/storagegroup/storagegroup_test.go +++ b/storagegroup/storagegroup_test.go @@ -49,13 +49,7 @@ func TestStorageGroup_ReadFromV2(t *testing.T) { v2 storagegroupV2.StorageGroup ) - x.ReadFromV2(v2) - - require.Zero(t, x.ExpirationEpoch()) - require.Zero(t, x.ValidationDataSize()) - _, set := x.ValidationDataHash() - require.False(t, set) - require.Zero(t, x.Members()) + require.Error(t, x.ReadFromV2(v2)) }) t.Run("from non-zero", func(t *testing.T) { @@ -72,13 +66,13 @@ func TestStorageGroup_ReadFromV2(t *testing.T) { mm := v2.GetMembers() hashV2 := v2.GetValidationHash() - x.ReadFromV2(*v2) + require.NoError(t, x.ReadFromV2(*v2)) require.Equal(t, epoch, x.ExpirationEpoch()) require.Equal(t, size, x.ValidationDataSize()) var hash checksum.Checksum - hash.ReadFromV2(*hashV2) + require.NoError(t, hash.ReadFromV2(*hashV2)) h, set := x.ValidationDataHash() require.True(t, set) require.Equal(t, hash, h) @@ -143,7 +137,7 @@ func TestStorageGroup_WriteToV2(t *testing.T) { require.Equal(t, x.ValidationDataSize(), v2.GetValidationDataSize()) var hash checksum.Checksum - hash.ReadFromV2(*v2.GetValidationHash()) + require.NoError(t, hash.ReadFromV2(*v2.GetValidationHash())) h, set := x.ValidationDataHash() require.True(t, set) diff --git a/version/version.go b/version/version.go index a31f1c4..296e9df 100644 --- a/version/version.go +++ b/version/version.go @@ -55,11 +55,13 @@ func (v Version) WriteToV2(m *refs.Version) { *m = (refs.Version)(v) } -// ReadFromV2 reads Version from the refs.Version message. +// ReadFromV2 reads Version from the refs.Version message. Checks if the message +// conforms to NeoFS API V2 protocol. // // See also WriteToV2. -func (v *Version) ReadFromV2(m refs.Version) { +func (v *Version) ReadFromV2(m refs.Version) error { *v = Version(m) + return nil } // String implements fmt.Stringer.