Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add handling of cnf claim #1092

Merged
merged 6 commits into from
Jul 24, 2024
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion command/ca/sign.go
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,7 @@
}

// certificate flow unifies online and offline flows on a single api
flow, err := cautils.NewCertificateFlow(ctx)
flow, err := cautils.NewCertificateFlow(ctx, cautils.WithCertificateRequest(csr))

Check warning on line 178 in command/ca/sign.go

View check run for this annotation

Codecov / codecov/patch

command/ca/sign.go#L178

Added line #L178 was not covered by tests
if err != nil {
return err
}
Expand Down
48 changes: 47 additions & 1 deletion command/ca/token.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
"fmt"
"os"

"github.com/pkg/errors"
"github.com/smallstep/certificates/api"
"github.com/smallstep/certificates/pki"
"github.com/smallstep/cli/flags"
Expand All @@ -12,6 +13,8 @@
"github.com/urfave/cli"
"go.step.sm/cli-utils/command"
"go.step.sm/cli-utils/errs"
"go.step.sm/crypto/pemutil"
"golang.org/x/crypto/ssh"
)

func tokenCommand() cli.Command {
Expand All @@ -27,6 +30,7 @@
[**--output-file**=<file>] [**--kms**=uri] [**--key**=<file>] [**--san**=<SAN>] [**--offline**]
[**--revoke**] [**--x5c-cert**=<file>] [**--x5c-key**=<file>] [**--x5c-insecure**]
[**--sshpop-cert**=<file>] [**--sshpop-key**=<file>]
[**--cnf-file**=<file>] [**--cnf-kid**=<fingerprint>]
[**--ssh**] [**--host**] [**--principal**=<name>] [**--k8ssa-token-path**=<file>]
[**--ca-url**=<uri>] [**--root**=<file>] [**--context**=<name>]`,
Description: `**step ca token** command generates a one-time token granting access to the
Expand Down Expand Up @@ -82,6 +86,11 @@
$ step ca token --not-before 30m --not-after 35m internal.example.com
'''

Get a new token with a confirmation claim to enforce the use of a given CSR:
'''
step ca token --cnf-file internal.csr internal.smallstep.com
'''

Get a new token signed with the given private key, the public key must be
configured in the certificate authority:
'''
Expand Down Expand Up @@ -133,6 +142,11 @@
$ step ca token my-remote.hostname --ssh --host
'''

Get a new token with a confirmation claim to enforce the use of a given public key:
'''
step ca token --ssh --host --cnf-file internal.pub internal.smallstep.com
'''

Generate a renew token and use it in a renew after expiry request:
'''
$ TOKEN=$(step ca token --x5c-cert internal.crt --x5c-key internal.key --renew internal.example.com)
Expand Down Expand Up @@ -186,6 +200,8 @@
flags.SSHPOPKey,
flags.NebulaCert,
flags.NebulaKey,
flags.ConfirmationFile,
flags.ConfirmationKid,
hslatman marked this conversation as resolved.
Show resolved Hide resolved
cli.StringFlag{
Name: "key",
Usage: `The private key <file> used to sign the JWT. This is usually downloaded from
Expand Down Expand Up @@ -240,6 +256,9 @@
isSSH := ctx.Bool("ssh")
isHost := ctx.Bool("host")
principals := ctx.StringSlice("principal")
// confirmation claims
cnfFile := ctx.String("cnf-file")
cnfKid := ctx.String("cnf-kid")

Check warning on line 261 in command/ca/token.go

View check run for this annotation

Codecov / codecov/patch

command/ca/token.go#L259-L261

Added lines #L259 - L261 were not covered by tests

switch {
case isSSH && len(sans) > 0:
Expand All @@ -252,6 +271,8 @@
return errs.RequiredWithFlag(ctx, "host", "ssh")
case !isSSH && len(principals) > 0:
return errs.RequiredWithFlag(ctx, "principal", "ssh")
case cnfFile != "" && cnfKid != "":
return errs.IncompatibleFlagWithFlag(ctx, "cnf-file", "cnf-kid")

Check warning on line 275 in command/ca/token.go

View check run for this annotation

Codecov / codecov/patch

command/ca/token.go#L274-L275

Added lines #L274 - L275 were not covered by tests
}

// Default token type is always a 'Sign' token.
Expand Down Expand Up @@ -295,6 +316,31 @@
}
}

// Add options to create a confirmation claim if a CSR or SSH public key is
// passed.
var tokenOpts []cautils.Option
if cnfFile != "" {
in, err := utils.ReadFile(cnfFile)
if err != nil {
return err
}
if isSSH {
sshPub, _, _, _, err := ssh.ParseAuthorizedKey(in)
if err != nil {
return errors.Wrap(err, "error parsing ssh public key")
}
tokenOpts = append(tokenOpts, cautils.WithSSHPublicKey(sshPub))
} else {
csr, err := pemutil.ParseCertificateRequest(in)
if err != nil {
return errors.Wrap(err, "error parsing certificate request")
}
tokenOpts = append(tokenOpts, cautils.WithCertificateRequest(csr))

Check warning on line 338 in command/ca/token.go

View check run for this annotation

Codecov / codecov/patch

command/ca/token.go#L321-L338

Added lines #L321 - L338 were not covered by tests
Comment on lines +334 to +345
Copy link
Member

@hslatman hslatman Jul 24, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of passing the CSR or SSH public key, the claim itself could be calculated here, and just pass it as the fingerprint directly? Or is there some flow in which to defer calculation of the fingerprint is necessary? step ca sign, maybe?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

step ca sign use cautils.WithCertificateRequest(csr) and step ssh certificate use cautils.WithSSHPublicKey(sshPub), although the SSH one is currently totally ignored. These fields go to the shared context of the cautils package and end up in the token package, which takes care of calculating the fingerprint and adding the proper attribute to the final token. Right now, only the CSR gets into the token package, but sending the type allows us to specify the right attribute now that we don't share just kid.

}
} else if cnfKid != "" {
tokenOpts = append(tokenOpts, cautils.WithConfirmationKid(cnfKid))
}

Check warning on line 342 in command/ca/token.go

View check run for this annotation

Codecov / codecov/patch

command/ca/token.go#L340-L342

Added lines #L340 - L342 were not covered by tests

// --san and --type revoke are incompatible. Revocation tokens do not support SANs.
if typ == cautils.RevokeType && len(sans) > 0 {
return errs.IncompatibleFlagWithFlag(ctx, "san", "revoke")
Expand Down Expand Up @@ -327,7 +373,7 @@
return err
}
} else {
token, err = cautils.NewTokenFlow(ctx, typ, subject, sans, caURL, root, notBefore, notAfter, certNotBefore, certNotAfter)
token, err = cautils.NewTokenFlow(ctx, typ, subject, sans, caURL, root, notBefore, notAfter, certNotBefore, certNotAfter, tokenOpts...)

Check warning on line 376 in command/ca/token.go

View check run for this annotation

Codecov / codecov/patch

command/ca/token.go#L376

Added line #L376 was not covered by tests
if err != nil {
return err
}
Expand Down
18 changes: 16 additions & 2 deletions command/certificate/fingerprint.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@
[**--bundle**] [**--roots**=<root-bundle>] [**--servername**=<servername>]
[**--format**=<format>] [**--sha1**] [**--insecure**]`,
Description: `**step certificate fingerprint** reads a certificate and prints to STDOUT the
certificate SHA256 of the raw certificate.
certificate SHA256 of the raw certificate or certificate signing request.

If <crt-file> contains multiple certificates (i.e., it is a certificate
"bundle") the fingerprint of the first certificate in the bundle will be
Expand Down Expand Up @@ -55,6 +55,12 @@
$ step certificate fingerprint --bundle https://smallstep.com
e2c4f12edfc1816cc610755d32e6f45d5678ba21ecda1693bb5b246e3c48c03d
25847d668eb4f04fdd40b12b6b0740c567da7d024308eb6c2c96fe41d9de218d
'''

Get the fingerprint for a CSR using base64-url without padding encoding:
'''
$ step certificate fingerprint --format base64-url-raw hello.csr
PJLNhtQoBE1yGN_ZKzr4Y2U5pyqIGiyyszkoz2raDOw
hslatman marked this conversation as resolved.
Show resolved Hide resolved
'''`,
Flags: []cli.Flag{
cli.StringFlag{
Expand Down Expand Up @@ -128,7 +134,15 @@
default:
certs, err = pemutil.ReadCertificateBundle(crtFile)
if err != nil {
return err
// Fallback to parse a CSR
csr, csrErr := pemutil.ReadCertificateRequest(crtFile)
if csrErr != nil {
return err
}

Check warning on line 141 in command/certificate/fingerprint.go

View check run for this annotation

Codecov / codecov/patch

command/certificate/fingerprint.go#L137-L141

Added lines #L137 - L141 were not covered by tests
// We will only need the raw the generate a fingerprint.
certs = []*x509.Certificate{
{Raw: csr.Raw},
}

Check warning on line 145 in command/certificate/fingerprint.go

View check run for this annotation

Codecov / codecov/patch

command/certificate/fingerprint.go#L143-L145

Added lines #L143 - L145 were not covered by tests
maraino marked this conversation as resolved.
Show resolved Hide resolved
}
}

Expand Down
68 changes: 35 additions & 33 deletions command/ssh/certificate.go
Original file line number Diff line number Diff line change
Expand Up @@ -267,7 +267,41 @@ func certificateAction(ctx *cli.Context) error {
}
}

flow, err := cautils.NewCertificateFlow(ctx)
var (
sshPub ssh.PublicKey
pub, priv interface{}
flowOptions []cautils.Option
)

if isSign {
in, err := utils.ReadFile(keyFile)
if err != nil {
return err
}

sshPub, _, _, _, err = ssh.ParseAuthorizedKey(in)
if err != nil {
return errors.Wrap(err, "error parsing ssh public key")
}
if len(sshPrivKeyFile) > 0 {
if priv, err = pemutil.Read(sshPrivKeyFile); err != nil {
return errors.Wrap(err, "error parsing private key")
}
}
flowOptions = append(flowOptions, cautils.WithSSHPublicKey(sshPub))
} else {
pub, priv, err = keyutil.GenerateDefaultKeyPair()
if err != nil {
return err
}

sshPub, err = ssh.NewPublicKey(pub)
if err != nil {
return errors.Wrap(err, "error creating public key")
}
}

flow, err := cautils.NewCertificateFlow(ctx, flowOptions...)
if err != nil {
return err
}
Expand Down Expand Up @@ -353,38 +387,6 @@ func certificateAction(ctx *cli.Context) error {
identityKey = key
}

var sshPub ssh.PublicKey
var pub, priv interface{}

if isSign {
// Use public key supplied as input.
in, err := utils.ReadFile(keyFile)
if err != nil {
return err
}

sshPub, _, _, _, err = ssh.ParseAuthorizedKey(in)
if err != nil {
return errors.Wrap(err, "error parsing ssh public key")
}
if len(sshPrivKeyFile) > 0 {
if priv, err = pemutil.Read(sshPrivKeyFile); err != nil {
return errors.Wrap(err, "error parsing private key")
}
}
} else {
// Generate keypair
pub, priv, err = keyutil.GenerateDefaultKeyPair()
if err != nil {
return err
}

sshPub, err = ssh.NewPublicKey(pub)
if err != nil {
return errors.Wrap(err, "error creating public key")
}
}

var sshAuPub ssh.PublicKey
var sshAuPubBytes []byte
var auPub, auPriv interface{}
Expand Down
15 changes: 15 additions & 0 deletions flags/flags.go
Original file line number Diff line number Diff line change
Expand Up @@ -379,6 +379,21 @@ be stored in the 'sshpop' header.`,
be stored in the 'nebula' header.`,
}

// ConfirmationFile is a cli.Flag used to add a confirmation claim in the
// tokens. It will add a confirmation kid with the fingerprint of the CSR or
// an SSH public key.
ConfirmationFile = cli.StringFlag{
Name: "cnf-file",
Usage: `The CSR or SSH public key <file> to restrict this token for.`,
}

// ConfirmationKid is a cli.Flag used to add a confirmation claim in the
// token.
ConfirmationKid = cli.StringFlag{
Name: "cnf-kid",
Usage: `The <fingerprint> of the CSR or SSH public key to restrict this token for.`,
}
hslatman marked this conversation as resolved.
Show resolved Hide resolved

// Team is a cli.Flag used to pass the team ID.
Team = cli.StringFlag{
Name: "team",
Expand Down
40 changes: 40 additions & 0 deletions token/options.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import (
"bytes"
"crypto"
"crypto/ecdh"
"crypto/ecdsa"
"crypto/ed25519"
Expand All @@ -15,9 +16,11 @@

"github.com/pkg/errors"
nebula "github.com/slackhq/nebula/cert"
"go.step.sm/crypto/fingerprint"
"go.step.sm/crypto/jose"
"go.step.sm/crypto/pemutil"
"go.step.sm/crypto/x25519"
"golang.org/x/crypto/ssh"
)

// Options is a function that set claims.
Expand Down Expand Up @@ -84,6 +87,43 @@
})
}

