From 1dffaf10e21d81f9cf5a3d7d45edbff35d12d9ed Mon Sep 17 00:00:00 2001 From: Elle Mouton Date: Thu, 11 Aug 2022 11:10:27 +0200 Subject: [PATCH 1/4] cmd/lncli: add metadata kvpair flag Add a new `metadata` string slice flag to lncli that allows the caller to specify multiple key-value string pairs that should be appended to the outgoing context. --- cmd/lncli/main.go | 81 ++++++++++++++++++++++++++++++++++++++------ cmd/lncli/profile.go | 38 +++++++++++++++------ 2 files changed, 98 insertions(+), 21 deletions(-) diff --git a/cmd/lncli/main.go b/cmd/lncli/main.go index 3b983696d..ba989ff2f 100644 --- a/cmd/lncli/main.go +++ b/cmd/lncli/main.go @@ -24,6 +24,7 @@ import ( "golang.org/x/term" "google.golang.org/grpc" "google.golang.org/grpc/credentials" + "google.golang.org/grpc/metadata" ) const ( @@ -109,6 +110,12 @@ func getClientConn(ctx *cli.Context, skipMacaroons bool) *grpc.ClientConn { // Create a dial options array. opts := []grpc.DialOption{ grpc.WithTransportCredentials(creds), + grpc.WithUnaryInterceptor( + addMetadataUnaryInterceptor(profile.Metadata), + ), + grpc.WithStreamInterceptor( + addMetaDataStreamInterceptor(profile.Metadata), + ), } // Only process macaroon credentials if --no-macaroons isn't set and @@ -141,16 +148,17 @@ func getClientConn(ctx *cli.Context, skipMacaroons bool) *grpc.ClientConn { } macConstraints := []macaroons.Constraint{ - // We add a time-based constraint to prevent replay of the - // macaroon. It's good for 60 seconds by default to make up for - // any discrepancy between client and server clocks, but leaking - // the macaroon before it becomes invalid makes it possible for - // an attacker to reuse the macaroon. In addition, the validity - // time of the macaroon is extended by the time the server clock - // is behind the client clock, or shortened by the time the + // We add a time-based constraint to prevent replay of + // the macaroon. It's good for 60 seconds by default to + // make up for any discrepancy between client and server + // clocks, but leaking the macaroon before it becomes + // invalid makes it possible for an attacker to reuse + // the macaroon. In addition, the validity time of the + // macaroon is extended by the time the server clock is + // behind the client clock, or shortened by the time the // server clock is ahead of the client clock (or invalid - // altogether if, in the latter case, this time is more than 60 - // seconds). + // altogether if, in the latter case, this time is more + // than 60 seconds). // TODO(aakselrod): add better anti-replay protection. macaroons.TimeoutConstraint(profile.Macaroons.Timeout), @@ -180,7 +188,9 @@ func getClientConn(ctx *cli.Context, skipMacaroons bool) *grpc.ClientConn { // to connect to the grpc server. if ctx.GlobalIsSet("socksproxy") { socksProxy := ctx.GlobalString("socksproxy") - torDialer := func(_ context.Context, addr string) (net.Conn, error) { + torDialer := func(_ context.Context, addr string) (net.Conn, + error) { + return tor.Dial( addr, socksProxy, false, false, tor.DefaultConnTimeout, @@ -204,6 +214,49 @@ func getClientConn(ctx *cli.Context, skipMacaroons bool) *grpc.ClientConn { return conn } +// addMetadataUnaryInterceptor returns a grpc client side interceptor that +// appends any key-value metadata strings to the outgoing context of a grpc +// unary call. +func addMetadataUnaryInterceptor( + md map[string]string) grpc.UnaryClientInterceptor { + + return func(ctx context.Context, method string, req, reply interface{}, + cc *grpc.ClientConn, invoker grpc.UnaryInvoker, + opts ...grpc.CallOption) error { + + outCtx := contextWithMetadata(ctx, md) + return invoker(outCtx, method, req, reply, cc, opts...) + } +} + +// addMetaDataStreamInterceptor returns a grpc client side interceptor that +// appends any key-value metadata strings to the outgoing context of a grpc +// stream call. +func addMetaDataStreamInterceptor( + md map[string]string) grpc.StreamClientInterceptor { + + return func(ctx context.Context, desc *grpc.StreamDesc, + cc *grpc.ClientConn, method string, streamer grpc.Streamer, + opts ...grpc.CallOption) (grpc.ClientStream, error) { + + outCtx := contextWithMetadata(ctx, md) + return streamer(outCtx, desc, cc, method, opts...) + } +} + +// contextWithMetaData appends the given metadata key-value pairs to the given +// context. +func contextWithMetadata(ctx context.Context, + md map[string]string) context.Context { + + kvPairs := make([]string, 0, 2*len(md)) + for k, v := range md { + kvPairs = append(kvPairs, k, v) + } + + return metadata.AppendToOutgoingContext(ctx, kvPairs...) +} + // extractPathArgs parses the TLS certificate and macaroon paths from the // command. func extractPathArgs(ctx *cli.Context) (string, string, error) { @@ -349,6 +402,14 @@ func main() { "macaroon jar instead of the default one. " + "Can only be used if profiles are defined.", }, + cli.StringSliceFlag{ + Name: "metadata", + Usage: "This flag can be used to specify a key-value " + + "pair that should be appended to the " + + "outgoing context before the request is sent " + + "to lnd. This flag may be specified multiple " + + "times. The format is: \"key:value\".", + }, } app.Commands = []cli.Command{ createCommand, diff --git a/cmd/lncli/profile.go b/cmd/lncli/profile.go index 2f2819df6..5e611af88 100644 --- a/cmd/lncli/profile.go +++ b/cmd/lncli/profile.go @@ -24,14 +24,15 @@ var ( // profileEntry is a struct that represents all settings for one specific // profile. type profileEntry struct { - Name string `json:"name"` - RPCServer string `json:"rpcserver"` - LndDir string `json:"lnddir"` - Chain string `json:"chain"` - Network string `json:"network"` - NoMacaroons bool `json:"no-macaroons,omitempty"` // nolint:tagliatelle - TLSCert string `json:"tlscert"` - Macaroons *macaroonJar `json:"macaroons"` + Name string `json:"name"` + RPCServer string `json:"rpcserver"` + LndDir string `json:"lnddir"` + Chain string `json:"chain"` + Network string `json:"network"` + NoMacaroons bool `json:"no-macaroons,omitempty"` // nolint:tagliatelle + TLSCert string `json:"tlscert"` + Macaroons *macaroonJar `json:"macaroons"` + Metadata map[string]string `json:"metadata,omitempty"` } // cert returns the profile's TLS certificate as a x509 certificate pool. @@ -128,17 +129,32 @@ func profileFromContext(ctx *cli.Context, store, skipMacaroons bool) ( var err error tlsCert, err = ioutil.ReadFile(tlsCertPath) if err != nil { - return nil, fmt.Errorf("could not load TLS cert file: %v", err) + return nil, fmt.Errorf("could not load TLS cert "+ + "file: %v", err) } } + metadata := make(map[string]string) + for _, m := range ctx.GlobalStringSlice("metadata") { + pair := strings.Split(m, ":") + if len(pair) != 2 { + return nil, fmt.Errorf("invalid format for metadata " + + "flag; expected \"key:value\"") + } + + metadata[pair[0]] = pair[1] + } + entry := &profileEntry{ - RPCServer: ctx.GlobalString("rpcserver"), - LndDir: lncfg.CleanAndExpandPath(ctx.GlobalString("lnddir")), + RPCServer: ctx.GlobalString("rpcserver"), + LndDir: lncfg.CleanAndExpandPath( + ctx.GlobalString("lnddir"), + ), Chain: ctx.GlobalString("chain"), Network: ctx.GlobalString("network"), NoMacaroons: ctx.GlobalBool("no-macaroons"), TLSCert: string(tlsCert), + Metadata: metadata, } // If we aren't using macaroons in general (flag --no-macaroons) or From 027e4cdf11a6e0cfb0ffc749726e70ead6dbf048 Mon Sep 17 00:00:00 2001 From: Elle Mouton Date: Thu, 11 Aug 2022 11:25:52 +0200 Subject: [PATCH 2/4] cmd/lncli: add insecure flag to skip tls auth Add an `insecure` global flag to lncli. If set, tls authentication with the specified rpc server will be skipped. --- cmd/lncli/main.go | 52 +++++++++++++++++++++++++++----------------- cmd/lncli/profile.go | 6 ++++- 2 files changed, 37 insertions(+), 21 deletions(-) diff --git a/cmd/lncli/main.go b/cmd/lncli/main.go index ba989ff2f..116a42b13 100644 --- a/cmd/lncli/main.go +++ b/cmd/lncli/main.go @@ -88,28 +88,8 @@ func getClientConn(ctx *cli.Context, skipMacaroons bool) *grpc.ClientConn { fatal(fmt.Errorf("could not load global options: %v", err)) } - // Load the specified TLS certificate. - certPool, err := profile.cert() - if err != nil { - fatal(fmt.Errorf("could not create cert pool: %v", err)) - } - - // Build transport credentials from the certificate pool. If there is no - // certificate pool, we expect the server to use a non-self-signed - // certificate such as a certificate obtained from Let's Encrypt. - var creds credentials.TransportCredentials - if certPool != nil { - creds = credentials.NewClientTLSFromCert(certPool, "") - } else { - // Fallback to the system pool. Using an empty tls config is an - // alternative to x509.SystemCertPool(). That call is not - // supported on Windows. - creds = credentials.NewTLS(&tls.Config{}) - } - // Create a dial options array. opts := []grpc.DialOption{ - grpc.WithTransportCredentials(creds), grpc.WithUnaryInterceptor( addMetadataUnaryInterceptor(profile.Metadata), ), @@ -118,6 +98,32 @@ func getClientConn(ctx *cli.Context, skipMacaroons bool) *grpc.ClientConn { ), } + if profile.Insecure { + opts = append(opts, grpc.WithInsecure()) + } else { + // Load the specified TLS certificate. + certPool, err := profile.cert() + if err != nil { + fatal(fmt.Errorf("could not create cert pool: %v", err)) + } + + // Build transport credentials from the certificate pool. If + // there is no certificate pool, we expect the server to use a + // non-self-signed certificate such as a certificate obtained + // from Let's Encrypt. + var creds credentials.TransportCredentials + if certPool != nil { + creds = credentials.NewClientTLSFromCert(certPool, "") + } else { + // Fallback to the system pool. Using an empty tls + // config is an alternative to x509.SystemCertPool(). + // That call is not supported on Windows. + creds = credentials.NewTLS(&tls.Config{}) + } + + opts = append(opts, grpc.WithTransportCredentials(creds)) + } + // Only process macaroon credentials if --no-macaroons isn't set and // if we're not skipping macaroon processing. if !profile.NoMacaroons && !skipMacaroons { @@ -410,6 +416,12 @@ func main() { "to lnd. This flag may be specified multiple " + "times. The format is: \"key:value\".", }, + cli.BoolFlag{ + Name: "insecure", + Usage: "Connect to the rpc server without TLS " + + "authentication", + Hidden: true, + }, } app.Commands = []cli.Command{ createCommand, diff --git a/cmd/lncli/profile.go b/cmd/lncli/profile.go index 5e611af88..e5d4aacb5 100644 --- a/cmd/lncli/profile.go +++ b/cmd/lncli/profile.go @@ -33,6 +33,7 @@ type profileEntry struct { TLSCert string `json:"tlscert"` Macaroons *macaroonJar `json:"macaroons"` Metadata map[string]string `json:"metadata,omitempty"` + Insecure bool `json:"insecure,omitempty"` } // cert returns the profile's TLS certificate as a x509 certificate pool. @@ -122,10 +123,12 @@ func profileFromContext(ctx *cli.Context, store, skipMacaroons bool) ( return nil, err } + insecure := ctx.GlobalBool("insecure") + // Load the certificate file now, if specified. We store it as plain PEM // directly. var tlsCert []byte - if tlsCertPath != "" { + if tlsCertPath != "" && !insecure { var err error tlsCert, err = ioutil.ReadFile(tlsCertPath) if err != nil { @@ -155,6 +158,7 @@ func profileFromContext(ctx *cli.Context, store, skipMacaroons bool) ( NoMacaroons: ctx.GlobalBool("no-macaroons"), TLSCert: string(tlsCert), Metadata: metadata, + Insecure: insecure, } // If we aren't using macaroons in general (flag --no-macaroons) or From bd857ee298fd056e0f1e480c5871d744e0e0019e Mon Sep 17 00:00:00 2001 From: Elle Mouton Date: Thu, 11 Aug 2022 11:28:18 +0200 Subject: [PATCH 3/4] cmd/lncli: use bitsize of 64 to read output index In the lnrpc proto, we use either int64 or uint32 for output index. So to keep things consistent, we should use a bitsize of 64 when reading an output index from a string. Otherwise, we fail could fail to parse an index that is valid based on the types we define in the protos. --- cmd/lncli/commands.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/lncli/commands.go b/cmd/lncli/commands.go index 98f39c549..7e46f1570 100644 --- a/cmd/lncli/commands.go +++ b/cmd/lncli/commands.go @@ -2052,7 +2052,7 @@ func parseChanPoint(s string) (*lnrpc.ChannelPoint, error) { return nil, errBadChanPoint } - index, err := strconv.ParseInt(split[1], 10, 32) + index, err := strconv.ParseInt(split[1], 10, 64) if err != nil { return nil, fmt.Errorf("unable to decode output index: %v", err) } From a75ab9f71ca4db70c84b69218a2866427bfd41e9 Mon Sep 17 00:00:00 2001 From: Elle Mouton Date: Thu, 11 Aug 2022 11:44:25 +0200 Subject: [PATCH 4/4] docs: update release notes --- docs/release-notes/release-notes-0.16.0.md | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/docs/release-notes/release-notes-0.16.0.md b/docs/release-notes/release-notes-0.16.0.md index 0b0a20039..c4d317f0e 100644 --- a/docs/release-notes/release-notes-0.16.0.md +++ b/docs/release-notes/release-notes-0.16.0.md @@ -20,6 +20,12 @@ transaction](https://github.com/lightningnetwork/lnd/pull/6730). * [The macaroon key store implementation was refactored to be more generally usable](https://github.com/lightningnetwork/lnd/pull/6509). +## `lncli` +* [Add an `insecure` flag to skip tls auth as well as a `metadata` string slice + flag](https://github.com/lightningnetwork/lnd/pull/6818) that allows the + caller to specify key-value string pairs that should be appended to the + outgoing context. + ## Code Health ### Tooling and documentation @@ -31,6 +37,7 @@ transaction](https://github.com/lightningnetwork/lnd/pull/6730). * Carla Kirk-Cohen * Daniel McNally +* Elle Mouton * ErikEk * Olaoluwa Osuntokun * Oliver Gugger