Skip to content
Merged
Show file tree
Hide file tree
Changes from all 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
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
}

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
157 changes: 119 additions & 38 deletions authority/provisioner/scep_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import (
"go.step.sm/crypto/kms/softkms"
"go.step.sm/crypto/minica"
"go.step.sm/crypto/pemutil"
"go.step.sm/crypto/x509util"
"go.step.sm/linkedca"
)

Expand All @@ -37,6 +38,7 @@ func Test_challengeValidationController_Validate(t *testing.T) {
}
type response struct {
Allow bool `json:"allow"`
Data any `json:"data"`
}
nokServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
req := &request{}
Expand All @@ -60,11 +62,22 @@ func Test_challengeValidationController_Validate(t *testing.T) {
if assert.NotNil(t, req.Request) {
assert.Equal(t, []byte{1}, req.Request.Raw)
}
b, err := json.Marshal(response{Allow: true})
resp := response{Allow: true}
if r.Header.Get("X-Smallstep-Webhook-Id") == "webhook-id-2" {
resp.Data = map[string]any{
"ID": "2adcbfec-5e4a-4b93-8913-640e24faf101",
"Email": "admin@example.com",
}
}
b, err := json.Marshal(resp)
require.NoError(t, err)
w.WriteHeader(200)
w.Write(b)
}))
t.Cleanup(func() {
nokServer.Close()
okServer.Close()
})
type fields struct {
client *http.Client
webhooks []*Webhook
Expand All @@ -78,7 +91,7 @@ func Test_challengeValidationController_Validate(t *testing.T) {
name string
fields fields
args args
server *httptest.Server
want x509util.TemplateData
expErr error
}{
{
Expand Down Expand Up @@ -134,7 +147,6 @@ func Test_challengeValidationController_Validate(t *testing.T) {
challenge: "not-allowed",
transactionID: "transaction-1",
},
server: nokServer,
expErr: errors.New("webhook server did not allow request"),
},
{
Expand All @@ -154,26 +166,58 @@ func Test_challengeValidationController_Validate(t *testing.T) {
challenge: "challenge",
transactionID: "transaction-1",
},
server: okServer,
want: x509util.TemplateData{
x509util.WebhooksKey: map[string]any{
"webhook-name-1": nil,
},
},
},
{
name: "ok with data",
fields: fields{http.DefaultClient, []*Webhook{
{
ID: "webhook-id-2",
Name: "webhook-name-2",
Secret: "MTIzNAo=",
Kind: linkedca.Webhook_SCEPCHALLENGE.String(),
CertType: linkedca.Webhook_X509.String(),
URL: okServer.URL,
},
}},
args: args{
provisionerName: "my-scep-provisioner",
challenge: "challenge",
transactionID: "transaction-1",
},
want: x509util.TemplateData{
x509util.WebhooksKey: map[string]any{
"webhook-name-2": map[string]any{
"ID": "2adcbfec-5e4a-4b93-8913-640e24faf101",
"Email": "admin@example.com",
},
},
},
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
c := newChallengeValidationController(tt.fields.client, tt.fields.webhooks)

if tt.server != nil {
defer tt.server.Close()
}

ctx := context.Background()
err := c.Validate(ctx, dummyCSR, tt.args.provisionerName, tt.args.challenge, tt.args.transactionID)

got, err := c.Validate(ctx, dummyCSR, tt.args.provisionerName, tt.args.challenge, tt.args.transactionID)
if tt.expErr != nil {
assert.EqualError(t, err, tt.expErr.Error())
return
}

assert.NoError(t, err)
data := x509util.TemplateData{}
for _, o := range got {
if m, ok := o.(TemplateDataModifier); ok {
m.Modify(data)
} else {
t.Errorf("Validate() got = %T, want TemplateDataModifier", o)
}
}
assert.Equal(t, tt.want, data)
})
}
}
Expand Down Expand Up @@ -257,6 +301,7 @@ func TestSCEP_ValidateChallenge(t *testing.T) {
}
type response struct {
Allow bool `json:"allow"`
Data any `json:"data"`
}
okServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
req := &request{}
Expand All @@ -268,11 +313,19 @@ func TestSCEP_ValidateChallenge(t *testing.T) {
if assert.NotNil(t, req.Request) {
assert.Equal(t, []byte{1}, req.Request.Raw)
}
b, err := json.Marshal(response{Allow: true})
resp := response{Allow: true}
if r.Header.Get("X-Smallstep-Webhook-Id") == "webhook-id-2" {
resp.Data = map[string]any{
"ID": "2adcbfec-5e4a-4b93-8913-640e24faf101",
"Email": "admin@example.com",
}
}
b, err := json.Marshal(resp)
require.NoError(t, err)
w.WriteHeader(200)
w.Write(b)
}))
t.Cleanup(okServer.Close)
type args struct {
challenge string
transactionID string
Expand All @@ -282,6 +335,7 @@ func TestSCEP_ValidateChallenge(t *testing.T) {
p *SCEP
server *httptest.Server
args args
want x509util.TemplateData
expErr error
}{
{"ok/webhooks", &SCEP{
Expand All @@ -299,9 +353,43 @@ func TestSCEP_ValidateChallenge(t *testing.T) {
},
},
},
}, okServer, args{"webhook-challenge", "webhook-transaction-1"},
nil,
},
}, okServer, args{"webhook-challenge", "webhook-transaction-1"}, x509util.TemplateData{
x509util.WebhooksKey: map[string]any{
"webhook-name-1": nil,
},
}, nil},
{"ok/with-data", &SCEP{
Name: "SCEP",
Type: "SCEP",
Options: &Options{
Webhooks: []*Webhook{
{
ID: "webhook-id-1",
Name: "webhook-name-1",
Secret: "MTIzNAo=",
Kind: linkedca.Webhook_SCEPCHALLENGE.String(),
CertType: linkedca.Webhook_X509.String(),
URL: okServer.URL,
},
{
ID: "webhook-id-2",
Name: "webhook-name-2",
Secret: "MTIzNAo=",
Kind: linkedca.Webhook_SCEPCHALLENGE.String(),
CertType: linkedca.Webhook_X509.String(),
URL: okServer.URL,
},
},
},
}, okServer, args{"webhook-challenge", "webhook-transaction-1"}, x509util.TemplateData{
x509util.WebhooksKey: map[string]any{
"webhook-name-1": nil,
"webhook-name-2": map[string]any{
"ID": "2adcbfec-5e4a-4b93-8913-640e24faf101",
"Email": "admin@example.com",
},
},
}, nil},
{"fail/webhooks-secret-configuration", &SCEP{
Name: "SCEP",
Type: "SCEP",
Expand All @@ -317,60 +405,53 @@ func TestSCEP_ValidateChallenge(t *testing.T) {
},
},
},
}, nil, args{"webhook-challenge", "webhook-transaction-1"},
errors.New("failed executing webhook request: illegal base64 data at input byte 0"),
},
}, nil, args{"webhook-challenge", "webhook-transaction-1"}, nil, errors.New("failed executing webhook request: illegal base64 data at input byte 0")},
{"ok/static-challenge", &SCEP{
Name: "SCEP",
Type: "SCEP",
Options: &Options{},
ChallengePassword: "secret-static-challenge",
}, nil, args{"secret-static-challenge", "static-transaction-1"},
nil,
},
}, nil, args{"secret-static-challenge", "static-transaction-1"}, x509util.TemplateData{}, nil},
{"fail/wrong-static-challenge", &SCEP{
Name: "SCEP",
Type: "SCEP",
Options: &Options{},
ChallengePassword: "secret-static-challenge",
}, nil, args{"the-wrong-challenge-secret", "static-transaction-1"},
errors.New("invalid challenge password provided"),
},
}, nil, args{"the-wrong-challenge-secret", "static-transaction-1"}, nil, errors.New("invalid challenge password provided")},
{"ok/no-challenge", &SCEP{
Name: "SCEP",
Type: "SCEP",
Options: &Options{},
ChallengePassword: "",
}, nil, args{"", "static-transaction-1"},
nil,
},
}, nil, args{"", "static-transaction-1"}, x509util.TemplateData{}, nil},
{"fail/no-challenge-but-provided", &SCEP{
Name: "SCEP",
Type: "SCEP",
Options: &Options{},
ChallengePassword: "",
}, nil, args{"a-challenge-value", "static-transaction-1"},
errors.New("invalid challenge password provided"),
},
}, nil, args{"a-challenge-value", "static-transaction-1"}, nil, errors.New("invalid challenge password provided")},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {

if tt.server != nil {
defer tt.server.Close()
}

err := tt.p.Init(Config{Claims: globalProvisionerClaims, WebhookClient: http.DefaultClient})
require.NoError(t, err)
ctx := context.Background()

err = tt.p.ValidateChallenge(ctx, dummyCSR, tt.args.challenge, tt.args.transactionID)
got, err := tt.p.ValidateChallenge(ctx, dummyCSR, tt.args.challenge, tt.args.transactionID)
if tt.expErr != nil {
assert.EqualError(t, err, tt.expErr.Error())
return
}

assert.NoError(t, err)
data := x509util.TemplateData{}
for _, o := range got {
if m, ok := o.(TemplateDataModifier); ok {
m.Modify(data)
} else {
t.Errorf("Validate() got = %T, want TemplateDataModifier", o)
}
}
assert.Equal(t, tt.want, data)
})
}
}
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 options in the SignCSR
// method of the SCEP authority.
type SignCSROption any

// TemplateDataModifier is an interface that allows to modify template data.
type TemplateDataModifier interface {
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,
}
}
Loading