// WithConfirmationKid returns an Options function that sets the cnf claim with
// the given kid.
func WithConfirmationKid(kid string) Options {
return func(c *Claims) error {
c.Set(ConfirmationClaim, map[string]string{
"kid": kid,
})
return nil
}
}

// WithFingerprint returns an Options function that the cnf claims with the kid
// representing the fingerprint of the certificate request or the ssh public
// key.
func WithFingerprint(v interface{}) Options {
maraino marked this conversation as resolved.
Show resolved Hide resolved
return func(c *Claims) error {
var data []byte
switch vv := v.(type) {
case *x509.CertificateRequest:
data = vv.Raw
case ssh.PublicKey:
data = vv.Marshal()
default:
return fmt.Errorf("unsupported fingerprint for %T", vv)
}
hslatman marked this conversation as resolved.
Show resolved Hide resolved

kid, err := fingerprint.New(data, crypto.SHA256, fingerprint.Base64RawURLFingerprint)
if err != nil {
return err
}

Check warning on line 119 in token/options.go

View check run for this annotation

Codecov / codecov/patch

token/options.go#L118-L119

Added lines #L118 - L119 were not covered by tests
c.Set(ConfirmationClaim, map[string]string{
"kid": kid,
})
hslatman marked this conversation as resolved.
Show resolved Hide resolved
return nil
}
}

