x: deprecate and rename MainSigner (#1065)

`MainSigner` function returns always the first condition as defined in
the transaction. Using this function can introduce a security hole. One
must never assume the order of transaction signatures, because those are
not part of signed content. Order of signatures in transaction can be
altered at any time.

CHANGELOG.md

@@ -2,6 +2,8 @@
 
 ## HEAD
 
+- deprecate and rename `x.MainSigner` to `x.AnySigner` to better describe
+  provided functionality.
 
 ## 0.22.0
 

cmd/bnsd/x/blueaccount/handler.go

@@ -91,7 +91,7 @@ func (h *registerDomainHandler) Deliver(ctx weave.Context, db weave.KVStore, tx
 
 	owner := msg.Owner
 	if len(owner) == 0 {
-		owner = x.MainSigner(ctx, h.auth).Address()
+		owner = x.AnySigner(ctx, h.auth).Address()
 	}
 
 	now, err := weave.BlockTime(ctx)

cmd/bnsd/x/username/handlers.go

@@ -44,7 +44,7 @@ func (h *registerTokenHandler) Deliver(ctx weave.Context, db weave.KVStore, tx w
 		return nil, err
 	}
 
-	owner := x.MainSigner(ctx, h.auth).Address()
+	owner := x.AnySigner(ctx, h.auth).Address()
 	if len(owner) == 0 {
 		return nil, errors.Wrap(errors.ErrUnauthorized, "message must be signed")
 	}

go.mod

@@ -29,3 +29,5 @@ require (
 	golang.org/x/crypto v0.0.0-20190513172903-22d7a77e9e5f
 	google.golang.org/grpc v1.21.0 // indirect
 )
+
+go 1.13

x/auth.go

@@ -61,13 +61,20 @@ func GetAddresses(ctx weave.Context, auth Authenticator) []weave.Address {
 	return addrs
 }
 
-// MainSigner returns the first permission if any, otherwise nil
-func MainSigner(ctx weave.Context, auth Authenticator) weave.Condition {
-	signers := auth.GetConditions(ctx)
-	if len(signers) == 0 {
-		return nil
+// AnySigner returns a permission or nil.
+//
+// This function returns always the first condition as defined in the
+// transaction. Using this function can introduce a security hole. One must
+// never assume the order of transaction signatures, because those are not part
+// of signed content. Order of signatures in transaction can be altered at any
+// time.
+//
+// This function is deprecated and must not be used for new implementations.
+func AnySigner(ctx weave.Context, auth Authenticator) weave.Condition {
+	if sigs := auth.GetConditions(ctx); len(sigs) > 0 {
+		return sigs[0]
 	}
-	return signers[0]
+	return nil
 }
 
 // HasAllAddresses returns true if all elements in required are

x/auth_test.go

@@ -20,7 +20,7 @@ func TestAuth(t *testing.T) {
 	cases := map[string]struct {
 		ctx          weave.Context
 		auth         Authenticator
-		mainSigner   weave.Condition
+		aSigner      weave.Condition
 		wantInCtx    weave.Condition
 		wantNotInCtx weave.Condition
 		wantAll      []weave.Condition
@@ -33,7 +33,7 @@ func TestAuth(t *testing.T) {
 		"signer a": {
 			ctx:          context.Background(),
 			auth:         &weavetest.Auth{Signer: a},
-			mainSigner:   a,
+			aSigner:      a,
 			wantInCtx:    a,
 			wantNotInCtx: b,
 			wantAll:      []weave.Condition{a},
@@ -43,15 +43,15 @@ func TestAuth(t *testing.T) {
 			auth: ChainAuth(
 				&weavetest.Auth{Signer: b},
 				&weavetest.Auth{Signer: a}),
-			mainSigner:   b,
+			aSigner:      b,
 			wantInCtx:    b,
 			wantNotInCtx: c,
 			wantAll:      []weave.Condition{b, a},
 		},
 		"ctxAuth checks what is set by same key": {
 			ctx:          ctx1.SetConditions(context.Background(), a, b),
 			auth:         ctx1,
-			mainSigner:   a,
+			aSigner:      a,
 			wantInCtx:    b,
 			wantNotInCtx: c,
 			wantAll:      []weave.Condition{a, b},
@@ -65,7 +65,7 @@ func TestAuth(t *testing.T) {
 
 	for testName, tc := range cases {
 		t.Run(testName, func(t *testing.T) {
-			assert.Equal(t, tc.mainSigner, MainSigner(tc.ctx, tc.auth))
+			assert.Equal(t, tc.aSigner, AnySigner(tc.ctx, tc.auth))
 			if tc.wantInCtx != nil && !tc.auth.HasAddress(tc.ctx, tc.wantInCtx.Address()) {
 				t.Fatal("condition address that was expected in context not found")
 			}

x/cash/dynamicfee.go

@@ -189,7 +189,7 @@ func (d DynamicFeeDecorator) extractFee(ctx weave.Context, tx weave.Tx, store we
 	var finfo *FeeInfo
 	ftx, ok := tx.(FeeTx)
 	if ok {
-		payer := x.MainSigner(ctx, d.auth).Address()
+		payer := x.AnySigner(ctx, d.auth).Address()
 		finfo = ftx.GetFees().DefaultPayer(payer)
 	}
 

x/cash/staticfee.go

@@ -104,7 +104,7 @@ func (d FeeDecorator) extractFee(ctx weave.Context, tx weave.Tx, store weave.KVS
 	var finfo *FeeInfo
 	ftx, ok := tx.(FeeTx)
 	if ok {
-		payer := x.MainSigner(ctx, d.auth).Address()
+		payer := x.AnySigner(ctx, d.auth).Address()
 		finfo = ftx.GetFees().DefaultPayer(payer)
 	}
 

x/escrow/handler.go

@@ -69,7 +69,7 @@ func (h CreateEscrowHandler) Deliver(ctx weave.Context, db weave.KVStore, tx wea
 	// apply a default for source
 	source := msg.Source
 	if source == nil {
-		source = x.MainSigner(ctx, h.auth).Address()
+		source = x.AnySigner(ctx, h.auth).Address()
 	}
 
 	key, err := escrowSeq.NextVal(db)
@@ -109,7 +109,7 @@ func (h CreateEscrowHandler) validate(ctx weave.Context, db weave.KVStore, tx we
 		return nil, errors.Wrap(errors.ErrInput, "timeout in the past")
 	}
 
-	// Source must authorize this (if not set, defaults to MainSigner).
+	// Source must authorize this (if not set, defaults to AnySigner).
 	if msg.Source != nil {
 		if !h.auth.HasAddress(ctx, msg.Source) {
 			return nil, errors.ErrUnauthorized

x/gov/handler.go

@@ -138,7 +138,7 @@ func (h VoteHandler) validate(ctx weave.Context, db weave.KVStore, tx weave.Tx)
 
 	voter := msg.Voter
 	if voter == nil {
-		voter = x.MainSigner(ctx, h.auth).Address()
+		voter = x.AnySigner(ctx, h.auth).Address()
 	}
 	obj, err := h.elecBucket.GetVersion(db, proposal.ElectorateRef)
 	if err != nil {
@@ -412,7 +412,7 @@ func (h CreateProposalHandler) validate(ctx weave.Context, db weave.KVStore, tx
 			return nil, nil, nil, errors.Wrap(errors.ErrUnauthorized, "author's signature required")
 		}
 	} else {
-		author = x.MainSigner(ctx, h.auth).Address()
+		author = x.AnySigner(ctx, h.auth).Address()
 	}
 	msg.Author = author
 

x/multisig/handlers.go

@@ -66,7 +66,7 @@ func (h CreateMsgHandler) Deliver(ctx weave.Context, db weave.KVStore, tx weave.
 // validate does all common pre-processing between Check and Deliver.
 func (h CreateMsgHandler) validate(ctx weave.Context, db weave.KVStore, tx weave.Tx) (*CreateMsg, error) {
 	// Retrieve tx main signer in this context.
-	sender := x.MainSigner(ctx, h.auth)
+	sender := x.AnySigner(ctx, h.auth)
 	if sender == nil {
 		return nil, errors.Wrap(errors.ErrUnauthorized, "no signer")
 	}

x/sigs/handler.go

@@ -56,7 +56,7 @@ func (h *bumpSequenceHandler) validate(ctx weave.Context, db weave.KVStore, tx w
 		return nil, nil, errors.Wrap(err, "load msg")
 	}
 
-	pubkey := x.MainSigner(ctx, h.auth)
+	pubkey := x.AnySigner(ctx, h.auth)
 	if pubkey == nil {
 		return nil, nil, errors.Wrap(errors.ErrUnauthorized, "missing signature")
 	}