From 55e0512a057a9f9e400de76906d2b8019d5eda2f Mon Sep 17 00:00:00 2001 From: Tom DNetto Date: Sun, 4 Dec 2022 22:55:57 -0800 Subject: [PATCH] ipn/ipnlocal,cmd/tailscale: minor improvements to lock modify command * Do not print the status at the end of a successful operation * Ensure the key of the current node is actually trusted to make these changes Signed-off-by: Tom DNetto --- client/tailscale/localclient.go | 11 +++++------ cmd/tailscale/cli/network-lock.go | 23 ++++++++++------------- ipn/ipnlocal/network-lock.go | 3 +++ ipn/localapi/localapi.go | 9 +-------- 4 files changed, 19 insertions(+), 27 deletions(-) diff --git a/client/tailscale/localclient.go b/client/tailscale/localclient.go index 3042f7f25..076c597f8 100644 --- a/client/tailscale/localclient.go +++ b/client/tailscale/localclient.go @@ -805,7 +805,7 @@ type initRequest struct { } // NetworkLockModify adds and/or removes key(s) to the tailnet key authority. -func (lc *LocalClient) NetworkLockModify(ctx context.Context, addKeys, removeKeys []tka.Key) (*ipnstate.NetworkLockStatus, error) { +func (lc *LocalClient) NetworkLockModify(ctx context.Context, addKeys, removeKeys []tka.Key) error { var b bytes.Buffer type modifyRequest struct { AddKeys []tka.Key @@ -813,14 +813,13 @@ type modifyRequest struct { } if err := json.NewEncoder(&b).Encode(modifyRequest{AddKeys: addKeys, RemoveKeys: removeKeys}); err != nil { - return nil, err + return err } - body, err := lc.send(ctx, "POST", "/localapi/v0/tka/modify", 200, &b) - if err != nil { - return nil, fmt.Errorf("error: %w", err) + if _, err := lc.send(ctx, "POST", "/localapi/v0/tka/modify", 204, &b); err != nil { + return fmt.Errorf("error: %w", err) } - return decodeJSON[*ipnstate.NetworkLockStatus](body) + return nil } // NetworkLockSign signs the specified node-key and transmits that signature to the control plane. diff --git a/cmd/tailscale/cli/network-lock.go b/cmd/tailscale/cli/network-lock.go index 8b02c6204..8cbc23bba 100644 --- a/cmd/tailscale/cli/network-lock.go +++ b/cmd/tailscale/cli/network-lock.go @@ -28,7 +28,7 @@ Name: "lock", ShortUsage: "lock ", ShortHelp: "Manage tailnet lock", - LongHelp: "Manage tailnet lock", + LongHelp: "Manage tailnet lock", Subcommands: []*ffcli.Command{ nlInitCmd, nlStatusCmd, @@ -155,7 +155,7 @@ func runNetworkLockInit(ctx context.Context, args []string) error { Name: "status", ShortUsage: "status", ShortHelp: "Outputs the state of network lock", - LongHelp: "Outputs the state of network lock", + LongHelp: "Outputs the state of network lock", Exec: runNetworkLockStatus, } @@ -229,7 +229,7 @@ func runNetworkLockStatus(ctx context.Context, args []string) error { Name: "add", ShortUsage: "add ...", ShortHelp: "Adds one or more trusted signing keys to tailnet lock", - LongHelp: "Adds one or more trusted signing keys to tailnet lock", + LongHelp: "Adds one or more trusted signing keys to tailnet lock", Exec: func(ctx context.Context, args []string) error { return runNetworkLockModify(ctx, args, nil) }, @@ -239,7 +239,7 @@ func runNetworkLockStatus(ctx context.Context, args []string) error { Name: "remove", ShortUsage: "remove ...", ShortHelp: "Removes one or more trusted signing keys from tailnet lock", - LongHelp: "Removes one or more trusted signing keys from tailnet lock", + LongHelp: "Removes one or more trusted signing keys from tailnet lock", Exec: func(ctx context.Context, args []string) error { return runNetworkLockModify(ctx, nil, args) }, @@ -310,12 +310,9 @@ func runNetworkLockModify(ctx context.Context, addArgs, removeArgs []string) err return err } - status, err := localClient.NetworkLockModify(ctx, addKeys, removeKeys) - if err != nil { + if err := localClient.NetworkLockModify(ctx, addKeys, removeKeys); err != nil { return err } - - fmt.Printf("Status: %+v\n\n", status) return nil } @@ -323,7 +320,7 @@ func runNetworkLockModify(ctx context.Context, addArgs, removeArgs []string) err Name: "sign", ShortUsage: "sign []", ShortHelp: "Signs a node key and transmits the signature to the coordination server", - LongHelp: "Signs a node key and transmits the signature to the coordination server", + LongHelp: "Signs a node key and transmits the signature to the coordination server", Exec: runNetworkLockSign, } @@ -363,7 +360,7 @@ func runNetworkLockSign(ctx context.Context, args []string) error { to all nodes in the tailnet and should be considered public. `), - Exec: runNetworkLockDisable, + Exec: runNetworkLockDisable, } func runNetworkLockDisable(ctx context.Context, args []string) error { @@ -392,7 +389,7 @@ func runNetworkLockDisable(ctx context.Context, args []string) error { that are locked out. `), - Exec: runNetworkLockLocalDisable, + Exec: runNetworkLockLocalDisable, } func runNetworkLockLocalDisable(ctx context.Context, args []string) error { @@ -403,7 +400,7 @@ func runNetworkLockLocalDisable(ctx context.Context, args []string) error { Name: "disablement-kdf", ShortUsage: "disablement-kdf ", ShortHelp: "Computes a disablement value from a disablement secret (advanced users only)", - LongHelp: "Computes a disablement value from a disablement secret (advanced users only)", + LongHelp: "Computes a disablement value from a disablement secret (advanced users only)", Exec: runNetworkLockDisablementKDF, } @@ -427,7 +424,7 @@ func runNetworkLockDisablementKDF(ctx context.Context, args []string) error { Name: "log", ShortUsage: "log [--limit N]", ShortHelp: "List changes applied to tailnet lock", - LongHelp: "List changes applied to tailnet lock", + LongHelp: "List changes applied to tailnet lock", Exec: runNetworkLockLog, FlagSet: (func() *flag.FlagSet { fs := newFlagSet("lock log") diff --git a/ipn/ipnlocal/network-lock.go b/ipn/ipnlocal/network-lock.go index 7fb4dd77c..c9563c941 100644 --- a/ipn/ipnlocal/network-lock.go +++ b/ipn/ipnlocal/network-lock.go @@ -654,6 +654,9 @@ func (b *LocalBackend) NetworkLockModify(addKeys, removeKeys []tka.Key) (err err if b.tka == nil { return errNetworkLockNotActive } + if !b.tka.authority.KeyTrusted(nlPriv.KeyID()) { + return errors.New("this node does not have a trusted tailnet lock key") + } updater := b.tka.authority.NewUpdater(nlPriv) diff --git a/ipn/localapi/localapi.go b/ipn/localapi/localapi.go index 7e4296d2b..7d1ecb383 100644 --- a/ipn/localapi/localapi.go +++ b/ipn/localapi/localapi.go @@ -1254,14 +1254,7 @@ type modifyRequest struct { http.Error(w, "network-lock modify failed: "+err.Error(), http.StatusInternalServerError) return } - - j, err := json.MarshalIndent(h.b.NetworkLockStatus(), "", "\t") - if err != nil { - http.Error(w, "JSON encoding error", 500) - return - } - w.Header().Set("Content-Type", "application/json") - w.Write(j) + w.WriteHeader(204) } func (h *Handler) serveTKADisable(w http.ResponseWriter, r *http.Request) {