Skip to content

Commit

Permalink
Extra checks for invalid args in gateway api
Browse files Browse the repository at this point in the history 
Additional argument validatation and consistently use getters to safely traverse proto structures.

Signed-off-by: andrew-coleman <andrew_coleman@uk.ibm.com>
(cherry picked from commit dfaa164)
  • Loading branch information
@andrew-coleman @denyeart
andrew-coleman authored and denyeart committed Aug 8, 2022
1 parent 84e8b3b commit 7c12fa6
Show file tree
Hide file tree
Showing 3 changed files with 57 additions and 36 deletions.
58 changes: 29 additions & 29 deletions internal/pkg/gateway/api.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ func (gs *Server) Evaluate(ctx context.Context, request *gp.EvaluateRequest) (*g
var response *peer.Response
var errDetails []proto.Message
for response == nil {
gs.logger.Debugw("Sending to peer:", "channel", channel, "chaincode", chaincodeID, "txID", request.TransactionId, "MSPID", endorser.mspid, "endpoint", endorser.address)
gs.logger.Debugw("Sending to peer:", "channel", channel, "chaincode", chaincodeID, "txID", request.GetTransactionId(), "MSPID", endorser.mspid, "endpoint", endorser.address)

done := make(chan error)
go func() {
Expand All @@ -84,10 +84,10 @@ func (gs *Server) Evaluate(ctx context.Context, request *gp.EvaluateRequest) (*g
if result, err := getResultFromProposalResponse(pr); err == nil {
response.Payload = result
} else {
logger.Warnw("Successful proposal response contained no transaction result", "error", err.Error(), "chaincode", chaincodeID, "channel", channel, "txID", request.TransactionId, "endorserAddress", endorser.endpointConfig.address, "endorserMspid", endorser.endpointConfig.mspid, "status", response.Status, "message", response.Message)
logger.Warnw("Successful proposal response contained no transaction result", "error", err.Error(), "chaincode", chaincodeID, "channel", channel, "txID", request.GetTransactionId(), "endorserAddress", endorser.endpointConfig.address, "endorserMspid", endorser.endpointConfig.mspid, "status", response.GetStatus(), "message", response.GetMessage())
}
} else {
logger.Debugw("Evaluate call to endorser failed", "chaincode", chaincodeID, "channel", channel, "txID", request.TransactionId, "endorserAddress", endorser.endpointConfig.address, "endorserMspid", endorser.endpointConfig.mspid, "error", message)
logger.Debugw("Evaluate call to endorser failed", "chaincode", chaincodeID, "channel", channel, "txID", request.GetTransactionId(), "endorserAddress", endorser.endpointConfig.address, "endorserMspid", endorser.endpointConfig.mspid, "error", message)
errDetails = append(errDetails, errorDetail(endorser.endpointConfig, message))
if remove {
gs.registry.removeEndorser(endorser)
Expand All @@ -109,7 +109,7 @@ func (gs *Server) Evaluate(ctx context.Context, request *gp.EvaluateRequest) (*g
}
case <-ctx.Done():
// Overall evaluation timeout expired
logger.Warnw("Evaluate call timed out while processing request", "channel", request.ChannelId, "txID", request.TransactionId)
logger.Warnw("Evaluate call timed out while processing request", "channel", request.GetChannelId(), "txID", request.GetTransactionId())
return nil, newRpcError(codes.DeadlineExceeded, "evaluate timeout expired")
}
}
Expand All @@ -118,7 +118,7 @@ func (gs *Server) Evaluate(ctx context.Context, request *gp.EvaluateRequest) (*g
Result: response,
}

logger.Debugw("Evaluate call to endorser returned success", "channel", request.ChannelId, "txID", request.TransactionId, "endorserAddress", endorser.endpointConfig.address, "endorserMspid", endorser.endpointConfig.mspid, "status", response.GetStatus(), "message", response.GetMessage())
logger.Debugw("Evaluate call to endorser returned success", "channel", request.GetChannelId(), "txID", request.GetTransactionId(), "endorserAddress", endorser.endpointConfig.address, "endorserMspid", endorser.endpointConfig.mspid, "status", response.GetStatus(), "message", response.GetMessage())
return evaluateResponse, nil
}

Expand All @@ -129,41 +129,41 @@ func (gs *Server) Endorse(ctx context.Context, request *gp.EndorseRequest) (*gp.
return nil, status.Error(codes.InvalidArgument, "an endorse request is required")
}
signedProposal := request.GetProposedTransaction()
if signedProposal == nil {
if len(signedProposal.GetProposalBytes()) == 0 {
return nil, status.Error(codes.InvalidArgument, "the proposed transaction must contain a signed proposal")
}
proposal, err := protoutil.UnmarshalProposal(signedProposal.ProposalBytes)
proposal, err := protoutil.UnmarshalProposal(signedProposal.GetProposalBytes())
if err != nil {
return nil, status.Error(codes.InvalidArgument, err.Error())
}
header, err := protoutil.UnmarshalHeader(proposal.Header)
header, err := protoutil.UnmarshalHeader(proposal.GetHeader())
if err != nil {
return nil, status.Error(codes.InvalidArgument, err.Error())
}
channelHeader, err := protoutil.UnmarshalChannelHeader(header.ChannelHeader)
channelHeader, err := protoutil.UnmarshalChannelHeader(header.GetChannelHeader())
if err != nil {
return nil, status.Error(codes.InvalidArgument, err.Error())
}
payload, err := protoutil.UnmarshalChaincodeProposalPayload(proposal.Payload)
payload, err := protoutil.UnmarshalChaincodeProposalPayload(proposal.GetPayload())
if err != nil {
return nil, status.Error(codes.InvalidArgument, err.Error())
}
spec, err := protoutil.UnmarshalChaincodeInvocationSpec(payload.Input)
spec, err := protoutil.UnmarshalChaincodeInvocationSpec(payload.GetInput())
if err != nil {
return nil, status.Error(codes.InvalidArgument, err.Error())
}

channel := channelHeader.ChannelId
channel := channelHeader.GetChannelId()
chaincodeID := spec.GetChaincodeSpec().GetChaincodeId().GetName()
hasTransientData := len(payload.GetTransientMap()) > 0

logger := gs.logger.With("channel", channel, "chaincode", chaincodeID, "txID", request.TransactionId)
logger := gs.logger.With("channel", channel, "chaincode", chaincodeID, "txID", request.GetTransactionId())

var plan *plan
var action *peer.ChaincodeEndorsedAction
if len(request.EndorsingOrganizations) > 0 {
if len(request.GetEndorsingOrganizations()) > 0 {
// The client is specifying the endorsing orgs and taking responsibility for ensuring it meets the signature policy
plan, err = gs.registry.planForOrgs(channel, chaincodeID, request.EndorsingOrganizations)
plan, err = gs.registry.planForOrgs(channel, chaincodeID, request.GetEndorsingOrganizations())
if err != nil {
return nil, status.Error(codes.Unavailable, err.Error())
}
Expand Down Expand Up @@ -484,25 +484,25 @@ func (gs *Server) broadcast(ctx context.Context, orderer *orderer, txn *common.E
// If the transaction commit status cannot be returned, for example if the specified channel does not exist, a
// FailedPrecondition error will be returned.
func (gs *Server) CommitStatus(ctx context.Context, signedRequest *gp.SignedCommitStatusRequest) (*gp.CommitStatusResponse, error) {
if signedRequest == nil {
if len(signedRequest.GetRequest()) == 0 {
return nil, status.Error(codes.InvalidArgument, "a commit status request is required")
}

request := &gp.CommitStatusRequest{}
if err := proto.Unmarshal(signedRequest.Request, request); err != nil {
if err := proto.Unmarshal(signedRequest.GetRequest(), request); err != nil {
return nil, status.Errorf(codes.InvalidArgument, "invalid status request: %v", err)
}

signedData := &protoutil.SignedData{
Data: signedRequest.Request,
Identity: request.Identity,
Signature: signedRequest.Signature,
Data: signedRequest.GetRequest(),
Identity: request.GetIdentity(),
Signature: signedRequest.GetSignature(),
}
if err := gs.policy.CheckACL(resources.Gateway_CommitStatus, request.ChannelId, signedData); err != nil {
if err := gs.policy.CheckACL(resources.Gateway_CommitStatus, request.GetChannelId(), signedData); err != nil {
return nil, status.Error(codes.PermissionDenied, err.Error())
}

txStatus, err := gs.commitFinder.TransactionStatus(ctx, request.ChannelId, request.TransactionId)
txStatus, err := gs.commitFinder.TransactionStatus(ctx, request.GetChannelId(), request.GetTransactionId())
if err != nil {
return nil, toRpcError(err, codes.Aborted)
}
Expand All @@ -520,21 +520,21 @@ func (gs *Server) CommitStatus(ctx context.Context, signedRequest *gp.SignedComm
// events within each response message are presented in the same order that the transactions that emitted them appear
// within the block.
func (gs *Server) ChaincodeEvents(signedRequest *gp.SignedChaincodeEventsRequest, stream gp.Gateway_ChaincodeEventsServer) error {
if signedRequest == nil {
if len(signedRequest.GetRequest()) == 0 {
return status.Error(codes.InvalidArgument, "a chaincode events request is required")
}

request := &gp.ChaincodeEventsRequest{}
if err := proto.Unmarshal(signedRequest.Request, request); err != nil {
if err := proto.Unmarshal(signedRequest.GetRequest(), request); err != nil {
return status.Errorf(codes.InvalidArgument, "invalid chaincode events request: %v", err)
}

signedData := &protoutil.SignedData{
Data: signedRequest.Request,
Identity: request.Identity,
Signature: signedRequest.Signature,
Data: signedRequest.GetRequest(),
Identity: request.GetIdentity(),
Signature: signedRequest.GetSignature(),
}
if err := gs.policy.CheckACL(resources.Gateway_ChaincodeEvents, request.ChannelId, signedData); err != nil {
if err := gs.policy.CheckACL(resources.Gateway_ChaincodeEvents, request.GetChannelId(), signedData); err != nil {
return status.Error(codes.PermissionDenied, err.Error())
}

Expand Down Expand Up @@ -565,7 +565,7 @@ func (gs *Server) ChaincodeEvents(signedRequest *gp.SignedChaincodeEventsRequest
}

var matchingEvents []*peer.ChaincodeEvent
for _, event := range response.Events {
for _, event := range response.GetEvents() {
if isMatch(event) {
matchingEvents = append(matchingEvents, event)
}
Expand Down
21 changes: 21 additions & 0 deletions internal/pkg/gateway/api_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1967,20 +1967,41 @@ func TestNilArgs(t *testing.T) {
_, err = server.Evaluate(ctx, &pb.EvaluateRequest{ProposedTransaction: nil})
require.ErrorIs(t, err, status.Error(codes.InvalidArgument, "failed to unpack transaction proposal: a signed proposal is required"))

_, err = server.Evaluate(ctx, &pb.EvaluateRequest{ProposedTransaction: &peer.SignedProposal{}})
require.ErrorIs(t, err, status.Error(codes.InvalidArgument, "failed to unpack transaction proposal: a signed proposal is required"))

_, err = server.Evaluate(ctx, &pb.EvaluateRequest{ProposedTransaction: &peer.SignedProposal{ProposalBytes: []byte("jibberish")}})
require.ErrorContains(t, err, "failed to unpack transaction proposal: error unmarshalling Proposal")

_, err = server.Endorse(ctx, nil)
require.ErrorIs(t, err, status.Error(codes.InvalidArgument, "an endorse request is required"))

_, err = server.Endorse(ctx, &pb.EndorseRequest{ProposedTransaction: nil})
require.ErrorIs(t, err, status.Error(codes.InvalidArgument, "the proposed transaction must contain a signed proposal"))

_, err = server.Endorse(ctx, &pb.EndorseRequest{ProposedTransaction: &peer.SignedProposal{}})
require.ErrorIs(t, err, status.Error(codes.InvalidArgument, "the proposed transaction must contain a signed proposal"))

_, err = server.Endorse(ctx, &pb.EndorseRequest{ProposedTransaction: &peer.SignedProposal{ProposalBytes: []byte("jibberish")}})
require.ErrorContains(t, err, "rpc error: code = InvalidArgument desc = error unmarshalling Proposal")

_, err = server.Submit(ctx, nil)
require.ErrorIs(t, err, status.Error(codes.InvalidArgument, "a submit request is required"))

_, err = server.Submit(ctx, &pb.SubmitRequest{})
require.ErrorIs(t, err, status.Error(codes.InvalidArgument, "a prepared transaction is required"))

_, err = server.CommitStatus(ctx, nil)
require.ErrorIs(t, err, status.Error(codes.InvalidArgument, "a commit status request is required"))

_, err = server.CommitStatus(ctx, &pb.SignedCommitStatusRequest{})
require.ErrorIs(t, err, status.Error(codes.InvalidArgument, "a commit status request is required"))

err = server.ChaincodeEvents(nil, &mocks.ChaincodeEventsServer{})
require.ErrorIs(t, err, status.Error(codes.InvalidArgument, "a chaincode events request is required"))

err = server.ChaincodeEvents(&pb.SignedChaincodeEventsRequest{}, &mocks.ChaincodeEventsServer{})
require.ErrorIs(t, err, status.Error(codes.InvalidArgument, "a chaincode events request is required"))
}

func TestRpcErrorWithBadDetails(t *testing.T) {
Expand Down
14 changes: 7 additions & 7 deletions internal/pkg/gateway/apiutils.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,31 +22,31 @@ import (
)

func getChannelAndChaincodeFromSignedProposal(signedProposal *peer.SignedProposal) (string, string, bool, error) {
if signedProposal == nil {
if len(signedProposal.GetProposalBytes()) == 0 {
return "", "", false, fmt.Errorf("a signed proposal is required")
}
proposal, err := protoutil.UnmarshalProposal(signedProposal.ProposalBytes)
proposal, err := protoutil.UnmarshalProposal(signedProposal.GetProposalBytes())
if err != nil {
return "", "", false, err
}
header, err := protoutil.UnmarshalHeader(proposal.Header)
header, err := protoutil.UnmarshalHeader(proposal.GetHeader())
if err != nil {
return "", "", false, err
}
channelHeader, err := protoutil.UnmarshalChannelHeader(header.ChannelHeader)
channelHeader, err := protoutil.UnmarshalChannelHeader(header.GetChannelHeader())
if err != nil {
return "", "", false, err
}
payload, err := protoutil.UnmarshalChaincodeProposalPayload(proposal.Payload)
payload, err := protoutil.UnmarshalChaincodeProposalPayload(proposal.GetPayload())
if err != nil {
return "", "", false, err
}
spec, err := protoutil.UnmarshalChaincodeInvocationSpec(payload.Input)
spec, err := protoutil.UnmarshalChaincodeInvocationSpec(payload.GetInput())
if err != nil {
return "", "", false, err
}

return channelHeader.ChannelId, spec.ChaincodeSpec.ChaincodeId.Name, len(payload.TransientMap) > 0, nil
return channelHeader.GetChannelId(), spec.GetChaincodeSpec().GetChaincodeId().GetName(), len(payload.GetTransientMap()) > 0, nil
}

func newRpcError(code codes.Code, message string, details ...proto.Message) error {
Expand Down

0 comments on commit 7c12fa6

Please sign in to comment.