Skip to content

Commit

Permalink
[FIX] fix panic on missing operator keys on push with --prune (#642)
Browse files Browse the repository at this point in the history 
* [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.
  • Loading branch information
@aricart
aricart authored Mar 7, 2024
1 parent f85538f commit bcb05a4
Show file tree
Hide file tree
Showing 4 changed files with 71 additions and 38 deletions.
43 changes: 27 additions & 16 deletions cmd/push.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
"errors"
"fmt"
"net/url"
"reflect"
"strings"
"time"

Expand Down Expand Up @@ -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 {
Expand All @@ -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 {
Expand All @@ -428,41 +429,51 @@ 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) {
if len(deleteList) == 0 {
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 {
Expand Down
48 changes: 41 additions & 7 deletions cmd/push_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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")
Expand Down
2 changes: 1 addition & 1 deletion go.mod
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
16 changes: 2 additions & 14 deletions go.sum
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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=
Expand Down Expand Up @@ -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=
Expand All @@ -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=
Expand All @@ -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=
Expand All @@ -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=
Expand Down

0 comments on commit bcb05a4

Please sign in to comment.