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 data support on SCEPCHALLENGE webhooks #2065

Merged
merged 2 commits into from
Nov 12, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
27 changes: 18 additions & 9 deletions authority/provisioner/scep.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import (

"go.step.sm/crypto/kms"
kmsapi "go.step.sm/crypto/kms/apiv1"
"go.step.sm/crypto/x509util"
"go.step.sm/linkedca"

"github.com/smallstep/certificates/webhook"
Expand Down Expand Up @@ -145,25 +146,33 @@ var (
// that case, the other webhooks will be skipped. If none of
// the webhooks indicates the value of the challenge was accepted,
// an error is returned.
func (c *challengeValidationController) Validate(ctx context.Context, csr *x509.CertificateRequest, provisionerName, challenge, transactionID string) error {
func (c *challengeValidationController) Validate(ctx context.Context, csr *x509.CertificateRequest, provisionerName, challenge, transactionID string) ([]SignCSROption, error) {
var opts []SignCSROption

for _, wh := range c.webhooks {
req, err := webhook.NewRequestBody(webhook.WithX509CertificateRequest(csr))
if err != nil {
return fmt.Errorf("failed creating new webhook request: %w", err)
return nil, fmt.Errorf("failed creating new webhook request: %w", err)
}
req.ProvisionerName = provisionerName
req.SCEPChallenge = challenge
req.SCEPTransactionID = transactionID
resp, err := wh.DoWithContext(ctx, c.client, req, nil) // TODO(hs): support templated URL? Requires some refactoring
if err != nil {
return fmt.Errorf("failed executing webhook request: %w", err)
return nil, fmt.Errorf("failed executing webhook request: %w", err)
}
if resp.Allow {
return nil // return early when response is positive
opts = append(opts, TemplateDataModifierFunc(func(data x509util.TemplateData) {
data.SetWebhook(wh.Name, resp.Data)
}))
}
}

return ErrSCEPChallengeInvalid
if len(opts) == 0 {
return nil, ErrSCEPChallengeInvalid
}
maraino marked this conversation as resolved.
Show resolved Hide resolved

return opts, nil
}

type notificationController struct {
Expand Down Expand Up @@ -440,18 +449,18 @@ func (s *SCEP) GetContentEncryptionAlgorithm() int {
// ValidateChallenge validates the provided challenge. It starts by
// selecting the validation method to use, then performs validation
// according to that method.
func (s *SCEP) ValidateChallenge(ctx context.Context, csr *x509.CertificateRequest, challenge, transactionID string) error {
func (s *SCEP) ValidateChallenge(ctx context.Context, csr *x509.CertificateRequest, challenge, transactionID string) ([]SignCSROption, error) {
if s.challengeValidationController == nil {
return fmt.Errorf("provisioner %q wasn't initialized", s.Name)
return nil, fmt.Errorf("provisioner %q wasn't initialized", s.Name)
}
switch s.selectValidationMethod() {
case validationMethodWebhook:
return s.challengeValidationController.Validate(ctx, csr, s.Name, challenge, transactionID)
default:
if subtle.ConstantTimeCompare([]byte(s.ChallengePassword), []byte(challenge)) == 0 {
return errors.New("invalid challenge password provided")
return nil, errors.New("invalid challenge password provided")
}
return nil
return []SignCSROption{}, nil
}
}

Expand Down
25 changes: 25 additions & 0 deletions authority/provisioner/sign_options.go
Original file line number Diff line number Diff line change
Expand Up @@ -545,3 +545,28 @@ func (s csrFingerprintValidator) Valid(cr *x509.CertificateRequest) error {
}
return nil
}

// SignCSROption is the interface used to collect extra option in the SignCSR
// method of the SCEP authority.
type SignCSROption interface{}
maraino marked this conversation as resolved.
Show resolved Hide resolved

// TemplateDataModifier in an interface that allows to modify template data.
type TemplateDataModifier interface {
maraino marked this conversation as resolved.
Show resolved Hide resolved
Modify(data x509util.TemplateData)
}

type templateDataModifier struct {
fn func(x509util.TemplateData)
}

func (t *templateDataModifier) Modify(data x509util.TemplateData) {
t.fn(data)
}

// TemplateDataModifierFunc returns a TemplateDataModifier with the given
// function.
func TemplateDataModifierFunc(fn func(data x509util.TemplateData)) TemplateDataModifier {
return &templateDataModifier{
fn: fn,
}
}
7 changes: 5 additions & 2 deletions scep/api/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -384,14 +384,17 @@ func PKIOperation(ctx context.Context, req request) (Response, error) {
// even if using the renewal flow as described in the README.md. MicroMDM SCEP client also only does PKCSreq by default, unless
// a certificate exists; then it will use RenewalReq. Adding the challenge check here may be a small breaking change for clients.
// We'll have to see how it works out.
var signCSROpts []provisioner.SignCSROption
if msg.MessageType == smallscep.PKCSReq || msg.MessageType == smallscep.RenewalReq {
if err := auth.ValidateChallenge(ctx, csr, challengePassword, transactionID); err != nil {
challengeOptions, err := auth.ValidateChallenge(ctx, csr, challengePassword, transactionID)
if err != nil {
if errors.Is(err, provisioner.ErrSCEPChallengeInvalid) {
return createFailureResponse(ctx, csr, msg, smallscep.BadRequest, err.Error(), err)
}
scepErr := errors.New("failed validating challenge password")
return createFailureResponse(ctx, csr, msg, smallscep.BadRequest, scepErr.Error(), scepErr)
}
signCSROpts = append(signCSROpts, challengeOptions...)
}

// TODO: authorize renewal: we can authorize renewals with the challenge password (if reusable secrets are used).
Expand All @@ -402,7 +405,7 @@ func PKIOperation(ctx context.Context, req request) (Response, error) {
// Authentication by the (self-signed) certificate with an optional challenge is required; supporting renewals incl. verification
// of the client cert is not.

certRep, err := auth.SignCSR(ctx, csr, msg)
certRep, err := auth.SignCSR(ctx, csr, msg, signCSROpts...)
if err != nil {
if notifyErr := auth.NotifyFailure(ctx, csr, transactionID, 0, err.Error()); notifyErr != nil {
// TODO(hs): ignore this error case? It's not critical if the notification fails; but logging it might be good
Expand Down
11 changes: 9 additions & 2 deletions scep/authority.go
Original file line number Diff line number Diff line change
Expand Up @@ -241,7 +241,7 @@ func (a *Authority) DecryptPKIEnvelope(ctx context.Context, msg *PKIMessage) err

// SignCSR creates an x509.Certificate based on a CSR template and Cert Authority credentials
// returns a new PKIMessage with CertRep data
func (a *Authority) SignCSR(ctx context.Context, csr *x509.CertificateRequest, msg *PKIMessage) (*PKIMessage, error) {
func (a *Authority) SignCSR(ctx context.Context, csr *x509.CertificateRequest, msg *PKIMessage, signCSROpts ...provisioner.SignCSROption) (*PKIMessage, error) {
// TODO: intermediate storage of the request? In SCEP it's possible to request a csr/certificate
// to be signed, which can be performed asynchronously / out-of-band. In that case a client can
// poll for the status. It seems to be similar as what can happen in ACME, so might want to model
Expand Down Expand Up @@ -284,6 +284,13 @@ func (a *Authority) SignCSR(ctx context.Context, csr *x509.CertificateRequest, m
CommonName: csr.Subject.CommonName,
})

// Apply CSR options. Currently only one option is defined.
for _, o := range signCSROpts {
if m, ok := o.(provisioner.TemplateDataModifier); ok {
m.Modify(data)
}
}

// Get authorizations from the SCEP provisioner.
ctx = provisioner.NewContextWithMethod(ctx, provisioner.SignMethod)
signOps, err := p.AuthorizeSign(ctx, "")
Expand Down Expand Up @@ -506,7 +513,7 @@ func (a *Authority) GetCACaps(ctx context.Context) []string {
return caps
}

func (a *Authority) ValidateChallenge(ctx context.Context, csr *x509.CertificateRequest, challenge, transactionID string) error {
func (a *Authority) ValidateChallenge(ctx context.Context, csr *x509.CertificateRequest, challenge, transactionID string) ([]provisioner.SignCSROption, error) {
p := provisionerFromContext(ctx)
return p.ValidateChallenge(ctx, csr, challenge, transactionID)
}
Expand Down
2 changes: 1 addition & 1 deletion scep/provisioner.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ type Provisioner interface {
GetDecrypter() (*x509.Certificate, crypto.Decrypter)
GetSigner() (*x509.Certificate, crypto.Signer)
GetContentEncryptionAlgorithm() int
ValidateChallenge(ctx context.Context, csr *x509.CertificateRequest, challenge, transactionID string) error
ValidateChallenge(ctx context.Context, csr *x509.CertificateRequest, challenge, transactionID string) ([]provisioner.SignCSROption, error)
NotifySuccess(ctx context.Context, csr *x509.CertificateRequest, cert *x509.Certificate, transactionID string) error
NotifyFailure(ctx context.Context, csr *x509.CertificateRequest, transactionID string, errorCode int, errorDescription string) error
}
Expand Down
Loading