From a32342507f2cdb906e2baf29be4a55757d7a3a98 Mon Sep 17 00:00:00 2001 From: Alexander Scheel Date: Tue, 25 Apr 2023 16:47:52 -0400 Subject: [PATCH] Enforce proper URL in ACME headers (#20357) See RFC 8555 Section 6.4. Signed-off-by: Alexander Scheel --- builtin/logical/pki/acme_state.go | 18 +++++++++++++++- builtin/logical/pki/acme_wrappers.go | 32 +++++++++++++++------------- 2 files changed, 34 insertions(+), 16 deletions(-) diff --git a/builtin/logical/pki/acme_state.go b/builtin/logical/pki/acme_state.go index 364dc5dbb..66746fb7b 100644 --- a/builtin/logical/pki/acme_state.go +++ b/builtin/logical/pki/acme_state.go @@ -389,7 +389,7 @@ func saveAuthorizationAtPath(sc *storageContext, path string, authz *ACMEAuthori return nil } -func (a *acmeState) ParseRequestParams(ac *acmeContext, data *framework.FieldData) (*jwsCtx, map[string]interface{}, error) { +func (a *acmeState) ParseRequestParams(ac *acmeContext, req *logical.Request, data *framework.FieldData) (*jwsCtx, map[string]interface{}, error) { var c jwsCtx var m map[string]interface{} @@ -415,6 +415,22 @@ func (a *acmeState) ParseRequestParams(ac *acmeContext, data *framework.FieldDat return nil, nil, fmt.Errorf("invalid or reused nonce: %w", ErrBadNonce) } + // If the path is incorrect, reject the request. + // + // See RFC 8555 Section 6.4. Request URL Integrity: + // + // > As noted in Section 6.2, all ACME request objects carry a "url" + // > header parameter in their protected header. ... On receiving such + // > an object in an HTTP request, the server MUST compare the "url" + // > header parameter to the request URL. If the two do not match, + // > then the server MUST reject the request as unauthorized. + if len(c.Url) == 0 { + return nil, nil, fmt.Errorf("missing required parameter 'url' in 'protected': %w", ErrMalformed) + } + if ac.clusterUrl.JoinPath(req.Path).String() != c.Url { + return nil, nil, fmt.Errorf("invalid value for 'url' in 'protected': got '%v' expected '%v': %w", c.Url, ac.clusterUrl.JoinPath(req.Path).String(), ErrUnauthorized) + } + rawPayloadBase64, ok := data.GetOk("payload") if !ok { return nil, nil, fmt.Errorf("missing required field 'payload': %w", ErrMalformed) diff --git a/builtin/logical/pki/acme_wrappers.go b/builtin/logical/pki/acme_wrappers.go index ba07f4419..114bc015b 100644 --- a/builtin/logical/pki/acme_wrappers.go +++ b/builtin/logical/pki/acme_wrappers.go @@ -17,10 +17,11 @@ import ( type acmeContext struct { // baseUrl is the combination of the configured cluster local URL and the acmePath up to /acme/ - baseUrl *url.URL - sc *storageContext - role *roleEntry - issuer *issuerEntry + baseUrl *url.URL + clusterUrl *url.URL + sc *storageContext + role *roleEntry + issuer *issuerEntry } type ( @@ -53,7 +54,7 @@ func (b *backend) acmeWrapper(op acmeOperation) framework.OperationFunc { return nil, fmt.Errorf("%w: Can not perform ACME operations until migration has completed", ErrServerInternal) } - acmeBaseUrl, err := getAcmeBaseUrl(sc, r.Path) + acmeBaseUrl, clusterBase, err := getAcmeBaseUrl(sc, r.Path) if err != nil { return nil, err } @@ -64,10 +65,11 @@ func (b *backend) acmeWrapper(op acmeOperation) framework.OperationFunc { } acmeCtx := &acmeContext{ - baseUrl: acmeBaseUrl, - sc: sc, - role: role, - issuer: issuer, + baseUrl: acmeBaseUrl, + clusterUrl: clusterBase, + sc: sc, + role: role, + issuer: issuer, } return op(acmeCtx, r, data) @@ -144,7 +146,7 @@ func getAcmeRoleAndIssuer(sc *storageContext, data *framework.FieldData) (*roleE func (b *backend) acmeParsedWrapper(op acmeParsedOperation) framework.OperationFunc { return b.acmeWrapper(func(acmeCtx *acmeContext, r *logical.Request, fields *framework.FieldData) (*logical.Response, error) { - user, data, err := b.acmeState.ParseRequestParams(acmeCtx, fields) + user, data, err := b.acmeState.ParseRequestParams(acmeCtx, r, fields) if err != nil { return nil, err } @@ -256,19 +258,19 @@ func buildAcmeFrameworkPaths(b *backend, patternFunc func(b *backend, pattern st return patterns } -func getAcmeBaseUrl(sc *storageContext, path string) (*url.URL, error) { +func getAcmeBaseUrl(sc *storageContext, path string) (*url.URL, *url.URL, error) { cfg, err := sc.getClusterConfig() if err != nil { - return nil, fmt.Errorf("failed loading cluster config: %w", err) + return nil, nil, fmt.Errorf("failed loading cluster config: %w", err) } if cfg.Path == "" { - return nil, fmt.Errorf("ACME feature requires local cluster path configuration to be set: %w", ErrServerInternal) + return nil, 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", ErrServerInternal) + return nil, nil, fmt.Errorf("ACME feature a proper URL configured in local cluster path: %w", ErrServerInternal) } directoryPrefix := "" @@ -277,5 +279,5 @@ func getAcmeBaseUrl(sc *storageContext, path string) (*url.URL, error) { directoryPrefix = path[0:lastIndex] } - return baseUrl.JoinPath(directoryPrefix, "/acme/"), nil + return baseUrl.JoinPath(directoryPrefix, "/acme/"), baseUrl, nil }