diff --git a/builtin/logical/pki/acme/errors.go b/builtin/logical/pki/acme_errors.go similarity index 99% rename from builtin/logical/pki/acme/errors.go rename to builtin/logical/pki/acme_errors.go index 69683837ee..f75ee88b5e 100644 --- a/builtin/logical/pki/acme/errors.go +++ b/builtin/logical/pki/acme_errors.go @@ -1,4 +1,4 @@ -package acme +package pki import ( "encoding/json" diff --git a/builtin/logical/pki/acme/jws.go b/builtin/logical/pki/acme_jws.go similarity index 82% rename from builtin/logical/pki/acme/jws.go rename to builtin/logical/pki/acme_jws.go index 0807b7b5d1..034c379c8b 100644 --- a/builtin/logical/pki/acme/jws.go +++ b/builtin/logical/pki/acme_jws.go @@ -1,4 +1,4 @@ -package acme +package pki import ( "crypto" @@ -23,7 +23,7 @@ var AllowedOuterJWSTypes = map[string]interface{}{ } // This wraps a JWS message structure. -type JWSCtx struct { +type jwsCtx struct { Algo string `json:"alg"` Kid string `json:"kid"` jwk json.RawMessage `json:"jwk"` @@ -33,7 +33,7 @@ type JWSCtx struct { Existing bool `json:"-"` } -func (c *JWSCtx) UnmarshalJSON(a *ACMEState, jws []byte) error { +func (c *jwsCtx) UnmarshalJSON(a *acmeState, jws []byte) error { var err error if err = json.Unmarshal(jws, c); err != nil { return err @@ -44,7 +44,7 @@ func (c *JWSCtx) UnmarshalJSON(a *ACMEState, jws []byte) error { // // > The "jwk" and "kid" fields are mutually exclusive. Servers MUST // > reject requests that contain both. - return fmt.Errorf("invalid header: got both account 'kid' and 'jwk' in the same message; expected only one") + return fmt.Errorf("invalid header: got both account 'kid' and 'jwk' in the same message; expected only one: %w", ErrMalformed) } if c.Kid == "" && len(c.jwk) == 0 { @@ -52,7 +52,7 @@ func (c *JWSCtx) UnmarshalJSON(a *ACMEState, jws []byte) error { // // > Either "jwk" (JSON Web Key) or "kid" (Key ID) as specified // > below - return fmt.Errorf("invalid header: got neither required fields of 'kid' nor 'jwk'") + return fmt.Errorf("invalid header: got neither required fields of 'kid' nor 'jwk': %w", ErrMalformed) } if _, present := AllowedOuterJWSTypes[c.Algo]; !present { @@ -65,7 +65,7 @@ func (c *JWSCtx) UnmarshalJSON(a *ACMEState, jws []byte) error { // > * This field MUST NOT contain "none" or a Message // > Authentication Code (MAC) algorithm (e.g. one in which the // > algorithm registry description mentions MAC/HMAC). - return fmt.Errorf("invalid header: unexpected value for 'algo'") + return fmt.Errorf("invalid header: unexpected value for 'algo': %w", ErrMalformed) } if c.Kid != "" { @@ -82,7 +82,7 @@ func (c *JWSCtx) UnmarshalJSON(a *ACMEState, jws []byte) error { } if !c.key.Valid() { - return fmt.Errorf("received invalid jwk") + return fmt.Errorf("received invalid jwk: %w", ErrMalformed) } if c.Kid != "" { @@ -103,7 +103,7 @@ func hasValues(h jose.Header) bool { return h.KeyID != "" || h.JSONWebKey != nil || h.Algorithm != "" || h.Nonce != "" || len(h.ExtraHeaders) > 0 } -func (c *JWSCtx) VerifyJWS(signature string) (map[string]interface{}, error) { +func (c *jwsCtx) VerifyJWS(signature string) (map[string]interface{}, error) { // See RFC 8555 Section 6.2. Request Authentication: // // > The JWS Unencoded Payload Option [RFC7797] MUST NOT be used @@ -111,21 +111,21 @@ func (c *JWSCtx) VerifyJWS(signature string) (map[string]interface{}, error) { // This is validated by go-jose. sig, err := jose.ParseSigned(signature) if err != nil { - return nil, fmt.Errorf("error parsing signature: %w", err) + return nil, fmt.Errorf("error parsing signature: %s: %w", err, ErrMalformed) } if len(sig.Signatures) > 1 { // See RFC 8555 Section 6.2. Request Authentication: // // > The JWS MUST NOT have multiple signatures - return nil, fmt.Errorf("request had multiple signatures") + return nil, fmt.Errorf("request had multiple signatures: %w", ErrMalformed) } if hasValues(sig.Signatures[0].Unprotected) { // See RFC 8555 Section 6.2. Request Authentication: // // > The JWS Unprotected Header [RFC7515] MUST NOT be used - return nil, fmt.Errorf("request had unprotected headers") + return nil, fmt.Errorf("request had unprotected headers: %w", ErrMalformed) } payload, err := sig.Verify(c.key) @@ -135,7 +135,7 @@ func (c *JWSCtx) VerifyJWS(signature string) (map[string]interface{}, error) { var m map[string]interface{} if err := json.Unmarshal(payload, &m); err != nil { - return nil, fmt.Errorf("failed to json unmarshal 'payload': %w", err) + return nil, fmt.Errorf("failed to json unmarshal 'payload': %s: %w", err, ErrMalformed) } return m, nil diff --git a/builtin/logical/pki/acme/state.go b/builtin/logical/pki/acme_state.go similarity index 68% rename from builtin/logical/pki/acme/state.go rename to builtin/logical/pki/acme_state.go index 2e341f1e83..36715e1689 100644 --- a/builtin/logical/pki/acme/state.go +++ b/builtin/logical/pki/acme_state.go @@ -1,4 +1,4 @@ -package acme +package pki import ( "crypto/rand" @@ -15,13 +15,13 @@ import ( // How long nonces are considered valid. const nonceExpiry = 15 * time.Minute -type ACMEState struct { +type acmeState struct { nextExpiry *atomic.Int64 nonces *sync.Map // map[string]time.Time } -func NewACMEState() *ACMEState { - return &ACMEState{ +func NewACMEState() *acmeState { + return &acmeState{ nextExpiry: new(atomic.Int64), nonces: new(sync.Map), } @@ -36,7 +36,7 @@ func generateNonce() (string, error) { return base64.RawURLEncoding.EncodeToString(data), nil } -func (a *ACMEState) GetNonce() (string, time.Time, error) { +func (a *acmeState) GetNonce() (string, time.Time, error) { now := time.Now() nonce, err := generateNonce() if err != nil { @@ -55,7 +55,7 @@ func (a *ACMEState) GetNonce() (string, time.Time, error) { return nonce, then, nil } -func (a *ACMEState) RedeemNonce(nonce string) bool { +func (a *acmeState) RedeemNonce(nonce string) bool { rawTimeout, present := a.nonces.LoadAndDelete(nonce) if !present { return false @@ -69,7 +69,7 @@ func (a *ACMEState) RedeemNonce(nonce string) bool { return true } -func (a *ACMEState) DoTidyNonces() { +func (a *acmeState) DoTidyNonces() { now := time.Now() expiry := a.nextExpiry.Load() then := time.Unix(expiry, 0) @@ -79,7 +79,7 @@ func (a *ACMEState) DoTidyNonces() { } } -func (a *ACMEState) TidyNonces() { +func (a *acmeState) TidyNonces() { now := time.Now() nextRun := now.Add(nonceExpiry) @@ -99,22 +99,22 @@ func (a *ACMEState) TidyNonces() { a.nextExpiry.Store(nextRun.Unix()) } -func (a *ACMEState) CreateAccount(c *JWSCtx, contact []string, termsOfServiceAgreed bool) (map[string]interface{}, error) { +func (a *acmeState) CreateAccount(c *jwsCtx, contact []string, termsOfServiceAgreed bool) (map[string]interface{}, error) { // TODO return nil, nil } -func (a *ACMEState) LoadAccount(keyID string) (map[string]interface{}, error) { +func (a *acmeState) LoadAccount(keyID string) (map[string]interface{}, error) { // TODO return nil, nil } -func (a *ACMEState) DoesAccountExist(keyId string) bool { +func (a *acmeState) DoesAccountExist(keyId string) bool { account, err := a.LoadAccount(keyId) return err == nil && len(account) > 0 } -func (a *ACMEState) LoadJWK(keyID string) ([]byte, error) { +func (a *acmeState) LoadJWK(keyID string) ([]byte, error) { key, err := a.LoadAccount(keyID) if err != nil { return nil, err @@ -128,15 +128,20 @@ func (a *ACMEState) LoadJWK(keyID string) ([]byte, error) { return jwk.([]byte), nil } -func (a *ACMEState) ParseRequestParams(data *framework.FieldData) (*JWSCtx, map[string]interface{}, error) { - var c JWSCtx +func (a *acmeState) ParseRequestParams(data *framework.FieldData) (*jwsCtx, map[string]interface{}, error) { + var c jwsCtx var m map[string]interface{} // Parse the key out. - jwkBase64 := data.Get("protected").(string) + rawJWKBase64, ok := data.GetOk("protected") + if !ok { + return nil, nil, fmt.Errorf("missing required field 'protected': %w", ErrMalformed) + } + jwkBase64 := rawJWKBase64.(string) + jwkBytes, err := base64.RawURLEncoding.DecodeString(jwkBase64) if err != nil { - return nil, nil, fmt.Errorf("failed to base64 parse 'protected': %w", err) + return nil, nil, fmt.Errorf("failed to base64 parse 'protected': %s: %w", err, ErrMalformed) } if err = c.UnmarshalJSON(a, jwkBytes); err != nil { return nil, nil, fmt.Errorf("failed to json unmarshal 'protected': %w", err) @@ -146,11 +151,20 @@ func (a *ACMEState) ParseRequestParams(data *framework.FieldData) (*JWSCtx, map[ // should read and redeem the nonce here too, to avoid doing any extra // work if it is invalid. if !a.RedeemNonce(c.Nonce) { - return nil, nil, fmt.Errorf("invalid or reused nonce") + return nil, nil, fmt.Errorf("invalid or reused nonce: %w", ErrBadNonce) } - payloadBase64 := data.Get("payload").(string) - signatureBase64 := data.Get("signature").(string) + rawPayloadBase64, ok := data.GetOk("payload") + if !ok { + return nil, nil, fmt.Errorf("missing required field 'payload': %w", ErrMalformed) + } + payloadBase64 := rawPayloadBase64.(string) + + rawSignatureBase64, ok := data.GetOk("signature") + if !ok { + return nil, nil, fmt.Errorf("missing required field 'signature': %w", ErrMalformed) + } + signatureBase64 := rawSignatureBase64.(string) // go-jose only seems to support compact signature encodings. compactSig := fmt.Sprintf("%v.%v.%v", jwkBase64, payloadBase64, signatureBase64) diff --git a/builtin/logical/pki/acme/state_test.go b/builtin/logical/pki/acme_state_test.go similarity index 98% rename from builtin/logical/pki/acme/state_test.go rename to builtin/logical/pki/acme_state_test.go index 55bde1fd78..edce657f59 100644 --- a/builtin/logical/pki/acme/state_test.go +++ b/builtin/logical/pki/acme_state_test.go @@ -1,4 +1,4 @@ -package acme +package pki import ( "testing" diff --git a/builtin/logical/pki/backend.go b/builtin/logical/pki/backend.go index de937ba733..7d2a8ea1e8 100644 --- a/builtin/logical/pki/backend.go +++ b/builtin/logical/pki/backend.go @@ -14,8 +14,6 @@ import ( atomic2 "go.uber.org/atomic" - "github.com/hashicorp/vault/builtin/logical/pki/acme" - "github.com/armon/go-metrics" "github.com/hashicorp/go-multierror" "github.com/hashicorp/vault/helper/constants" @@ -290,7 +288,7 @@ func Backend(conf *logical.BackendConfig) *backend { b.unifiedTransferStatus = newUnifiedTransferStatus() - b.acmeState = acme.NewACMEState() + b.acmeState = NewACMEState() return &b } @@ -325,7 +323,7 @@ type backend struct { issuersLock sync.RWMutex // Context around ACME operations - acmeState *acme.ACMEState + acmeState *acmeState } type roleOperation func(ctx context.Context, req *logical.Request, data *framework.FieldData, role *roleEntry) (*logical.Response, error) diff --git a/builtin/logical/pki/path_acme_directory.go b/builtin/logical/pki/path_acme_directory.go index 4770ff7aba..bd9f2e2106 100644 --- a/builtin/logical/pki/path_acme_directory.go +++ b/builtin/logical/pki/path_acme_directory.go @@ -8,7 +8,6 @@ import ( "net/url" "strings" - "github.com/hashicorp/vault/builtin/logical/pki/acme" "github.com/hashicorp/vault/sdk/framework" "github.com/hashicorp/vault/sdk/logical" ) @@ -69,7 +68,7 @@ func (b *backend) acmeWrapper(op acmeOperation) framework.OperationFunc { if false { // TODO sclark: Check if ACME is enable here - return nil, fmt.Errorf("ACME is disabled in configuration: %w", acme.ErrServerInternal) + return nil, fmt.Errorf("ACME is disabled in configuration: %w", ErrServerInternal) } baseUrl, err := getAcmeBaseUrl(sc, r.Path) @@ -93,12 +92,12 @@ func getAcmeBaseUrl(sc *storageContext, path string) (*url.URL, error) { } if cfg.Path == "" { - return nil, fmt.Errorf("ACME feature requires local cluster path configuration to be set: %w", acme.ErrServerInternal) + return nil, fmt.Errorf("ACME feature requires local cluster path configuration to be set: %w", ErrServerInternal) } baseUrl, err := url.Parse(cfg.Path) if err != nil { - return nil, fmt.Errorf("ACME feature a proper URL configured in local cluster path: %w", acme.ErrServerInternal) + return nil, fmt.Errorf("ACME feature a proper URL configured in local cluster path: %w", ErrServerInternal) } directoryPrefix := "" @@ -114,7 +113,7 @@ func acmeErrorWrapper(op framework.OperationFunc) framework.OperationFunc { return func(ctx context.Context, r *logical.Request, data *framework.FieldData) (*logical.Response, error) { resp, err := op(ctx, r, data) if err != nil { - return acme.TranslateError(err) + return TranslateError(err) } return resp, nil diff --git a/builtin/logical/pki/path_acme_new_account.go b/builtin/logical/pki/path_acme_new_account.go index 7095e67a7f..46b49ab397 100644 --- a/builtin/logical/pki/path_acme_new_account.go +++ b/builtin/logical/pki/path_acme_new_account.go @@ -4,7 +4,6 @@ import ( "fmt" "strings" - "github.com/hashicorp/vault/builtin/logical/pki/acme" "github.com/hashicorp/vault/sdk/framework" "github.com/hashicorp/vault/sdk/logical" ) @@ -50,19 +49,19 @@ func addFieldsForACMERequest(fields map[string]*framework.FieldSchema) map[strin fields["protected"] = &framework.FieldSchema{ Type: framework.TypeString, Description: "ACME request 'protected' value", - Required: true, + Required: false, } fields["payload"] = &framework.FieldSchema{ Type: framework.TypeString, Description: "ACME request 'payload' value", - Required: true, + Required: false, } fields["signature"] = &framework.FieldSchema{ Type: framework.TypeString, Description: "ACME request 'signature' value", - Required: true, + Required: false, } return fields @@ -89,7 +88,7 @@ func patternAcmeNewAccount(b *backend, pattern string) *framework.Path { } } -type acmeParsedOperation func(acmeCtx acmeContext, r *logical.Request, fields *framework.FieldData, userCtx *acme.JWSCtx, data map[string]interface{}) (*logical.Response, error) +type acmeParsedOperation func(acmeCtx acmeContext, r *logical.Request, fields *framework.FieldData, userCtx *jwsCtx, data map[string]interface{}) (*logical.Response, error) func (b *backend) acmeParsedWrapper(op acmeParsedOperation) framework.OperationFunc { return b.acmeWrapper(func(acmeCtx acmeContext, r *logical.Request, fields *framework.FieldData) (*logical.Response, error) { @@ -102,7 +101,7 @@ func (b *backend) acmeParsedWrapper(op acmeParsedOperation) framework.OperationF }) } -func (b *backend) acmeNewAccountHandler(acmeCtx acmeContext, r *logical.Request, fields *framework.FieldData, userCtx *acme.JWSCtx, data map[string]interface{}) (*logical.Response, error) { +func (b *backend) acmeNewAccountHandler(acmeCtx acmeContext, r *logical.Request, fields *framework.FieldData, userCtx *jwsCtx, data map[string]interface{}) (*logical.Response, error) { // Parameters var ok bool var onlyReturnExisting bool @@ -113,7 +112,7 @@ func (b *backend) acmeNewAccountHandler(acmeCtx acmeContext, r *logical.Request, if present { contact, ok = rawContact.([]string) if !ok { - return nil, fmt.Errorf("invalid type for field 'contact': %w", acme.ErrMalformed) + return nil, fmt.Errorf("invalid type for field 'contact': %w", ErrMalformed) } } @@ -121,7 +120,7 @@ func (b *backend) acmeNewAccountHandler(acmeCtx acmeContext, r *logical.Request, if present { termsOfServiceAgreed, ok = rawTermsOfServiceAgreed.(bool) if !ok { - return nil, fmt.Errorf("invalid type for field 'termsOfServiceAgreed': %w", acme.ErrMalformed) + return nil, fmt.Errorf("invalid type for field 'termsOfServiceAgreed': %w", ErrMalformed) } } @@ -129,7 +128,7 @@ func (b *backend) acmeNewAccountHandler(acmeCtx acmeContext, r *logical.Request, if present { onlyReturnExisting, ok = rawOnlyReturnExisting.(bool) if !ok { - return nil, fmt.Errorf("invalid type for field 'onlyReturnExisting': %w", acme.ErrMalformed) + return nil, fmt.Errorf("invalid type for field 'onlyReturnExisting': %w", ErrMalformed) } } @@ -160,7 +159,7 @@ func formatAccountResponse(location string, status string, contact []string) *lo return resp } -func (b *backend) acmeNewAccountSearchHandler(acmeCtx acmeContext, r *logical.Request, fields *framework.FieldData, userCtx *acme.JWSCtx, data map[string]interface{}) (*logical.Response, error) { +func (b *backend) acmeNewAccountSearchHandler(acmeCtx acmeContext, r *logical.Request, fields *framework.FieldData, userCtx *jwsCtx, data map[string]interface{}) (*logical.Response, error) { if userCtx.Existing || b.acmeState.DoesAccountExist(userCtx.Kid) { // This account exists; return its details. It would be slightly // weird to specify a kid in the request (and not use an explicit @@ -179,12 +178,12 @@ func (b *backend) acmeNewAccountSearchHandler(acmeCtx acmeContext, r *logical.Re // > If a client sends such a request and an account does not exist, // > then the server MUST return an error response with status code // > 400 (Bad Request) and type "urn:ietf:params:acme:error:accountDoesNotExist". - return nil, fmt.Errorf("An account with this key does not exist: %w", acme.ErrAccountDoesNotExist) + return nil, fmt.Errorf("An account with this key does not exist: %w", ErrAccountDoesNotExist) } -func (b *backend) acmeNewAccountCreateHandler(acmeCtx acmeContext, r *logical.Request, fields *framework.FieldData, userCtx *acme.JWSCtx, data map[string]interface{}, contact []string, termsOfServiceAgreed bool) (*logical.Response, error) { +func (b *backend) acmeNewAccountCreateHandler(acmeCtx acmeContext, r *logical.Request, fields *framework.FieldData, userCtx *jwsCtx, data map[string]interface{}, contact []string, termsOfServiceAgreed bool) (*logical.Response, error) { if userCtx.Existing { - return nil, fmt.Errorf("cannot submit to newAccount with 'kid': %w", acme.ErrMalformed) + return nil, fmt.Errorf("cannot submit to newAccount with 'kid': %w", ErrMalformed) } // If the account already exists, return the existing one. @@ -194,7 +193,7 @@ func (b *backend) acmeNewAccountCreateHandler(acmeCtx acmeContext, r *logical.Re // TODO: Limit this only when ToS are required by the operator. if !termsOfServiceAgreed { - return nil, fmt.Errorf("terms of service not agreed to: %w", acme.ErrUserActionRequired) + return nil, fmt.Errorf("terms of service not agreed to: %w", ErrUserActionRequired) } account, err := b.acmeState.CreateAccount(userCtx, contact, termsOfServiceAgreed)