firewall: extract session ID from metadata

Author: ellemouton

firewall/privacy_mapper.go

@@ -106,9 +106,11 @@ func (p *PrivacyMapper) Intercept(ctx context.Context,
 			"interception request: %v", err)
 	}
 
-	sessionID, err := session.IDFromMacaroon(ri.Macaroon)
+	sessionID, ok, err := ri.extractSessionID()
 	if err != nil {
-		return nil, fmt.Errorf("could not extract ID from macaroon")
+		return nil, fmt.Errorf("could not extract session ID: %v", err)
+	} else if !ok {
+		return nil, fmt.Errorf("no session ID found in request")
 	}
 
 	log.Tracef("PrivacyMapper: Intercepting %v", ri)

firewall/request_info.go

@@ -4,7 +4,9 @@ import (
 	"fmt"
 	"strings"
 
+	"github.com/lightninglabs/lightning-terminal/session"
 	"github.com/lightningnetwork/lnd/lnrpc"
+	"google.golang.org/grpc/metadata"
 	"gopkg.in/macaroon.v2"
 )
 
@@ -38,18 +40,27 @@ type RequestInfo struct {
 	MetaInfo        *InterceptMetaInfo
 	Rules           *InterceptRules
 	WithPrivacy     bool
+	MDPairs         metadata.MD
 }
 
 // NewInfoFromRequest parses the given RPC middleware interception request and
 // returns a RequestInfo struct.
 func NewInfoFromRequest(req *lnrpc.RPCMiddlewareRequest) (*RequestInfo, error) {
+	md := make(metadata.MD)
+	for k, vs := range req.MetadataPairs {
+		for _, v := range vs.Values {
+			md.Append(k, v)
+		}
+	}
+
 	var ri *RequestInfo
 	switch t := req.InterceptType.(type) {
 	case *lnrpc.RPCMiddlewareRequest_StreamAuth:
 		ri = &RequestInfo{
 			MWRequestType: MWRequestTypeStreamAuth,
 			URI:           t.StreamAuth.MethodFullUri,
 			Streaming:     true,
+			MDPairs:       md,
 		}
 
 	case *lnrpc.RPCMiddlewareRequest_Request:
@@ -60,6 +71,7 @@ func NewInfoFromRequest(req *lnrpc.RPCMiddlewareRequest) (*RequestInfo, error) {
 			IsError:         t.Request.IsError,
 			Serialized:      t.Request.Serialized,
 			Streaming:       t.Request.StreamRpc,
+			MDPairs:         md,
 		}
 
 	case *lnrpc.RPCMiddlewareRequest_Response:
@@ -70,6 +82,7 @@ func NewInfoFromRequest(req *lnrpc.RPCMiddlewareRequest) (*RequestInfo, error) {
 			IsError:         t.Response.IsError,
 			Serialized:      t.Response.Serialized,
 			Streaming:       t.Response.StreamRpc,
+			MDPairs:         md,
 		}
 
 	default:
@@ -134,3 +147,35 @@ func (ri *RequestInfo) String() string {
 		ri.GRPCMessageType, ri.Streaming, strings.Join(ri.Caveats, ","),
 		ri.MetaInfo, ri.Rules)
 }
+
+func (ri *RequestInfo) extractSessionID() (session.ID, bool, error) {
+	// First prize is to extract the session ID from the MD pairs.
+	id, ok, err := session.FromGrpcMD(ri.MDPairs)
+	if err != nil {
+		return id, ok, err
+	} else if ok {
+		return id, ok, nil
+	}
+
+	// TODO(elle): This is a temporary workaround to support older versions
+	// of LND that don't attach metadata pairs to the request.
+	// We should remove this once we have bumped our minimum compatible
+	// LND version to one that always attaches metadata pairs.
+	// This is because the macaroon root key ID is not a reliable way of
+	// extracting the session ID since the macaroon root key ID may also
+	// be the first 4 bytes of an account ID and so collisions are possible
+	// here.
+
+	// Otherwise, fall back to extracting the session ID from the macaroon.
+	if ri.Macaroon == nil {
+		return session.ID{}, false, nil
+	}
+
+	id, err = session.IDFromMacaroon(ri.Macaroon)
+	if err != nil {
+		return session.ID{}, false, fmt.Errorf("could not extract "+
+			"ID from macaroon: %w", err)
+	}
+
+	return id, true, nil
+}

firewall/request_logger.go

@@ -182,15 +182,30 @@ func (r *RequestLogger) Intercept(_ context.Context,
 func (r *RequestLogger) addNewAction(ri *RequestInfo,
 	withPayloadData bool) error {
 
-	// If no macaroon is provided, then an empty 4-byte array is used as the
-	// session ID. Otherwise, the macaroon is used to derive a session ID.
-	var sessionID [4]byte
-	if ri.Macaroon != nil {
-		var err error
-		sessionID, err = session.IDFromMacaroon(ri.Macaroon)
-		if err != nil {
-			return fmt.Errorf("could not extract ID from macaroon")
-		}
+	// NOTE: Here is where we determine if we are linking the action to a
+	// known session OR a known account OR both. Right now (to not change
+	// behaviour from before): we give preference to a linked session.
+	// The original behaviour here was to purely use the macaroon's
+	// root key ID (4 bytes) to extract the "session ID". BUT if this
+	// action was triggered by an account call (that is not coupled to a
+	// session), then the "session ID" we are extracting here is actually
+	// the first 4 bytes of the account ID.
+	//
+	// What we actually need to do is:
+	// 1) only use the ctx/metadata to extract the session ID.
+	// 2) use the macaroon caveat to extract the account ID.
+	// With the above complete, we will be able to link an action to:
+	//    a session OR an account OR both OR none.
+	//
+	// Given the above info, it is also clear that the name "session ID"
+	// here is misleading as it is not necessarily the session ID. It could
+	// be: a session ID, an account ID or None. So it is more like a
+	// "macaroon identifier".
+	var sessionID session.ID
+	if sessID, ok, err := ri.extractSessionID(); err != nil {
+		return fmt.Errorf("could not extract session ID: %v", err)
+	} else if ok {
+		sessionID = sessID
 	}
 
 	action := &firewalldb.Action{

firewall/rule_enforcer.go

@@ -233,9 +233,11 @@ func (r *RuleEnforcer) Intercept(ctx context.Context,
 func (r *RuleEnforcer) handleRequest(ctx context.Context,
 	ri *RequestInfo) (proto.Message, error) {
 
-	sessionID, err := session.IDFromMacaroon(ri.Macaroon)
+	sessionID, ok, err := ri.extractSessionID()
 	if err != nil {
-		return nil, fmt.Errorf("could not extract ID from macaroon")
+		return nil, fmt.Errorf("could not extract session ID: %v", err)
+	} else if !ok {
+		return nil, fmt.Errorf("no session ID found in request")
 	}
 
 	rules, err := r.collectEnforcers(ctx, ri, sessionID)