diff --git a/acme/api/order.go b/acme/api/order.go index 8d8e4ddc..bb7786b0 100644 --- a/acme/api/order.go +++ b/acme/api/order.go @@ -282,18 +282,21 @@ func newAuthorization(ctx context.Context, az *acme.Authorization) error { if err != nil { return acme.WrapError(acme.ErrorMalformedType, err, "failed parsing ClientID") } - - var targetProvider interface{ GetTarget(string) (string, error) } + wireOptions, err := prov.GetOptions().GetWireOptions() + if err != nil { + return acme.WrapErrorISE(err, "failed getting Wire options") + } + var targetProvider interface{ EvaluateTarget(string) (string, error) } switch typ { case acme.WIREOIDC01: - targetProvider = prov.GetOptions().GetWireOptions().GetOIDCOptions() + targetProvider = wireOptions.GetOIDCOptions() case acme.WIREDPOP01: - targetProvider = prov.GetOptions().GetWireOptions().GetDPOPOptions() + targetProvider = wireOptions.GetDPOPOptions() default: return acme.NewError(acme.ErrorMalformedType, "unsupported type %q", typ) } - target, err = targetProvider.GetTarget(clientID.DeviceID) + target, err = targetProvider.EvaluateTarget(clientID.DeviceID) if err != nil { return acme.WrapError(acme.ErrorMalformedType, err, "invalid Go template registered for 'target'") } diff --git a/acme/api/order_test.go b/acme/api/order_test.go index 45bff828..1a3c5df4 100644 --- a/acme/api/order_test.go +++ b/acme/api/order_test.go @@ -885,6 +885,10 @@ func TestHandler_NewOrder(t *testing.T) { u := fmt.Sprintf("%s/acme/%s/order/ordID", baseURL.String(), escProvName) + fakeWireSigningKey := `-----BEGIN PUBLIC KEY----- +MCowBQYDK2VwAyEA5c+4NKZSNQcR1T8qN6SjwgdPZQ0Ge12Ylx/YeGAJ35k= +-----END PUBLIC KEY-----` + type test struct { ca acme.CertificateAuthority db acme.DB @@ -1719,17 +1723,17 @@ func TestHandler_NewOrder(t *testing.T) { acmeWireProv := newWireProvisionerWithOptions(t, &provisioner.Options{ Wire: &wire.Options{ OIDC: &wire.OIDCOptions{ - Provider: wire.ProviderJSON{ - IssuerURL: "", + Provider: &wire.Provider{ + IssuerURL: "https://issuer.example.com", AuthURL: "", TokenURL: "", JWKSURL: "", UserInfoURL: "", Algorithms: []string{}, }, - Config: wire.ConfigJSON{ + Config: &wire.Config{ ClientID: "integration test", - SupportedSigningAlgs: []string{}, + SignatureAlgorithms: []string{}, SkipClientIDCheck: true, SkipExpiryCheck: true, SkipIssuerCheck: true, @@ -1737,7 +1741,9 @@ func TestHandler_NewOrder(t *testing.T) { Now: time.Now, }, }, - DPOP: &wire.DPOPOptions{}, + DPOP: &wire.DPOPOptions{ + SigningKey: []byte(fakeWireSigningKey), + }, }, }) acc := &acme.Account{ID: "accID"} diff --git a/acme/api/wire_integration_test.go b/acme/api/wire_integration_test.go index 666e1137..8e2497cd 100644 --- a/acme/api/wire_integration_test.go +++ b/acme/api/wire_integration_test.go @@ -51,20 +51,23 @@ func newWireProvisionerWithOptions(t *testing.T, options *provisioner.Options) * } func TestWireIntegration(t *testing.T) { + fakeKey := `-----BEGIN PUBLIC KEY----- +MCowBQYDK2VwAyEA5c+4NKZSNQcR1T8qN6SjwgdPZQ0Ge12Ylx/YeGAJ35k= +-----END PUBLIC KEY-----` prov := newWireProvisionerWithOptions(t, &provisioner.Options{ Wire: &wire.Options{ OIDC: &wire.OIDCOptions{ - Provider: wire.ProviderJSON{ - IssuerURL: "", + Provider: &wire.Provider{ + IssuerURL: "https://issuer.example.com", AuthURL: "", TokenURL: "", JWKSURL: "", UserInfoURL: "", Algorithms: []string{}, }, - Config: wire.ConfigJSON{ + Config: &wire.Config{ ClientID: "integration test", - SupportedSigningAlgs: []string{}, + SignatureAlgorithms: []string{}, SkipClientIDCheck: true, SkipExpiryCheck: true, SkipIssuerCheck: true, @@ -72,7 +75,9 @@ func TestWireIntegration(t *testing.T) { Now: time.Now, }, }, - DPOP: &wire.DPOPOptions{}, + DPOP: &wire.DPOPOptions{ + SigningKey: []byte(fakeKey), + }, }, }) diff --git a/acme/challenge.go b/acme/challenge.go index d75a21fd..5d190519 100644 --- a/acme/challenge.go +++ b/acme/challenge.go @@ -370,7 +370,12 @@ func wireOIDC01Validate(ctx context.Context, ch *Challenge, db DB, jwk *jose.JSO "error unmarshalling Wire challenge payload")) } - oidcOptions := prov.GetOptions().GetWireOptions().GetOIDCOptions() + wireOptions, err := prov.GetOptions().GetWireOptions() + if err != nil { + return WrapErrorISE(err, "failed getting Wire options") + } + + oidcOptions := wireOptions.GetOIDCOptions() idToken, err := oidcOptions.GetProvider(ctx).Verifier(oidcOptions.GetConfig()).Verify(ctx, oidcPayload.IDToken) if err != nil { return storeError(ctx, db, ch, false, WrapError(ErrorRejectedIdentifierType, err, @@ -468,20 +473,25 @@ func wireDPOP01Validate(ctx context.Context, ch *Challenge, db DB, accountJWK *j return WrapErrorISE(err, "error parsing device id") } - dpopOptions := prov.GetOptions().GetWireOptions().GetDPOPOptions() - issuer, err := dpopOptions.GetTarget(clientID.DeviceID) + wireOptions, err := prov.GetOptions().GetWireOptions() + if err != nil { + return WrapErrorISE(err, "failed getting Wire options") + } + + dpopOptions := wireOptions.GetDPOPOptions() + issuer, err := dpopOptions.EvaluateTarget(clientID.DeviceID) if err != nil { return WrapErrorISE(err, "invalid Go template registered for 'target'") } params := verifyParams{ - token: dpopPayload.AccessToken, - key: dpopOptions.GetSigningKey(), - accountJWK: accountJWK, - issuer: issuer, - wireID: wireID, - challenge: ch, - t: clock.Now().UTC(), + token: dpopPayload.AccessToken, + tokenKey: dpopOptions.GetSigningKey(), + dpopKey: accountJWK, + issuer: issuer, + wireID: wireID, + challenge: ch, + t: clock.Now().UTC(), } _, dpop, err := parseAndVerifyWireAccessToken(params) if err != nil { @@ -530,33 +540,23 @@ type wireAccessToken struct { type wireDpopToken map[string]any type verifyParams struct { - token string - key string - issuer string - accountJWK *jose.JSONWebKey - wireID wire.ID - challenge *Challenge - t time.Time + token string + tokenKey crypto.PublicKey + dpopKey *jose.JSONWebKey + issuer string + wireID wire.ID + challenge *Challenge + t time.Time } func parseAndVerifyWireAccessToken(v verifyParams) (*wireAccessToken, *wireDpopToken, error) { - k, err := pemutil.Parse([]byte(v.key)) // TODO(hs): move this to earlier in the configuration process? Do it once? - if err != nil { - return nil, nil, fmt.Errorf("failed parsing public key: %w", err) - } - - pk, ok := k.(ed25519.PublicKey) // TODO(hs): allow more key types - if !ok { - return nil, nil, fmt.Errorf("unexpected type: %T", k) - } - jwt, err := jose.ParseSigned(v.token) if err != nil { return nil, nil, fmt.Errorf("failed parsing token: %w", err) } var accessToken wireAccessToken - if err = jwt.Claims(pk, &accessToken); err != nil { + if err = jwt.Claims(v.tokenKey, &accessToken); err != nil { return nil, nil, fmt.Errorf("failed validating Wire DPoP token claims: %w", err) } @@ -567,7 +567,7 @@ func parseAndVerifyWireAccessToken(v verifyParams) (*wireAccessToken, *wireDpopT return nil, nil, fmt.Errorf("failed validation: %w", err) } - rawKid, err := v.accountJWK.Thumbprint(crypto.SHA256) + rawKid, err := v.dpopKey.Thumbprint(crypto.SHA256) if err != nil { return nil, nil, fmt.Errorf("failed to compute JWK thumbprint") } @@ -590,9 +590,8 @@ func parseAndVerifyWireAccessToken(v verifyParams) (*wireAccessToken, *wireDpopT if err != nil { return nil, nil, fmt.Errorf("invalid Wire DPoP token: %w", err) } - var dpopToken wireDpopToken - if err := dpopJWT.Claims(v.accountJWK.Key, &dpopToken); err != nil { + if err := dpopJWT.Claims(v.dpopKey.Key, &dpopToken); err != nil { return nil, nil, fmt.Errorf("failed validating Wire DPoP token claims: %w", err) } diff --git a/acme/challenge_test.go b/acme/challenge_test.go index 3c611d1b..d917b1e2 100644 --- a/acme/challenge_test.go +++ b/acme/challenge_test.go @@ -34,6 +34,7 @@ import ( "go.step.sm/crypto/jose" "go.step.sm/crypto/keyutil" "go.step.sm/crypto/minica" + "go.step.sm/crypto/pemutil" "go.step.sm/crypto/x509util" "github.com/smallstep/certificates/acme/wire" @@ -4308,7 +4309,9 @@ func Test_parseAndVerifyWireAccessToken(t *testing.T) { key := ` -----BEGIN PUBLIC KEY----- MCowBQYDK2VwAyEAB2IYqBWXAouDt3WcCZgCM3t9gumMEKMlgMsGenSu+fA= ------END PUBLIC KEY-----` // TODO(hs): different format? +-----END PUBLIC KEY-----` + publicKey, err := pemutil.Parse([]byte(key)) + require.NoError(t, err) issuer := "http://wire.com:19983/clients/7a41cf5b79683410/access-token" wireID := wire.ID{ ClientID: "wireapp://guVX5xeFS3eTatmXBIyA4A!7a41cf5b79683410@wire.com", @@ -4330,13 +4333,13 @@ MCowBQYDK2VwAyEAB2IYqBWXAouDt3WcCZgCM3t9gumMEKMlgMsGenSu+fA= json.Unmarshal(jwkBytes, &accountJWK) at, dpop, err := parseAndVerifyWireAccessToken(verifyParams{ - token: token, - key: key, - accountJWK: &accountJWK, - issuer: issuer, - wireID: wireID, - challenge: ch, - t: issuedAt.Add(1 * time.Minute), // set validation time to be one minute after issuance + token: token, + tokenKey: publicKey, + dpopKey: &accountJWK, + issuer: issuer, + wireID: wireID, + challenge: ch, + t: issuedAt.Add(1 * time.Minute), // set validation time to be one minute after issuance }) if assert.NoError(t, err) { // token assertions diff --git a/authority/provisioner/options.go b/authority/provisioner/options.go index 1e0457c5..fcb2ca0d 100644 --- a/authority/provisioner/options.go +++ b/authority/provisioner/options.go @@ -2,6 +2,7 @@ package provisioner import ( "encoding/json" + "fmt" "strings" "github.com/pkg/errors" @@ -54,11 +55,14 @@ func (o *Options) GetSSHOptions() *SSHOptions { } // GetWireOptions returns the SSH options. -func (o *Options) GetWireOptions() *wire.Options { +func (o *Options) GetWireOptions() (*wire.Options, error) { if o == nil { - return nil + return nil, errors.New("no Wire options available") } - return o.Wire + if err := o.Wire.Validate(); err != nil { + return nil, fmt.Errorf("failed validating Wire options: %w", err) + } + return o.Wire, nil } // GetWebhooks returns the webhooks options. diff --git a/authority/provisioner/wire/dpop_options.go b/authority/provisioner/wire/dpop_options.go index ebf5fc21..c4172a65 100644 --- a/authority/provisioner/wire/dpop_options.go +++ b/authority/provisioner/wire/dpop_options.go @@ -2,44 +2,44 @@ package wire import ( "bytes" - "errors" + "crypto" "fmt" "text/template" + + "go.step.sm/crypto/pemutil" ) type DPOPOptions struct { - // Backend signing key for DPoP access token - SigningKey string `json:"key"` - // URI template acme client must call to fetch the DPoP challenge proof (an access token from wire-server) - DpopTarget string `json:"dpop-target"` + // Public part of the signing key for DPoP access token + SigningKey []byte `json:"key"` + // URI template for the URI the ACME client must call to fetch the DPoP challenge proof (an access token from wire-server) + Target string `json:"target"` + + signingKey crypto.PublicKey + target *template.Template } -func (o *DPOPOptions) GetSigningKey() string { - if o == nil { - return "" - } - return o.SigningKey +func (o *DPOPOptions) GetSigningKey() crypto.PublicKey { + return o.signingKey } -func (o *DPOPOptions) GetDPOPTarget() string { - if o == nil { - return "" - } - return o.DpopTarget -} - -func (o *DPOPOptions) GetTarget(deviceID string) (string, error) { - if o == nil { - return "", errors.New("misconfigured target template configuration") - } - targetTemplate := o.GetDPOPTarget() - tmpl, err := template.New("DeviceId").Parse(targetTemplate) - if err != nil { - return "", fmt.Errorf("failed parsing dpop template: %w", err) - } +func (o *DPOPOptions) EvaluateTarget(deviceID string) (string, error) { buf := new(bytes.Buffer) - if err = tmpl.Execute(buf, struct{ DeviceId string }{DeviceId: deviceID}); err != nil { //nolint:revive,stylecheck // TODO(hs): this requires changes in configuration + if err := o.target.Execute(buf, struct{ DeviceID string }{DeviceID: deviceID}); err != nil { return "", fmt.Errorf("failed executing dpop template: %w", err) } return buf.String(), nil } + +func (o *DPOPOptions) validateAndInitialize() (err error) { + o.signingKey, err = pemutil.Parse(o.SigningKey) + if err != nil { + return fmt.Errorf("failed parsing key: %w", err) + } + o.target, err = template.New("DeviceID").Parse(o.Target) + if err != nil { + return fmt.Errorf("failed parsing DPoP template: %w", err) + } + + return nil +} diff --git a/authority/provisioner/wire/oidc_options.go b/authority/provisioner/wire/oidc_options.go index 66497a16..e009c6f9 100644 --- a/authority/provisioner/wire/oidc_options.go +++ b/authority/provisioner/wire/oidc_options.go @@ -12,7 +12,7 @@ import ( "github.com/coreos/go-oidc/v3/oidc" ) -type ProviderJSON struct { +type Provider struct { IssuerURL string `json:"issuer,omitempty"` AuthURL string `json:"authorization_endpoint,omitempty"` TokenURL string `json:"token_endpoint,omitempty"` @@ -21,56 +21,80 @@ type ProviderJSON struct { Algorithms []string `json:"id_token_signing_alg_values_supported,omitempty"` } -type ConfigJSON struct { - ClientID string `json:"client-id,omitempty"` - SupportedSigningAlgs []string `json:"support-signing-algs,omitempty"` +type Config struct { + ClientID string `json:"clientId,omitempty"` + SignatureAlgorithms []string `json:"signatureAlgorithms,omitempty"` SkipClientIDCheck bool `json:"-"` SkipExpiryCheck bool `json:"-"` SkipIssuerCheck bool `json:"-"` - Now func() time.Time `json:"-"` InsecureSkipSignatureCheck bool `json:"-"` + Now func() time.Time `json:"-"` } type OIDCOptions struct { - Provider ProviderJSON `json:"provider,omitempty"` - Config ConfigJSON `json:"config,omitempty"` + Provider *Provider `json:"provider,omitempty"` + Config *Config `json:"config,omitempty"` + + oidcProviderConfig *oidc.ProviderConfig + target *template.Template } func (o *OIDCOptions) GetProvider(ctx context.Context) *oidc.Provider { - if o == nil { + if o == nil || o.Provider == nil || o.oidcProviderConfig == nil { return nil } - return toProviderConfig(o.Provider).NewProvider(ctx) + return o.oidcProviderConfig.NewProvider(ctx) } func (o *OIDCOptions) GetConfig() *oidc.Config { - if o == nil { + if o == nil || o.Config == nil { return &oidc.Config{} } - config := oidc.Config(o.Config) - return &config + + return &oidc.Config{ + ClientID: o.Config.ClientID, + SupportedSigningAlgs: o.Config.SignatureAlgorithms, + SkipClientIDCheck: o.Config.SkipClientIDCheck, + SkipExpiryCheck: o.Config.SkipExpiryCheck, + SkipIssuerCheck: o.Config.SkipIssuerCheck, + Now: o.Config.Now, + InsecureSkipSignatureCheck: o.Config.InsecureSkipSignatureCheck, + } } -func (o *OIDCOptions) GetTarget(deviceID string) (string, error) { - if o == nil { - return "", errors.New("misconfigured target template configuration") +func (o *OIDCOptions) validateAndInitialize() (err error) { + if o.Provider == nil { + return errors.New("provider not set") } - targetTemplate := o.Provider.IssuerURL - tmpl, err := template.New("DeviceId").Parse(targetTemplate) + if o.Provider.IssuerURL == "" { + return errors.New("issuer URL must not be empty") + } + + o.oidcProviderConfig, err = toOIDCProviderConfig(o.Provider) if err != nil { - return "", fmt.Errorf("failed parsing oidc template: %w", err) + return fmt.Errorf("failed creationg OIDC provider config: %w", err) } + + o.target, err = template.New("DeviceID").Parse(o.Provider.IssuerURL) + if err != nil { + return fmt.Errorf("failed parsing OIDC template: %w", err) + } + + return nil +} + +func (o *OIDCOptions) EvaluateTarget(deviceID string) (string, error) { buf := new(bytes.Buffer) - if err = tmpl.Execute(buf, struct{ DeviceId string }{DeviceId: deviceID}); err != nil { //nolint:revive,stylecheck // TODO(hs): this requires changes in configuration - return "", fmt.Errorf("failed executing oidc template: %w", err) + if err := o.target.Execute(buf, struct{ DeviceID string }{DeviceID: deviceID}); err != nil { + return "", fmt.Errorf("failed executing OIDC template: %w", err) } return buf.String(), nil } -func toProviderConfig(in ProviderJSON) *oidc.ProviderConfig { +func toOIDCProviderConfig(in *Provider) (*oidc.ProviderConfig, error) { issuerURL, err := url.Parse(in.IssuerURL) if err != nil { - panic(err) // config error, it's ok to panic here + return nil, fmt.Errorf("failed parsing issuer URL: %w", err) } // Removes query params from the URL because we use it as a way to notify client about the actual OAuth ClientId // for this provisioner. @@ -86,5 +110,5 @@ func toProviderConfig(in ProviderJSON) *oidc.ProviderConfig { UserInfoURL: in.UserInfoURL, JWKSURL: in.JWKSURL, Algorithms: in.Algorithms, - } + }, nil } diff --git a/authority/provisioner/wire/wire_options.go b/authority/provisioner/wire/wire_options.go index 9ab300fb..f143c287 100644 --- a/authority/provisioner/wire/wire_options.go +++ b/authority/provisioner/wire/wire_options.go @@ -1,9 +1,18 @@ package wire +import ( + "errors" + "fmt" + "sync" +) + // Options holds the Wire ACME extension options type Options struct { OIDC *OIDCOptions `json:"oidc,omitempty"` DPOP *DPOPOptions `json:"dpop,omitempty"` + + validateOnce sync.Once + validationErr error } // GetOIDCOptions returns the OIDC options. @@ -21,3 +30,33 @@ func (o *Options) GetDPOPOptions() *DPOPOptions { } return o.DPOP } + +func (o *Options) Validate() error { + o.validateOnce.Do( + func() { + o.validationErr = validate(o) + }, + ) + + return o.validationErr +} + +func validate(o *Options) error { + if oidc := o.GetOIDCOptions(); oidc != nil { + if err := oidc.validateAndInitialize(); err != nil { + return fmt.Errorf("failed initializing OIDC options: %w", err) + } + } else { + return errors.New("no OIDC options available") + } + + if dpop := o.GetDPOPOptions(); dpop != nil { + if err := dpop.validateAndInitialize(); err != nil { + return fmt.Errorf("failed initializing DPoP options: %w", err) + } + } else { + return errors.New("no DPoP options available") + } + + return nil +} diff --git a/authority/provisioner/wire/wire_options_test.go b/authority/provisioner/wire/wire_options_test.go new file mode 100644 index 00000000..068790d3 --- /dev/null +++ b/authority/provisioner/wire/wire_options_test.go @@ -0,0 +1,147 @@ +package wire + +import ( + "errors" + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestOptions_Validate(t *testing.T) { + key := []byte(`-----BEGIN PUBLIC KEY----- +MCowBQYDK2VwAyEA5c+4NKZSNQcR1T8qN6SjwgdPZQ0Ge12Ylx/YeGAJ35k= +-----END PUBLIC KEY-----`) + + type fields struct { + OIDC *OIDCOptions + DPOP *DPOPOptions + } + tests := []struct { + name string + fields fields + expectedErr error + }{ + { + name: "ok", + fields: fields{ + OIDC: &OIDCOptions{ + Provider: &Provider{ + IssuerURL: "https://example.com", + }, + Config: &Config{}, + }, + DPOP: &DPOPOptions{ + SigningKey: key, + }, + }, + expectedErr: nil, + }, + { + name: "fail/no-oidc-options", + fields: fields{ + OIDC: nil, + DPOP: &DPOPOptions{}, + }, + expectedErr: errors.New("no OIDC options available"), + }, + { + name: "fail/empty-issuer-url", + fields: fields{ + OIDC: &OIDCOptions{ + Provider: &Provider{ + IssuerURL: "", + }, + Config: &Config{}, + }, + DPOP: &DPOPOptions{}, + }, + expectedErr: errors.New("failed initializing OIDC options: issuer URL must not be empty"), + }, + { + name: "fail/invalid-issuer-url", + fields: fields{ + OIDC: &OIDCOptions{ + Provider: &Provider{ + IssuerURL: "\x00", + }, + Config: &Config{}, + }, + DPOP: &DPOPOptions{}, + }, + expectedErr: errors.New(`failed initializing OIDC options: failed creationg OIDC provider config: failed parsing issuer URL: parse "\x00": net/url: invalid control character in URL`), + }, + { + name: "fail/issuer-url-template", + fields: fields{ + OIDC: &OIDCOptions{ + Provider: &Provider{ + IssuerURL: "https://issuer.example.com/{{}", + }, + Config: &Config{}, + }, + DPOP: &DPOPOptions{}, + }, + expectedErr: errors.New(`failed initializing OIDC options: failed parsing OIDC template: template: DeviceID:1: unexpected "}" in command`), + }, + { + name: "fail/no-dpop-options", + fields: fields{ + OIDC: &OIDCOptions{ + Provider: &Provider{ + IssuerURL: "https://example.com", + }, + Config: &Config{}, + }, + DPOP: nil, + }, + expectedErr: errors.New("no DPoP options available"), + }, + { + name: "fail/invalid-key", + fields: fields{ + OIDC: &OIDCOptions{ + Provider: &Provider{ + IssuerURL: "https://example.com", + }, + Config: &Config{}, + }, + DPOP: &DPOPOptions{ + SigningKey: []byte{0x00}, + Target: "", + }, + }, + expectedErr: errors.New(`failed initializing DPoP options: failed parsing key: error decoding PEM: not a valid PEM encoded block`), + }, + { + name: "fail/target-template", + fields: fields{ + OIDC: &OIDCOptions{ + Provider: &Provider{ + IssuerURL: "https://example.com", + }, + Config: &Config{}, + }, + DPOP: &DPOPOptions{ + SigningKey: key, + Target: "{{}", + }, + }, + expectedErr: errors.New(`failed initializing DPoP options: failed parsing DPoP template: template: DeviceID:1: unexpected "}" in command`), + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + o := &Options{ + OIDC: tt.fields.OIDC, + DPOP: tt.fields.DPOP, + } + err := o.Validate() + if tt.expectedErr != nil { + assert.EqualError(t, err, tt.expectedErr.Error()) + return + } + + assert.NoError(t, err) + }) + } +}