crypt/step-ca-cli
crypt/step-ca-cli
1
0
Fork 0
mirror of https://github.com/smallstep/cli.git synced 2025-04-19 10:42:15 +03:00
Code

Fix some provisioner prompt issues related to SCEP and policies

Browse Source 
This PR fixes the following issues:

 - SCEP provisioners not detected in admin token flows
 - Invalid provisioner selection logic when managing provisioner policies
 - Unexpected error messages showing "issuer" instead or "provisioner" flag
This commit is contained in:
Herman Slatman 2025-03-20 11:35:02 +01:00
parent 2122975d75
commit 1cec31e5b4
No known key found for this signature in database
GPG Key ID: F4D8A44EA0A75A4F
7 changed files with 93 additions and 33 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

5
command/ca/policy/acme/acme.go
View File

@ -8,6 +8,7 @@ import (
"github.com/smallstep/cli/command/ca/policy/actions"
"github.com/smallstep/cli/command/ca/policy/policycontext"
"github.com/smallstep/cli/command/ca/policy/x509"
"github.com/smallstep/cli/internal/provisionerflag"
)
// Command returns the ACME account policy subcommand.
@ -27,5 +28,9 @@ Please note that certificate issuance policies for ACME accounts are currently o
actions.RemoveCommand(ctx),
x509.Command(ctx),
},
Before: func(ctx *cli.Context) error {
provisionerflag.Ignore()
return nil
},
}
}

5
command/ca/policy/provisioner/provisioner.go
View File

@ -9,6 +9,7 @@ import (
"github.com/smallstep/cli/command/ca/policy/policycontext"
"github.com/smallstep/cli/command/ca/policy/ssh"
"github.com/smallstep/cli/command/ca/policy/x509"
"github.com/smallstep/cli/internal/provisionerflag"
)
// Command returns the policy subcommand.
@ -29,5 +30,9 @@ Please note that certificate issuance policies on the provisioner level are curr
x509.Command(ctx),
ssh.Command(ctx),
},
Before: func(ctx *cli.Context) error {
provisionerflag.Ignore()
return nil
},
}
}

14
flags/flags.go
View File

@ -673,18 +673,24 @@ func parseCaURL(ctx *cli.Context, caURL string) (string, error) {
// FirstStringOf returns the value of the first defined flag from the input list.
// If no defined flags, returns first flag with non-empty default value.
func FirstStringOf(ctx *cli.Context, flags ...string) string {
func FirstStringOf(ctx *cli.Context, flags ...string) (string, string) {
// Return first defined flag.
for _, f := range flags {
if ctx.IsSet(f) {
return ctx.String(f)
return ctx.String(f), f
}
}
// Return first non-empty, default, flag value.
for _, f := range flags {
if val := ctx.String(f); val != "" {
return val
return val, f
}
}
return ""
var name = "<unknown>"
if len(flags) > 0 {
name = flags[0]
}
return "", name
}

29
flags/flags_test.go
View File

@ -11,6 +11,7 @@ import (
"testing"
"github.com/smallstep/assert"
"github.com/stretchr/testify/require"
"github.com/urfave/cli"
"go.step.sm/crypto/fingerprint"
)
@ -242,6 +243,7 @@ func TestFirstStringOf(t *testing.T) {
getContext func() *cli.Context
inputs []string
want string
wantName string
}{
{
name: "no-flags-empty",
@ -250,8 +252,9 @@ func TestFirstStringOf(t *testing.T) {
//_ = set.String("ca-url", "", "")
return cli.NewContext(app, set, nil)
},
inputs: []string{"foo", "bar"},
want: "",
inputs: []string{"foo", "bar"},
want: "",
wantName: "foo",
},
{
name: "return-first-set-flag",
@ -265,8 +268,9 @@ func TestFirstStringOf(t *testing.T) {
ctx.Set("baz", "test2")
return ctx
},
inputs: []string{"foo", "bar", "baz"},
want: "test1",
inputs: []string{"foo", "bar", "baz"},
want: "test1",
wantName: "bar",
},
{
name: "return-first-default-flag",
@ -278,8 +282,9 @@ func TestFirstStringOf(t *testing.T) {
ctx := cli.NewContext(app, set, nil)
return ctx
},
inputs: []string{"foo", "bar", "baz"},
want: "test1",
inputs: []string{"foo", "bar", "baz"},
want: "test1",
wantName: "baz",
},
{
name: "all-empty",
@ -291,17 +296,17 @@ func TestFirstStringOf(t *testing.T) {
ctx := cli.NewContext(app, set, nil)
return ctx
},
inputs: []string{"foo", "bar", "baz"},
want: "",
inputs: []string{"foo", "bar", "baz"},
want: "",
wantName: "foo",
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
ctx := tt.getContext()
val := FirstStringOf(ctx, tt.inputs...)
if val != tt.want {
t.Errorf("expected %v, but got %v", tt.want, val)
}
val, name := FirstStringOf(ctx, tt.inputs...)
require.Equal(t, tt.want, val)
require.Equal(t, tt.wantName, name)
})
}
}

