From bcb05a434540c0e6b936746a2dfa878e6d28b1cb Mon Sep 17 00:00:00 2001 From: Alberto Ricart Date: Thu, 7 Mar 2024 11:12:54 -0400 Subject: [PATCH] [FIX] fix panic on missing operator keys on push with --prune (#642) * [FIX] panic when pushing with a request to prune but the operator keys are not available in the keychain. * [FIX] push cmd when encoding the delete request if there was an error encoding, the operation continued, but shouldn't. * [FEAT] added support for the -K flag to specify the operator key on push --prune operations when the operator key is not in the keychain. --- cmd/push.go | 43 +++++++++++++++++++++++++++---------------- cmd/push_test.go | 48 +++++++++++++++++++++++++++++++++++++++++------- go.mod | 2 +- go.sum | 16 ++-------------- 4 files changed, 71 insertions(+), 38 deletions(-) diff --git a/cmd/push.go b/cmd/push.go index c5506fa4..85bf0f06 100644 --- a/cmd/push.go +++ b/cmd/push.go @@ -20,6 +20,7 @@ import ( "errors" "fmt" "net/url" + "reflect" "strings" "time" @@ -397,7 +398,7 @@ func multiRequest(nc *nats.Conn, timeout int, report *store.Report, operation st end := start.Add(time.Second * time.Duration(timeout)) for ; end.After(now); now = time.Now() { // try with decreasing timeout until we dont get responses if resp, err := sub.NextMsg(end.Sub(now)); err != nil { - if err != nats.ErrTimeout || responses == 0 { + if !errors.Is(err, nats.ErrTimeout) || responses == 0 { report.AddError("failed to get response to %s: %v", operation, err) } } else if ok, srv, data := processResponse(report, resp); ok { @@ -410,16 +411,16 @@ func multiRequest(nc *nats.Conn, timeout int, report *store.Report, operation st return responses } -func obtainRequestKey(ctx ActionCtx, subPrune *store.Report) (nkeys.KeyPair, string, error) { +func obtainRequestKey(ctx ActionCtx, subPrune *store.Report) (nkeys.KeyPair, error) { opc, err := ctx.StoreCtx().Store.ReadOperatorClaim() if err != nil { subPrune.AddError("Operator needed to prune (err:%v)", err) - return nil, "", err + return nil, err } keys, err := ctx.StoreCtx().GetOperatorKeys() if err != nil { subPrune.AddError("Operator keys needed to prune (err:%v)", err) - return nil, "", err + return nil, err } if opc.StrictSigningKeyUsage { if len(keys) > 1 { @@ -428,22 +429,25 @@ func obtainRequestKey(ctx ActionCtx, subPrune *store.Report) (nkeys.KeyPair, str keys = []string{} } } - var okp nkeys.KeyPair for _, k := range keys { - if okp, err = ctx.StoreCtx().KeyStore.GetKeyPair(k); err == nil { - break + kp, err := ctx.StoreCtx().KeyStore.GetKeyPair(k) + if err != nil { + return nil, err + } + if kp != nil && !reflect.ValueOf(kp).IsNil() { + return kp, nil } } - if okp == nil { - subPrune.AddError("Operator private key needed to prune (err:%v)", err) - return nil, "", err - } - opPk, err := okp.PublicKey() + + // if we are here we don't have it - see if it was provided by the + kp, err := ctx.StoreCtx().ResolveKey(KeyPathFlag, nkeys.PrefixByteOperator) if err != nil { - subPrune.AddError("Public key needed to prune (err:%v)", err) - return nil, "", err + return nil, err + } + if kp != nil && !reflect.ValueOf(kp).IsNil() { + return kp, nil } - return okp, opPk, nil + return nil, errors.New("operator keys needed to prune: no operator keys were not found in the keystore") } func sendDeleteRequest(ctx ActionCtx, nc *nats.Conn, timeout int, report *store.Report, deleteList []string, expectedResponses int) { @@ -451,18 +455,25 @@ func sendDeleteRequest(ctx ActionCtx, nc *nats.Conn, timeout int, report *store. report.AddOK("nothing to prune") return } - okp, opPk, err := obtainRequestKey(ctx, report) + okp, err := obtainRequestKey(ctx, report) if err != nil { report.AddError("Could not obtain Operator key to sign the delete request (err:%v)", err) return } defer okp.Wipe() + opPk, err := okp.PublicKey() + if err != nil { + report.AddError("Error decoding the operator public key (err:%v)", err) + return + } + claim := jwt.NewGenericClaims(opPk) claim.Data["accounts"] = deleteList pruneJwt, err := claim.Encode(okp) if err != nil { report.AddError("Could not encode delete request (err:%v)", err) + return } respPrune := multiRequest(nc, timeout, report, "prune", "$SYS.REQ.CLAIMS.DELETE", []byte(pruneJwt), func(srv string, data interface{}) { if dataMap, ok := data.(map[string]interface{}); ok { diff --git a/cmd/push_test.go b/cmd/push_test.go index f86376f4..07c3fab6 100644 --- a/cmd/push_test.go +++ b/cmd/push_test.go @@ -135,17 +135,19 @@ func Test_SyncManualServer(t *testing.T) { func deleteSetup(t *testing.T, del bool) (string, []string, *TestStore) { t.Helper() - ts := NewEmptyStore(t) - _, _, err := ExecuteCmd(createAddOperatorCmd(), "--name", "OP", "--sys") + ts := NewTestStore(t, "O") + ts.AddAccount(t, "SYS") + ts.AddAccount(t, "AC1") + ts.AddAccount(t, "AC2") + + _, _, err := ExecuteCmd(createEditOperatorCmd(), "--system-account", "SYS") require.NoError(t, err) + serverconf := filepath.Join(ts.Dir, "server.conf") _, _, err = ExecuteCmd(createServerConfigCmd(), "--nats-resolver", "--config-file", serverconf) require.NoError(t, err) - _, _, err = ExecuteCmd(CreateAddAccountCmd(), "--name", "AC1") - require.NoError(t, err) - _, _, err = ExecuteCmd(CreateAddAccountCmd(), "--name", "AC2") - require.NoError(t, err) + // modify the generated file so testing becomes easier by knowing where the jwt directory is data, err := os.ReadFile(serverconf) require.NoError(t, err) @@ -174,10 +176,42 @@ func deleteSetup(t *testing.T, del bool) (string, []string, *TestStore) { return dir, filesPre, ts } +func Test_SyncNatsResolverDeleteNoOperatorKey(t *testing.T) { + _, _, ts := deleteSetup(t, true) + defer ts.Done(t) + + opk, err := ts.OperatorKey.PublicKey() + require.NoError(t, err) + require.NoError(t, ts.KeyStore.Remove(opk)) + + _, stderr, err := ExecuteCmd(createPushCmd(), "--prune") + t.Log(stderr) + require.Error(t, err) +} + +func Test_SyncNatsResolverDeleteOperatorKeyInFlag(t *testing.T) { + _, _, ts := deleteSetup(t, true) + defer ts.Done(t) + + okp := ts.OperatorKey + seed, err := okp.Seed() + require.NoError(t, err) + + opk, err := ts.OperatorKey.PublicKey() + require.NoError(t, err) + require.NoError(t, ts.KeyStore.Remove(opk)) + + cmd := createPushCmd() + HoistRootFlags(cmd) + _, _, err = ExecuteCmd(cmd, "--prune", "-K", string(seed)) + require.NoError(t, err) +} + func Test_SyncNatsResolverDelete(t *testing.T) { dir, filesPre, ts := deleteSetup(t, true) defer ts.Done(t) - _, _, err := ExecuteCmd(createPushCmd(), "--prune", "--system-account", "SYS", "--system-user", "sys") + + _, _, err := ExecuteCmd(createPushCmd(), "--prune") require.NoError(t, err) // test to assure AC1/SYS where pushed/pruned filesPost, err := filepath.Glob(dir + string(os.PathSeparator) + "*.jwt") diff --git a/go.mod b/go.mod index 536070b7..52bea1dc 100644 --- a/go.mod +++ b/go.mod @@ -27,7 +27,7 @@ require ( github.com/cpuguy83/go-md2man/v2 v2.0.3 // indirect github.com/davecgh/go-spew v1.1.1 // indirect github.com/fatih/color v1.16.0 // indirect - github.com/golang/protobuf v1.5.3 // indirect + github.com/golang/protobuf v1.5.4 // indirect github.com/google/go-github/v30 v30.1.0 // indirect github.com/google/go-querystring v1.1.0 // indirect github.com/inconshreveable/go-update v0.0.0-20160112193335-8152e7eb6ccf // indirect diff --git a/go.sum b/go.sum index 1423663d..b82eb787 100644 --- a/go.sum +++ b/go.sum @@ -24,8 +24,8 @@ github.com/golang/protobuf v1.2.0/go.mod h1:6lQm79b+lXiMfvg/cZm0SGofjICqVBUtrP5y github.com/golang/protobuf v1.3.2/go.mod h1:6lQm79b+lXiMfvg/cZm0SGofjICqVBUtrP5yJMmIC1U= github.com/golang/protobuf v1.5.0/go.mod h1:FsONVRAS9T7sI+LIUmWTfcYkHO4aIWwzhcaSAoJOfIk= github.com/golang/protobuf v1.5.2/go.mod h1:XVQd3VNwM+JqD3oG2Ue2ip4fOMUkwXdXDdiuN0vRsmY= -github.com/golang/protobuf v1.5.3 h1:KhyjKVUg7Usr/dYsdSqoFveMYd5ko72D+zANwlG1mmg= -github.com/golang/protobuf v1.5.3/go.mod h1:XVQd3VNwM+JqD3oG2Ue2ip4fOMUkwXdXDdiuN0vRsmY= +github.com/golang/protobuf v1.5.4 h1:i7eJL8qZTpSEXOPTxNKhASYpMn+8e5Q6AdndVa1dWek= +github.com/golang/protobuf v1.5.4/go.mod h1:lnTiLA8Wa4RWRcIUkrtSVa5nRhsEGBg48fD6rSs7xps= github.com/google/go-cmp v0.5.2/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE= github.com/google/go-cmp v0.5.5/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE= github.com/google/go-cmp v0.5.9 h1:O2Tfq5qg4qc4AmwVlvv0oLiVAGB7enBSJ2x2DqQFi38= @@ -114,8 +114,6 @@ golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACk golang.org/x/crypto v0.0.0-20190530122614-20be4c3c3ed5/go.mod h1:yigFU9vqHzYiE8UmvKecakEJjdnWj3jj499lnFckfCI= golang.org/x/crypto v0.0.0-20201221181555-eec23a3978ad/go.mod h1:jdWPYTVW3xRLrWPugEBEK3UY2ZEsg3UU495nc5E+M+I= golang.org/x/crypto v0.0.0-20210921155107-089bfa567519/go.mod h1:GvvjBRRGRdwPK5ydBHafDWAxML/pGHZbMvKqRZ5+Abc= -golang.org/x/crypto v0.19.0 h1:ENy+Az/9Y1vSrlrvBSyna3PITt4tiZLf7sgCjZBX7Wo= -golang.org/x/crypto v0.19.0/go.mod h1:Iy9bg/ha4yyC70EfRS8jz+B6ybOBKMaSxLj6P6oBDfU= golang.org/x/crypto v0.21.0 h1:X31++rzVUdKhX5sWmSOFZxx8UW/ldWx55cbf08iNAMA= golang.org/x/crypto v0.21.0/go.mod h1:0BP7YvVV9gBbVKyeTG0Gyn+gZm94bibOW5BjDEYAOMs= golang.org/x/mod v0.6.0-dev.0.20220419223038-86c51ed26bb4/go.mod h1:jJ57K6gSWd91VN4djpZkiMVwK6gcyfeH4XE8wZrZaV4= @@ -126,14 +124,10 @@ golang.org/x/net v0.0.0-20190404232315-eb5bcb51f2a3/go.mod h1:t9HGtf8HONx5eT2rtn golang.org/x/net v0.0.0-20190620200207-3b0461eec859/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s= golang.org/x/net v0.0.0-20210226172049-e18ecbb05110/go.mod h1:m0MpNAwzfU5UDzcl9v0D8zg8gWTRqZa9RBIspLL5mdg= golang.org/x/net v0.0.0-20220722155237-a158d28d115b/go.mod h1:XRhObCWvk6IyKnWLug+ECip1KBveYUHfp+8e9klMJ9c= -golang.org/x/net v0.21.0 h1:AQyQV4dYCvJ7vGmJyKki9+PBdyvhkSd8EIx/qb0AYv4= -golang.org/x/net v0.21.0/go.mod h1:bIjVDfnllIU7BJ2DNgfnXvpSvtn8VRwhlsaeUTyUS44= golang.org/x/net v0.22.0 h1:9sGLhx7iRIHEiX0oAJ3MRZMUCElJgy7Br1nO+AMN3Tc= golang.org/x/net v0.22.0/go.mod h1:JKghWKKOSdJwpW2GEx0Ja7fmaKnMsbu+MWVZTokSYmg= golang.org/x/oauth2 v0.0.0-20180821212333-d2e6202438be/go.mod h1:N/0e6XlmueqKjAGxoOufVs8QHGRruUQn6yWY3a++T0U= golang.org/x/oauth2 v0.0.0-20181106182150-f42d05182288/go.mod h1:N/0e6XlmueqKjAGxoOufVs8QHGRruUQn6yWY3a++T0U= -golang.org/x/oauth2 v0.17.0 h1:6m3ZPmLEFdVxKKWnKq4VqZ60gutO35zm+zrAHVmHyDQ= -golang.org/x/oauth2 v0.17.0/go.mod h1:OzPDGQiuQMguemayvdylqddI7qcD9lnSDb+1FiwQ5HA= golang.org/x/oauth2 v0.18.0 h1:09qnuIAgzdx1XplqJvW6CQqMCtGZykZWcXzPMPUusvI= golang.org/x/oauth2 v0.18.0/go.mod h1:Wf7knwG0MPoWIMMBgFlEaSUDaKskp0dCfrlJRJXbBi8= golang.org/x/sync v0.0.0-20180314180146-1d60e4601c6f/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= @@ -152,15 +146,11 @@ golang.org/x/sys v0.0.0-20220520151302-bc2c85ada10a/go.mod h1:oPkhp1MJrh7nUepCBc golang.org/x/sys v0.0.0-20220722155257-8c9f86f7a55f/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.0.0-20220811171246-fbc7d0a398ab/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.6.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= -golang.org/x/sys v0.17.0 h1:25cE3gD+tdBA7lp7QfhuV+rJiE9YXTcS3VG1SqssI/Y= -golang.org/x/sys v0.17.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA= golang.org/x/sys v0.18.0 h1:DBdB3niSjOA/O0blCZBqDefyWNYveAYMNF1Wum0DYQ4= golang.org/x/sys v0.18.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA= golang.org/x/term v0.0.0-20201117132131-f5c789dd3221/go.mod h1:Nr5EML6q2oocZ2LXRh80K7BxOlk5/8JxuGnuhpl+muw= golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1/go.mod h1:bj7SfCRtBDWHUb9snDiAeCFNEtKQo2Wmx5Cou7ajbmo= golang.org/x/term v0.0.0-20210927222741-03fcf44c2211/go.mod h1:jbD1KX2456YbFQfuXm/mYQcufACuNUgVhRMnK/tPxf8= -golang.org/x/term v0.17.0 h1:mkTF7LCd6WGJNL3K1Ad7kwxNfYAW6a8a8QqtMblp/4U= -golang.org/x/term v0.17.0/go.mod h1:lLRBjIVuehSbZlaOtGMbcMncT+aqLLLmKrsjNrUguwk= golang.org/x/term v0.18.0 h1:FcHjZXDMxI8mM3nwhX9HlKop4C0YQvCVCdwYl2wOtE8= golang.org/x/term v0.18.0/go.mod h1:ILwASektA3OnRv7amZ1xhE/KTR+u50pbXfZ03+6Nx58= golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ= @@ -184,8 +174,6 @@ google.golang.org/appengine v1.6.8 h1:IhEN5q69dyKagZPYMSdIjS2HqprW324FRQZJcGqPAs google.golang.org/appengine v1.6.8/go.mod h1:1jJ3jBArFh5pcgW8gCtRJnepW8FzD1V44FJffLiz/Ds= google.golang.org/protobuf v1.26.0-rc.1/go.mod h1:jlhhOSvTdKEhbULTjvd4ARK9grFBp09yW+WbY/TyQbw= google.golang.org/protobuf v1.26.0/go.mod h1:9q0QmTI4eRPtz6boOQmLYwt+qCgq0jsYwAQnmE0givc= -google.golang.org/protobuf v1.32.0 h1:pPC6BG5ex8PDFnkbrGU3EixyhKcQ2aDuBS36lqK/C7I= -google.golang.org/protobuf v1.32.0/go.mod h1:c6P6GXX6sHbq/GpV6MGZEdwhWPcYBgnhAHhKbcUYpos= google.golang.org/protobuf v1.33.0 h1:uNO2rsAINq/JlFpSdYEKIZ0uKD/R9cpdv0T+yoGwGmI= google.golang.org/protobuf v1.33.0/go.mod h1:c6P6GXX6sHbq/GpV6MGZEdwhWPcYBgnhAHhKbcUYpos= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0=