Skip to content

Commit

Permalink
Merge pull request #2065 from smallstep/mariano/challenge-webhook
Browse files Browse the repository at this point in the history 
Add data support on SCEPCHALLENGE webhooks
  • Loading branch information
@maraino
maraino authored Nov 12, 2024
2 parents 295892e + ff37bf1 commit 2f7690c
Show file tree
Hide file tree
Showing 7 changed files with 353 additions and 52 deletions.
27 changes: 18 additions & 9 deletions authority/provisioner/scep.go
View file
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
View file
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": "[email protected]",
}
}
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": "[email protected]",
},
},
},
},
}
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": "[email protected]",
}
}
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": "[email protected]",
},
},
}, 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
View file
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

0 comments on commit 2f7690c

Please sign in to comment.