From c02bf19fc5b4810a4b0fb0340639e2bd78476706 Mon Sep 17 00:00:00 2001 From: Oliver Gugger Date: Thu, 21 Oct 2021 20:03:58 +0200 Subject: [PATCH 1/2] config: fix reflection parsing in LiT In the case where lnd's config struct is embedded inside another struct (for example in lightning-terminal), the flag won't be found under its original name. So we try to also look it up under the prefixed name. --- config.go | 36 +++++++++++++++++++++++++++++------- 1 file changed, 29 insertions(+), 7 deletions(-) diff --git a/config.go b/config.go index f3dc063d6..158e152be 100644 --- a/config.go +++ b/config.go @@ -717,21 +717,43 @@ func ValidateConfig(cfg Config, usageMessage string, // IsSet returns true if an option has been set in either the config // file or by a flag. isSet := func(field string) bool { - fieldname, ok := reflect.TypeOf(Config{}).FieldByName(field) + fieldName, ok := reflect.TypeOf(Config{}).FieldByName(field) if !ok { - fmt.Fprintf(os.Stderr, "could not find field %s\n", field) + str := "%s: could not find field %s" + err := fmt.Errorf(str, funcName, field) + _, _ = fmt.Fprintln(os.Stderr, err) return false } - long, ok := fieldname.Tag.Lookup("long") + long, ok := fieldName.Tag.Lookup("long") if !ok { - fmt.Fprintf(os.Stderr, - "field %s does not have a long tag\n", field) + str := "%s: field %s does not have a long tag" + err := fmt.Errorf(str, funcName, field) + _, _ = fmt.Fprintln(os.Stderr, err) return false } - return fileParser.FindOptionByLongName(long).IsSet() || - flagParser.FindOptionByLongName(long).IsSet() + // The user has the option to set the flag in either the config + // file or as a command line flag. If any is set, we consider it + // to be set, not applying any precedence rules here (since it + // is a boolean the default is false anyway which would screw up + // any precedence rules). Additionally, we need to also support + // the use case where the config struct is embedded _within_ + // another struct with a prefix (as is the case with + // lightning-terminal). + fileOption := fileParser.FindOptionByLongName(long) + fileOptionNested := fileParser.FindOptionByLongName( + "lnd." + long, + ) + flagOption := flagParser.FindOptionByLongName(long) + flagOptionNested := flagParser.FindOptionByLongName( + "lnd." + long, + ) + + return (fileOption != nil && fileOption.IsSet()) || + (fileOptionNested != nil && fileOptionNested.IsSet()) || + (flagOption != nil && flagOption.IsSet()) || + (flagOptionNested != nil && flagOptionNested.IsSet()) } // As soon as we're done parsing configuration options, ensure all paths From af09f11c1cf0c84f48c11e54c4b16bcabbe1611c Mon Sep 17 00:00:00 2001 From: Oliver Gugger Date: Thu, 21 Oct 2021 20:28:30 +0200 Subject: [PATCH 2/2] rpcperms: don't intercept if no middleware is registered If there is no middleware registered, we don't need to intercept any call and therefore can skip the request and macaroon parsing section. --- rpcperms/interceptor.go | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/rpcperms/interceptor.go b/rpcperms/interceptor.go index 71481592a..97e752b17 100644 --- a/rpcperms/interceptor.go +++ b/rpcperms/interceptor.go @@ -777,6 +777,12 @@ func (r *InterceptorChain) middlewareUnaryServerInterceptor() grpc.UnaryServerIn return nil, err } + // If there is no middleware registered, we don't need to + // intercept anything. + if !r.middlewareRegistered() { + return handler(ctx, req) + } + msg, err := NewMessageInterceptionRequest( ctx, TypeRequest, false, info.FullMethod, req, ) @@ -822,6 +828,12 @@ func (r *InterceptorChain) middlewareStreamServerInterceptor() grpc.StreamServer return err } + // If there is no middleware registered, we don't need to + // intercept anything. + if !r.middlewareRegistered() { + return handler(srv, ss) + } + // To give the middleware a chance to accept or reject the // establishment of the stream itself (and not only when the // first message is sent on the stream), we send an intercept @@ -875,6 +887,15 @@ func (r *InterceptorChain) checkMandatoryMiddleware(fullMethod string) error { return nil } +// middlewareRegistered returns true if there is at least one middleware +// currently registered. +func (r *InterceptorChain) middlewareRegistered() bool { + r.RLock() + defer r.RUnlock() + + return len(r.registeredMiddleware) > 0 +} + // acceptRequest sends an intercept request to all middlewares that have // registered for it. This means either a middleware has requested read-only // access or the request actually has a macaroon which a caveat the middleware