17
internal/provisionerflag/ignore.go Normal file
View File

@ -0,0 +1,17 @@
package provisionerflag
import (
"sync/atomic"
)
var disabled atomic.Bool
// Ignore marks the provisionerflag to be ignored
func Ignore() {
disabled.Store(true)
}
// ShouldBeIgnored returns true if the provisioner flag should be ignored
func ShouldBeIgnored() bool {
return disabled.Load()
}

10
utils/cautils/offline.go
View File

@ -12,15 +12,17 @@ import (
"time"
"github.com/pkg/errors"
"github.com/urfave/cli"
"golang.org/x/crypto/ssh"
"github.com/smallstep/certificates/api"
"github.com/smallstep/certificates/authority"
"github.com/smallstep/certificates/authority/config"
"github.com/smallstep/certificates/authority/provisioner"
"github.com/smallstep/cli/utils"
"github.com/urfave/cli"
"go.step.sm/crypto/pemutil"
"go.step.sm/crypto/x509util"
"golang.org/x/crypto/ssh"
"github.com/smallstep/cli/utils"
)
// OfflineCA is a wrapper on top of the certificates authority methods that is
@ -585,6 +587,8 @@ func (c *OfflineCA) GenerateToken(ctx *cli.Context, tokType int, subject string,
return p.GetIdentityToken(subject, c.CaURL())
case *provisioner.ACME: // Return an error with the provisioner ID.
return "", &ACMETokenError{p.GetName()}
case *provisioner.SCEP:
return "", &SCEPTokenError{p.GetName()}
default: // Default is assumed to be a standard JWT.
jwkP, ok := p.(*provisioner.JWK)
if !ok {

46
utils/cautils/token_flow.go
View File

@ -15,6 +15,7 @@ import (
"github.com/smallstep/cli-utils/ui"
"github.com/smallstep/cli/flags"
"github.com/smallstep/cli/internal/provisionerflag"
"github.com/smallstep/cli/utils"
)
@ -86,6 +87,17 @@ func (e *ACMETokenError) Error() string {
return "step ACME provisioners do not support token auth flows"
}
// SCEPTokenError is the error type returned when the user attempts a Token Flow
// while using a SCEP provisioner.
type SCEPTokenError struct {
Name string
}
// Error implements the error interface.
func (e *SCEPTokenError) Error() string {
return "step SCEP provisioners do not support token auth flows"
}
// NewTokenFlow implements the common flow used to generate a token
func NewTokenFlow(ctx *cli.Context, tokType int, subject string, sans []string, caURL, root string, notBefore, notAfter time.Time, certNotBefore, certNotAfter provisioner.TimeDuration, opts ...Option) (string, error) {
// Apply options to shared context
@ -164,6 +176,8 @@ func NewTokenFlow(ctx *cli.Context, tokType int, subject string, sans []string,
return p.GetIdentityToken(subject, caURL)
case *provisioner.ACME: // Return an error with the provisioner ID.
return "", &ACMETokenError{p.GetName()}
case *provisioner.SCEP:
return "", &SCEPTokenError{p.GetName()}
default:
return "", errors.Errorf("unknown provisioner type %T", p)
}
@ -212,13 +226,13 @@ func OfflineTokenFlow(ctx *cli.Context, typ int, subject string, sans []string,
}
kid := ctx.String("kid")
issuer := flags.FirstStringOf(ctx, "provisioner", "issuer")
issuer, flag := flags.FirstStringOf(ctx, "provisioner", "issuer")
// Require issuer and keyFile if ca.json does not exists.
// kid can be passed or created using jwk.Thumbprint.
switch {
case issuer == "":
return "", errs.RequiredWithFlag(ctx, "offline", "issuer")
return "", errs.RequiredWithFlag(ctx, "offline", flag)
case ctx.String("key") == "":
return "", errs.RequiredWithFlag(ctx, "offline", "key")
}
@ -293,7 +307,7 @@ func provisionerPrompt(ctx *cli.Context, provisioners provisioner.List) (provisi
provisioners = provisionerFilter(provisioners, func(p provisioner.Interface) bool {
switch p.GetType() {
case provisioner.TypeJWK, provisioner.TypeOIDC,
provisioner.TypeACME, provisioner.TypeK8sSA,
provisioner.TypeACME, provisioner.TypeSCEP, provisioner.TypeK8sSA,
provisioner.TypeX5C, provisioner.TypeSSHPOP, provisioner.TypeNebula:
return true
case provisioner.TypeGCP, provisioner.TypeAWS, provisioner.TypeAzure:
@ -325,16 +339,6 @@ func provisionerPrompt(ctx *cli.Context, provisioners provisioner.List) (provisi
}
}
// Filter by issuer (provisioner name)
if issuer := flags.FirstStringOf(ctx, "provisioner", "issuer"); issuer != "" {
provisioners = provisionerFilter(provisioners, func(p provisioner.Interface) bool {
return p.GetName() == issuer
})
if len(provisioners) == 0 {
return nil, errs.InvalidFlagValue(ctx, "issuer", issuer, "")
}
}
// Filter by admin-provisioner (provisioner name)
if issuer := ctx.String("admin-provisioner"); issuer != "" {
provisioners = provisionerFilter(provisioners, func(p provisioner.Interface) bool {
@ -345,6 +349,20 @@ func provisionerPrompt(ctx *cli.Context, provisioners provisioner.List) (provisi
}
}
// Filter by provisioner / issuer (provisioner name)
if issuer, flag := flags.FirstStringOf(ctx, "provisioner", "issuer"); issuer != "" {
provisioners = provisionerFilter(provisioners, func(p provisioner.Interface) bool {
if provisionerflag.ShouldBeIgnored() {
return true // fake match; effectively skipping provisioner flag value for provisioner-dependent policy commands
}
return p.GetName() == issuer
})
if len(provisioners) == 0 {
return nil, errs.InvalidFlagValue(ctx, flag, issuer, "")
}
}
// Select provisioner
var items []*provisionersSelect
for _, prov := range provisioners {
@ -364,7 +382,7 @@ func provisionerPrompt(ctx *cli.Context, provisioners provisioner.List) (provisi
Name: fmt.Sprintf("%s (%s) [tenant: %s]", p.Name, p.GetType(), p.TenantID),
Provisioner: p,
})
case *provisioner.GCP, *provisioner.AWS, *provisioner.X5C, *provisioner.SSHPOP, *provisioner.ACME, *provisioner.Nebula:
case *provisioner.GCP, *provisioner.AWS, *provisioner.X5C, *provisioner.SSHPOP, *provisioner.ACME, *provisioner.SCEP, *provisioner.Nebula:
items = append(items, &provisionersSelect{
Name: fmt.Sprintf("%s (%s)", p.GetName(), p.GetType()),
Provisioner: p,