diff --git a/pkg/bwagreement/server.go b/pkg/bwagreement/server.go index 4203c35a6..fd806d641 100644 --- a/pkg/bwagreement/server.go +++ b/pkg/bwagreement/server.go @@ -62,8 +62,9 @@ func (s *Server) BandwidthAgreements(ctx context.Context, ba *pb.RenterBandwidth Status: pb.AgreementsSummary_FAIL, } - if err = s.verifySignature(ctx, ba); err != nil { - return reply, err + // storagenode signature is empty + if len(ba.GetSignature()) == 0 { + return reply, BwAgreementError.New("Invalid Storage Node Signature length in the RenterBandwidthAllocation") } rbad := &pb.RenterBandwidthAllocation_Data{} @@ -77,10 +78,19 @@ func (s *Server) BandwidthAgreements(ctx context.Context, ba *pb.RenterBandwidth return reply, BwAgreementError.New("Failed to unmarshal PayerBandwidthAllocation: %+v", err) } + // satellite signature is empty + if len(pba.GetSignature()) == 0 { + return reply, BwAgreementError.New("Invalid Satellite Signature length in the PayerBandwidthAllocation") + } + if len(pbad.SerialNumber) == 0 { return reply, BwAgreementError.New("Invalid SerialNumber in the PayerBandwidthAllocation") } + if err = s.verifySignature(ctx, ba); err != nil { + return reply, err + } + serialNum := pbad.GetSerialNumber() + rbad.StorageNodeId.String() // get and check expiration @@ -132,16 +142,25 @@ func (s *Server) verifySignature(ctx context.Context, ba *pb.RenterBandwidthAllo return peertls.ErrUnsupportedKey.New("%T", pubkey) } + signatureLength := k.Curve.Params().P.BitLen() / 8 + if len(ba.GetSignature()) < signatureLength { + return BwAgreementError.New("Invalid Renter's Signature Length") + } // verify Renter's (uplink) signature if ok := cryptopasta.Verify(ba.GetData(), ba.GetSignature(), k); !ok { return BwAgreementError.New("Failed to verify Renter's Signature") } + // satellite public key k, ok = s.pkey.(*ecdsa.PublicKey) if !ok { return peertls.ErrUnsupportedKey.New("%T", s.pkey) } + signatureLength = k.Curve.Params().P.BitLen() / 8 + if len(rbad.GetPayerAllocation().GetSignature()) < signatureLength { + return BwAgreementError.New("Inavalid Payer's Signature Length") + } // verify Payer's (satellite) signature if ok := cryptopasta.Verify(rbad.GetPayerAllocation().GetData(), rbad.GetPayerAllocation().GetSignature(), k); !ok { return BwAgreementError.New("Failed to verify Payer's Signature") diff --git a/pkg/bwagreement/test/server_test.go b/pkg/bwagreement/test/server_test.go index 4c9853997..c05e9b162 100644 --- a/pkg/bwagreement/test/server_test.go +++ b/pkg/bwagreement/test/server_test.go @@ -79,7 +79,7 @@ func TestSameSerialNumberBandwidthAgreements(t *testing.T) { }) } -func TestInvalidBandwidthAgreements(t *testing.T) { +func TestManipulatedBandwidthAgreements(t *testing.T) { satellitedbtest.Run(t, func(t *testing.T, db satellite.DB) { ctx := testcontext.New(t) defer ctx.Cleanup() @@ -116,11 +116,34 @@ func TestInvalidBandwidthAgreements(t *testing.T) { /* manipulate PayerBandwidthAllocation -> invalid signature */ /* self signed. Storage node sends a self signed bwagreement to get a higher payout */ + }) +} - /* malicious storage node would like to force a crash */ +func TestInvalidBandwidthAgreements(t *testing.T) { + satellitedbtest.Run(t, func(t *testing.T, db satellite.DB) { + ctx := testcontext.New(t) + defer ctx.Cleanup() - /* corrupted signature. Storage node sends an corrupted signuature to force a satellite crash */ + satellitePubKey, satellitePrivKey, uplinkPrivKey := generateKeys(ctx, t) + server := bwagreement.NewServer(db.BandwidthAgreement(), zap.NewNop(), satellitePubKey) + pba, err := GeneratePayerBandwidthAllocation(pb.PayerBandwidthAllocation_GET, satellitePrivKey, uplinkPrivKey, false) + assert.NoError(t, err) + + rba, err := GenerateRenterBandwidthAllocation(pba, teststorj.NodeIDFromString("Storage node 1"), uplinkPrivKey) + assert.NoError(t, err) + + /* Make sure the bwagreement we are using as bluleprint is valid and avoid false positives that way. */ + reply, err := server.BandwidthAgreements(ctx, rba) + assert.NoError(t, err) + assert.Equal(t, pb.AgreementsSummary_OK, reply.Status) + + /* Storage node sends an corrupted signuature to force a satellite crash */ + rba.Signature = []byte("invalid") + + reply, err = server.BandwidthAgreements(ctx, rba) + assert.EqualError(t, err, "bwagreement error: Invalid Renter's Signature Length") + assert.Equal(t, pb.AgreementsSummary_FAIL, reply.Status) }) }