// WithValidity validates boundary inputs and sets the 'nbf' (NotBefore) and
// 'exp' (expiration) options.
func WithValidity(notBefore, expiration time.Time) Options {
Expand Down
23 changes: 23 additions & 0 deletions token/options_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,9 @@ import (
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
"go.step.sm/crypto/jose"
"go.step.sm/crypto/pemutil"
"go.step.sm/crypto/x25519"
"golang.org/x/crypto/ssh"
)

func TestOptions(t *testing.T) {
Expand All @@ -35,6 +37,11 @@ func TestOptions(t *testing.T) {
p256ECDHSigner, err := p256Signer.ECDH()
require.NoError(t, err)

testCSR, err := pemutil.ReadCertificateRequest("testdata/test.csr")
require.NoError(t, err)

testSSH := mustReadSSHPublicKey(t, "testdata/ssh-key.pub")

wrongNebulaContentsFilename := "testdata/ca.crt"

emptyFile, err := os.CreateTemp(tempDir, "empty-file")
Expand Down Expand Up @@ -79,6 +86,10 @@ func TestOptions(t *testing.T) {
{"WithNebulaCurve25519Cert empty file fail", WithNebulaCert(emptyFile.Name(), nil), empty, true},
{"WithNebulaCurve25519Cert invalid content fail", WithNebulaCert(c25519CertFilename, nil), empty, true},
{"WithNebulaCurve25519Cert mismatching key fail", WithNebulaCert(c25519CertFilename, p256Signer), empty, true},
{"WithConfirmationKid ok", WithConfirmationKid("my-kid"), &Claims{ExtraClaims: map[string]any{"cnf": map[string]string{"kid": "my-kid"}}}, false},
{"WithFingerprint csr ok", WithFingerprint(testCSR), &Claims{ExtraClaims: map[string]any{"cnf": map[string]string{"kid": "ak6j6CwuZbd_mOQ-pNOUwhpmtSN0mY0xrLvaQL4J5l8"}}}, false},
{"WithFingerprint ssh ok", WithFingerprint(testSSH), &Claims{ExtraClaims: map[string]any{"cnf": map[string]string{"kid": "hpTQOoB7fIRxTp-FhXCIm94mGBv7_dzr_5SxLn1Pnwk"}}}, false},
{"WithFingerprint fail", WithFingerprint("unexpected type"), empty, true},
}

for _, tt := range tests {
Expand All @@ -96,6 +107,18 @@ func TestOptions(t *testing.T) {
}
}

func mustReadSSHPublicKey(t *testing.T, filename string) ssh.PublicKey {
t.Helper()

b, err := os.ReadFile(filename)
require.NoError(t, err)

pub, _, _, _, err := ssh.ParseAuthorizedKey(b)
require.NoError(t, err)

return pub
}

func serializeAndWriteNebulaCert(t *testing.T, tempDir string, cert *nebula.NebulaCertificate) (string, []byte) {
file, err := os.CreateTemp(tempDir, "nebula-test-cert-*")
require.NoError(t, err)
Expand Down
1 change: 1 addition & 0 deletions token/testdata/ssh-key.pub
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAIF14RP3HJkO1yoZHjo9t/4bJgyJGiSPxhm6FApa3VtG1 [email protected]
8 changes: 8 additions & 0 deletions token/testdata/test.csr
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
-----BEGIN CERTIFICATE REQUEST-----
MIIBBDCBqwIBADAbMRkwFwYDVQQDDBB0ZXN0QGV4YW1wbGUuY29tMFkwEwYHKoZI
zj0CAQYIKoZIzj0DAQcDQgAEPj0tlICeGPiz361yM+AGlZmDK+N/cT0SVloozOQH
1ljdNbookliEX8eRnFnelZRaql1KhrVOXhfwBmd/eGhti6AuMCwGCSqGSIb3DQEJ
DjEfMB0wGwYDVR0RBBQwEoEQdGVzdEBleGFtcGxlLmNvbTAKBggqhkjOPQQDAgNI
ADBFAiEA4WuukEVIFJQHNqlZVsWtsWsSVLNRCxBBJfH7/+txNw4CIGyK3eo5MDvR
DepPHVRF16/b+iW/4HgAgIC90+5Q4IrL
-----END CERTIFICATE REQUEST-----
Loading
Loading