Skip to content

Commit

Permalink
Merge pull request #520 from sandrask/issue-517
Browse files Browse the repository at this point in the history
feat: Add max operation hash length protocol parameter
  • Loading branch information
sandrask authored Dec 3, 2020
2 parents 0bbe2c4 + b5b935b commit bad37e2
Show file tree
Hide file tree
Showing 15 changed files with 200 additions and 91 deletions.
2 changes: 2 additions & 0 deletions pkg/api/protocol/protocol.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,8 @@ type Protocol struct {
// MaxOperationSize is maximum operation size in bytes (used to reject operations before parsing them)
// It has to be greater than max delta size (big) + max proof size (medium) + other small values (operation type, suffix-data)
MaxOperationSize uint `json:"maxOperationSize"`
// MaxOperationHashLength is maximum operation hash length
MaxOperationHashLength uint `json:"maxOperationHashLength"`
// MaxDeltaSize is maximum size of operation's delta property.
MaxDeltaSize uint `json:"maxDeltaSize"`
// MaxProofSize is maximum size of operation's proof property.
Expand Down
1 change: 1 addition & 0 deletions pkg/mocks/protocol.go
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,7 @@ func GetDefaultProtocolParameters() protocol.Protocol {
MultihashAlgorithm: sha2_256,
MaxOperationCount: 2,
MaxOperationSize: MaxOperationByteSize,
MaxOperationHashLength: 100,
MaxDeltaSize: MaxDeltaByteSize,
MaxProofSize: MaxProofByteSize,
MaxCasURILength: 100,
Expand Down
1 change: 1 addition & 0 deletions pkg/processor/processor_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1265,6 +1265,7 @@ func newMockProtocolClient() *mocks.MockProtocolClient {
MultihashAlgorithm: sha2_512,
MaxOperationCount: 2,
MaxOperationSize: mocks.MaxOperationByteSize,
MaxOperationHashLength: 100,
MaxDeltaSize: mocks.MaxDeltaByteSize,
MaxProofSize: 700, // has to be increased from 500 since we now use sha2_512
MaxCasURILength: 100,
Expand Down
1 change: 1 addition & 0 deletions pkg/versions/0_1/operationapplier/operationapplier_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ var (
MultihashAlgorithm: sha2_256,
MaxOperationCount: 2,
MaxOperationSize: 2000,
MaxOperationHashLength: 100,
MaxProofSize: 500,
MaxDeltaSize: 1000,
MaxCasURILength: 100,
Expand Down
26 changes: 17 additions & 9 deletions pkg/versions/0_1/operationparser/create.go
Original file line number Diff line number Diff line change
Expand Up @@ -99,13 +99,25 @@ func (p *Parser) ValidateDelta(delta *model.DeltaModel) error {
}
}

if !hashing.IsComputedUsingMultihashAlgorithm(delta.UpdateCommitment, uint64(p.MultihashAlgorithm)) {
return fmt.Errorf("next update commitment hash is not computed with the required supported hash algorithm: %d", p.MultihashAlgorithm)
if err := p.validateMultihash(delta.UpdateCommitment, "update commitment"); err != nil {
return err
}

return p.validateDeltaSize(delta)
}

func (p *Parser) validateMultihash(mh, alias string) error {
if len(mh) > int(p.MaxOperationHashLength) {
return fmt.Errorf("%s length[%d] exceeds maximum hash length[%d]", alias, len(mh), p.MaxOperationHashLength)
}

if !hashing.IsComputedUsingMultihashAlgorithm(mh, uint64(p.MultihashAlgorithm)) {
return fmt.Errorf("%s is not computed with the required hash algorithm: %d", alias, p.MultihashAlgorithm)
}

return nil
}

func (p *Parser) validateDeltaSize(delta *model.DeltaModel) error {
canonicalDelta, err := canonicalizer.MarshalCanonical(delta)
if err != nil {
Expand Down Expand Up @@ -135,15 +147,11 @@ func (p *Parser) ValidateSuffixData(suffixData *model.SuffixDataModel) error {
return errors.New("missing suffix data")
}

if !hashing.IsComputedUsingMultihashAlgorithm(suffixData.RecoveryCommitment, uint64(p.MultihashAlgorithm)) {
return fmt.Errorf("next recovery commitment hash is not computed with the required supported hash algorithm: %d", p.MultihashAlgorithm)
if err := p.validateMultihash(suffixData.RecoveryCommitment, "recovery commitment"); err != nil {
return err
}

if !hashing.IsComputedUsingMultihashAlgorithm(suffixData.DeltaHash, uint64(p.MultihashAlgorithm)) {
return fmt.Errorf("patch data hash is not computed with the required supported hash algorithm: %d", p.MultihashAlgorithm)
}

return nil
return p.validateMultihash(suffixData.DeltaHash, "delta hash")
}

func (p *Parser) validateCreateRequest(create *model.CreateRequest) error {
Expand Down
67 changes: 52 additions & 15 deletions pkg/versions/0_1/operationparser/create_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,13 +22,16 @@ import (
"github.com/trustbloc/sidetree-core-go/pkg/versions/0_1/model"
)

const invalid = "invalid"
const (
invalid = "invalid"
)

func TestParseCreateOperation(t *testing.T) {
p := protocol.Protocol{
MaxDeltaSize: maxDeltaSize,
MultihashAlgorithm: sha2_256,
Patches: []string{"replace", "add-public-keys", "remove-public-keys", "add-services", "remove-services", "ietf-json-patch"},
MaxOperationHashLength: 100,
MaxDeltaSize: maxDeltaSize,
MultihashAlgorithm: sha2_256,
Patches: []string{"replace", "add-public-keys", "remove-public-keys", "add-services", "remove-services", "ietf-json-patch"},
}

parser := New(p)
Expand Down Expand Up @@ -78,7 +81,7 @@ func TestParseCreateOperation(t *testing.T) {

op, err := parser.ParseCreateOperation(request, true)
require.Error(t, err)
require.Contains(t, err.Error(), "next recovery commitment hash is not computed with the required supported hash algorithm: 18")
require.Contains(t, err.Error(), "recovery commitment is not computed with the required hash algorithm: 18")
require.Nil(t, op)
})
t.Run("missing delta", func(t *testing.T) {
Expand Down Expand Up @@ -158,7 +161,8 @@ func TestParseCreateOperation(t *testing.T) {

func TestValidateSuffixData(t *testing.T) {
p := protocol.Protocol{
MultihashAlgorithm: sha2_256,
MaxOperationHashLength: maxHashLength,
MultihashAlgorithm: sha2_256,
}

parser := New(p)
Expand All @@ -170,7 +174,7 @@ func TestValidateSuffixData(t *testing.T) {
suffixData.DeltaHash = ""
err = parser.ValidateSuffixData(suffixData)
require.Error(t, err)
require.Contains(t, err.Error(), "patch data hash is not computed with the required supported hash algorithm")
require.Contains(t, err.Error(), "delta hash is not computed with the required hash algorithm: 18")
})
t.Run("invalid next recovery commitment hash", func(t *testing.T) {
suffixData, err := getSuffixData()
Expand All @@ -179,17 +183,31 @@ func TestValidateSuffixData(t *testing.T) {
suffixData.RecoveryCommitment = ""
err = parser.ValidateSuffixData(suffixData)
require.Error(t, err)
require.Contains(t, err.Error(), "next recovery commitment hash is not computed with the required supported hash algorithm")
require.Contains(t, err.Error(), "recovery commitment is not computed with the required hash algorithm: 18")
})
t.Run("recovery commitment exceeds maximum hash length", func(t *testing.T) {
lowHashLength := protocol.Protocol{
MaxOperationHashLength: 10,
MultihashAlgorithm: sha2_256,
}

suffixData, err := getSuffixData()
require.NoError(t, err)

err = New(lowHashLength).ValidateSuffixData(suffixData)
require.Error(t, err)
require.Contains(t, err.Error(), "recovery commitment length[46] exceeds maximum hash length[10]")
})
}

func TestValidateDelta(t *testing.T) {
patches := []string{"add-public-keys", "remove-public-keys", "add-services", "remove-services", "ietf-json-patch"}

p := protocol.Protocol{
MaxDeltaSize: maxDeltaSize,
MultihashAlgorithm: sha2_256,
Patches: patches,
MaxOperationHashLength: maxHashLength,
MaxDeltaSize: maxDeltaSize,
MultihashAlgorithm: sha2_256,
Patches: patches,
}

parser := New(p)
Expand All @@ -204,9 +222,10 @@ func TestValidateDelta(t *testing.T) {

t.Run("error - delta exceeds max delta size ", func(t *testing.T) {
parserWithLowMaxDeltaSize := New(protocol.Protocol{
MaxDeltaSize: 50,
MultihashAlgorithm: sha2_256,
Patches: patches,
MaxOperationHashLength: maxHashLength,
MaxDeltaSize: 50,
MultihashAlgorithm: sha2_256,
Patches: patches,
})

delta, err := getDelta()
Expand All @@ -225,8 +244,26 @@ func TestValidateDelta(t *testing.T) {
err = parser.ValidateDelta(delta)
require.Error(t, err)
require.Contains(t, err.Error(),
"next update commitment hash is not computed with the required supported hash algorithm")
"update commitment is not computed with the required hash algorithm: 18")
})

t.Run("update commitment exceeds maximum hash length", func(t *testing.T) {
lowMaxHashLength := protocol.Protocol{
MaxOperationHashLength: 10,
MaxDeltaSize: 50,
MultihashAlgorithm: sha2_256,
Patches: patches,
}

delta, err := getDelta()
require.NoError(t, err)

err = New(lowMaxHashLength).ValidateDelta(delta)
require.Error(t, err)
require.Contains(t, err.Error(),
"update commitment length[46] exceeds maximum hash length[10]")
})

t.Run("missing patches", func(t *testing.T) {
delta, err := getDelta()
require.NoError(t, err)
Expand Down
2 changes: 1 addition & 1 deletion pkg/versions/0_1/operationparser/deactivate.go
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ func (p *Parser) ParseSignedDataForDeactivate(compactJWS string) (*model.Deactiv
}

if err := p.validateSigningKey(signedData.RecoveryKey, p.KeyAlgorithms); err != nil {
return nil, fmt.Errorf("signed data for deactivate: %s", err.Error())
return nil, fmt.Errorf("validate signed data for deactivate: %s", err.Error())
}

return signedData, nil
Expand Down
2 changes: 1 addition & 1 deletion pkg/versions/0_1/operationparser/deactivate_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ func TestParseDeactivateOperation(t *testing.T) {

op, err := parser.ParseDeactivateOperation(request, false)
require.Error(t, err)
require.Contains(t, err.Error(), "signed data for deactivate: key algorithm 'crv' is not in the allowed list [other]")
require.Contains(t, err.Error(), "validate signed data for deactivate: key algorithm 'crv' is not in the allowed list [other]")
require.Nil(t, op)
})
}
Expand Down
16 changes: 9 additions & 7 deletions pkg/versions/0_1/operationparser/operation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,19 +19,21 @@ const (
namespace = "did:sidetree"

maxOperationSize = 2000
maxHashLength = 100
maxDeltaSize = 1000
maxProofSize = 500
)

func TestGetOperation(t *testing.T) {
p := protocol.Protocol{
MaxOperationSize: maxOperationSize,
MaxDeltaSize: maxDeltaSize,
MaxProofSize: maxProofSize,
MultihashAlgorithm: sha2_256,
SignatureAlgorithms: []string{"alg"},
KeyAlgorithms: []string{"crv"},
Patches: []string{"add-public-keys", "remove-public-keys", "add-services", "remove-services", "ietf-json-patch"},
MaxOperationSize: maxOperationSize,
MaxOperationHashLength: maxHashLength,
MaxDeltaSize: maxDeltaSize,
MaxProofSize: maxProofSize,
MultihashAlgorithm: sha2_256,
SignatureAlgorithms: []string{"alg"},
KeyAlgorithms: []string{"crv"},
Patches: []string{"add-public-keys", "remove-public-keys", "add-services", "remove-services", "ietf-json-patch"},
}

parser := New(p)
Expand Down
9 changes: 4 additions & 5 deletions pkg/versions/0_1/operationparser/recover.go
Original file line number Diff line number Diff line change
Expand Up @@ -97,13 +97,12 @@ func (p *Parser) validateSignedDataForRecovery(signedData *model.RecoverSignedDa
return err
}

code := uint64(p.MultihashAlgorithm)
if !hashing.IsComputedUsingMultihashAlgorithm(signedData.RecoveryCommitment, code) {
return fmt.Errorf("next recovery commitment hash is not computed with the required hash algorithm: %d", code)
if err := p.validateMultihash(signedData.RecoveryCommitment, "recovery commitment"); err != nil {
return err
}

if !hashing.IsComputedUsingMultihashAlgorithm(signedData.DeltaHash, code) {
return fmt.Errorf("patch data hash is not computed with the required hash algorithm: %d", code)
if err := p.validateMultihash(signedData.DeltaHash, "delta hash"); err != nil {
return err
}

return validateCommitment(signedData.RecoveryKey, p.MultihashAlgorithm, signedData.RecoveryCommitment)
Expand Down
37 changes: 26 additions & 11 deletions pkg/versions/0_1/operationparser/recover_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,12 +29,13 @@ const (

func TestParseRecoverOperation(t *testing.T) {
p := protocol.Protocol{
MaxDeltaSize: maxDeltaSize,
MaxProofSize: maxProofSize,
MultihashAlgorithm: sha2_256,
SignatureAlgorithms: []string{"alg"},
KeyAlgorithms: []string{"crv"},
Patches: []string{"add-public-keys", "remove-public-keys", "add-services", "remove-services", "ietf-json-patch"},
MaxOperationHashLength: maxHashLength,
MaxDeltaSize: maxDeltaSize,
MaxProofSize: maxProofSize,
MultihashAlgorithm: sha2_256,
SignatureAlgorithms: []string{"alg"},
KeyAlgorithms: []string{"crv"},
Patches: []string{"add-public-keys", "remove-public-keys", "add-services", "remove-services", "ietf-json-patch"},
}

parser := New(p)
Expand Down Expand Up @@ -145,7 +146,7 @@ func TestParseRecoverOperation(t *testing.T) {

op, err := parser.ParseRecoverOperation(request, false)
require.Error(t, err)
require.Contains(t, err.Error(), "signed data for recovery: missing signing key")
require.Contains(t, err.Error(), "validate signed data for recovery: missing signing key")
require.Nil(t, op)
})

Expand Down Expand Up @@ -194,8 +195,9 @@ func TestParseRecoverOperation(t *testing.T) {

func TestValidateSignedDataForRecovery(t *testing.T) {
p := protocol.Protocol{
MultihashAlgorithm: sha2_256,
KeyAlgorithms: []string{"crv"},
MaxOperationHashLength: maxHashLength,
MultihashAlgorithm: sha2_256,
KeyAlgorithms: []string{"crv"},
}

parser := New(p)
Expand All @@ -213,14 +215,27 @@ func TestValidateSignedDataForRecovery(t *testing.T) {
signed.DeltaHash = ""
err := parser.validateSignedDataForRecovery(signed)
require.Error(t, err)
require.Contains(t, err.Error(), "patch data hash is not computed with the required hash algorithm")
require.Contains(t, err.Error(), "delta hash is not computed with the required hash algorithm: 18")
})
t.Run("invalid next recovery commitment hash", func(t *testing.T) {
signed := getSignedDataForRecovery()
signed.RecoveryCommitment = ""
err := parser.validateSignedDataForRecovery(signed)
require.Error(t, err)
require.Contains(t, err.Error(), "next recovery commitment hash is not computed with the required hash algorithm")
require.Contains(t, err.Error(), "recovery commitment is not computed with the required hash algorithm: 18")
})
t.Run("recovery commitment exceeds maximum hash length", func(t *testing.T) {
lowMaxHashLength := protocol.Protocol{
MaxOperationHashLength: 10,
MultihashAlgorithm: sha2_256,
KeyAlgorithms: []string{"crv"},
}

signed := getSignedDataForRecovery()

err := New(lowMaxHashLength).validateSignedDataForRecovery(signed)
require.Error(t, err)
require.Contains(t, err.Error(), "recovery commitment length[46] exceeds maximum hash length[10]")
})
}

Expand Down
12 changes: 3 additions & 9 deletions pkg/versions/0_1/operationparser/update.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ import (
"fmt"

"github.com/trustbloc/sidetree-core-go/pkg/api/operation"
"github.com/trustbloc/sidetree-core-go/pkg/hashing"
"github.com/trustbloc/sidetree-core-go/pkg/versions/0_1/model"
)

Expand Down Expand Up @@ -83,7 +82,7 @@ func (p *Parser) ParseSignedDataForUpdate(compactJWS string) (*model.UpdateSigne
}

if err := p.validateSignedDataForUpdate(schema); err != nil {
return nil, err
return nil, fmt.Errorf("validate signed data for update: %s", err.Error())
}

return schema, nil
Expand All @@ -103,13 +102,8 @@ func (p *Parser) validateUpdateRequest(update *model.UpdateRequest) error {

func (p *Parser) validateSignedDataForUpdate(signedData *model.UpdateSignedDataModel) error {
if err := p.validateSigningKey(signedData.UpdateKey, p.KeyAlgorithms); err != nil {
return fmt.Errorf("signed data for update: %s", err.Error())
}

code := uint64(p.MultihashAlgorithm)
if !hashing.IsComputedUsingMultihashAlgorithm(signedData.DeltaHash, code) {
return fmt.Errorf("delta hash is not computed with the required multihash algorithm: %d", code)
return err
}

return nil
return p.validateMultihash(signedData.DeltaHash, "delta hash")
}
Loading

0 comments on commit bad37e2

Please sign in to comment.