luxolus
/
open-vault
2
0
Fork 0
Code Issues 1 Pull Requests Releases Activity

Merge ACME package back into the PKI package (#19826) 

* Squash pki/acme package down to pki folder

Without refactoring most of PKI to export the storage layer, which we
were initially hesitant about, it would be nearly impossible to have the
ACME layer handle its own storage while being in the acme/ subpackage
under the pki package.

Thus, merge the two packages together again.

Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>

* Properly format errors for missing parameters

When missing required ACME request parameters, don't return Vault-level
errors, but drop into the PKI package to return properly-formatted ACME
error messages.

Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>

* Error type clarifications

Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>

* Fix GetOk with type conversion calls

Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>

---------

Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
This commit is contained in:
Alexander Scheel 2023-03-29 17:08:31 -04:00 committed by GitHub
parent f4fbca8050
commit b4c3aca7a1
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
7 changed files with 66 additions and 56 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
builtin/logical/pki/acme/errors.go → builtin/logical/pki/acme_errors.go
View File

@ -1,4 +1,4 @@
package acme
package pki
import (
"encoding/json"

24
builtin/logical/pki/acme/jws.go → builtin/logical/pki/acme_jws.go
View File

@ -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

52
builtin/logical/pki/acme/state.go → builtin/logical/pki/acme_state.go
View File

@ -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)

2
builtin/logical/pki/acme/state_test.go → builtin/logical/pki/acme_state_test.go
View File

@ -1,4 +1,4 @@
package acme
package pki
import (
"testing"

6
builtin/logical/pki/backend.go
View File

@ -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)

9
builtin/logical/pki/path_acme_directory.go
View File

@ -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

27
builtin/logical/pki/path_acme_new_account.go
View File

@ -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)