diff --git a/builtin/logical/pki/ocsp.go b/builtin/logical/pki/path_ocsp.go similarity index 98% rename from builtin/logical/pki/ocsp.go rename to builtin/logical/pki/path_ocsp.go index 71236528f..4c06fcf56 100644 --- a/builtin/logical/pki/ocsp.go +++ b/builtin/logical/pki/path_ocsp.go @@ -13,6 +13,7 @@ import ( "io" "math/big" "net/http" + "net/url" "strings" "time" @@ -248,7 +249,12 @@ func fetchDerEncodedRequest(request *logical.Request, data *framework.FieldData) return nil, errors.New("request is too large") } - return base64.StdEncoding.DecodeString(base64Req) + unescapedBase64, err := url.QueryUnescape(base64Req) + if err != nil { + return nil, fmt.Errorf("failed to unescape base64 string: %w", err) + } + + return base64.StdEncoding.DecodeString(unescapedBase64) case logical.UpdateOperation: // POST bodies should contain the binary form of the DER request. // NOTE: Writing an empty update request to Vault causes a nil request.HTTPRequest, and that object diff --git a/builtin/logical/pki/ocsp_test.go b/builtin/logical/pki/path_ocsp_test.go similarity index 86% rename from builtin/logical/pki/ocsp_test.go rename to builtin/logical/pki/path_ocsp_test.go index 22a2d3da3..5e4bd4f74 100644 --- a/builtin/logical/pki/ocsp_test.go +++ b/builtin/logical/pki/path_ocsp_test.go @@ -9,11 +9,15 @@ import ( "fmt" "io" "net/http" + "net/url" "strconv" "strings" "testing" "time" + vaulthttp "github.com/hashicorp/vault/http" + "github.com/hashicorp/vault/vault" + "github.com/hashicorp/vault/sdk/logical" "github.com/stretchr/testify/require" "golang.org/x/crypto/ocsp" @@ -362,6 +366,102 @@ func TestOcsp_MultipleMatchingIssuersOneWithoutSigningUsage(t *testing.T) { requireOcspResponseSignedBy(t, ocspResp, rotatedCert) } +// Make sure OCSP GET/POST requests work through the entire stack, and not just +// through the quicker backend layer the other tests are doing. +func TestOcsp_HigherLevel(t *testing.T) { + t.Parallel() + coreConfig := &vault.CoreConfig{ + LogicalBackends: map[string]logical.Factory{ + "pki": Factory, + }, + } + cluster := vault.NewTestCluster(t, coreConfig, &vault.TestClusterOptions{ + HandlerFunc: vaulthttp.Handler, + }) + cluster.Start() + defer cluster.Cleanup() + client := cluster.Cores[0].Client + mountPKIEndpoint(t, client, "pki") + resp, err := client.Logical().Write("pki/root/generate/internal", map[string]interface{}{ + "key_type": "ec", + "common_name": "root-ca.com", + "ttl": "600h", + }) + + require.NoError(t, err, "error generating root ca: %v", err) + require.NotNil(t, resp, "expected ca info from root") + + issuerCert := parseCert(t, resp.Data["certificate"].(string)) + + resp, err = client.Logical().Write("pki/roles/example", map[string]interface{}{ + "allowed_domains": "example.com", + "allow_subdomains": "true", + "no_store": "false", // make sure we store this cert + "max_ttl": "1h", + "key_type": "ec", + }) + require.NoError(t, err, "error setting up pki role: %v", err) + + resp, err = client.Logical().Write("pki/issue/example", map[string]interface{}{ + "common_name": "test.example.com", + "ttl": "15m", + }) + require.NoError(t, err, "error issuing certificate: %v", err) + require.NotNil(t, resp, "got nil response from issuing request") + certToRevoke := parseCert(t, resp.Data["certificate"].(string)) + serialNum := resp.Data["serial_number"].(string) + + // Revoke the certificate + resp, err = client.Logical().Write("pki/revoke", map[string]interface{}{ + "serial_number": serialNum, + }) + require.NoError(t, err, "error revoking certificate: %v", err) + require.NotNil(t, resp, "got nil response from revoke") + + // Make sure that OCSP handler responds properly + ocspReq := generateRequest(t, crypto.SHA256, certToRevoke, issuerCert) + ocspPostReq := client.NewRequest(http.MethodPost, "/v1/pki/ocsp") + ocspPostReq.Headers.Set("Content-Type", "application/ocsp-request") + ocspPostReq.BodyBytes = ocspReq + rawResp, err := client.RawRequest(ocspPostReq) + require.NoError(t, err, "failed sending ocsp post request") + + require.Equal(t, 200, rawResp.StatusCode) + require.Equal(t, ocspResponseContentType, rawResp.Header.Get("Content-Type")) + bodyReader := rawResp.Body + respDer, err := io.ReadAll(bodyReader) + bodyReader.Close() + require.NoError(t, err, "failed reading response body") + + ocspResp, err := ocsp.ParseResponse(respDer, issuerCert) + require.NoError(t, err, "parsing ocsp get response") + + require.Equal(t, ocsp.Revoked, ocspResp.Status) + require.Equal(t, issuerCert, ocspResp.Certificate) + require.Equal(t, certToRevoke.SerialNumber, ocspResp.SerialNumber) + + // Test OCSP Get request for ocsp + urlEncoded := url.QueryEscape(base64.StdEncoding.EncodeToString(ocspReq)) + ocspGetReq := client.NewRequest(http.MethodGet, "/v1/pki/ocsp/"+urlEncoded) + ocspGetReq.Headers.Set("Content-Type", "application/ocsp-request") + rawResp, err = client.RawRequest(ocspGetReq) + require.NoError(t, err, "failed sending ocsp get request") + + require.Equal(t, 200, rawResp.StatusCode) + require.Equal(t, ocspResponseContentType, rawResp.Header.Get("Content-Type")) + bodyReader = rawResp.Body + respDer, err = io.ReadAll(bodyReader) + bodyReader.Close() + require.NoError(t, err, "failed reading response body") + + ocspResp, err = ocsp.ParseResponse(respDer, issuerCert) + require.NoError(t, err, "parsing ocsp get response") + + require.Equal(t, ocsp.Revoked, ocspResp.Status) + require.Equal(t, issuerCert, ocspResp.Certificate) + require.Equal(t, certToRevoke.SerialNumber, ocspResp.SerialNumber) +} + func TestOcsp_ValidRequests(t *testing.T) { type caKeyConf struct { keyType string @@ -588,7 +688,7 @@ func SendOcspRequest(t *testing.T, b *backend, s logical.Storage, getOrPost stri } func sendOcspGetRequest(b *backend, s logical.Storage, ocspRequest []byte) (*logical.Response, error) { - urlEncoded := base64.StdEncoding.EncodeToString(ocspRequest) + urlEncoded := url.QueryEscape(base64.StdEncoding.EncodeToString(ocspRequest)) return CBRead(b, s, "ocsp/"+urlEncoded) } diff --git a/changelog/18938.txt b/changelog/18938.txt new file mode 100644 index 000000000..de937fc1a --- /dev/null +++ b/changelog/18938.txt @@ -0,0 +1,3 @@ +```release-note:bug +secrets/pki: OCSP GET request parameter was not being URL unescaped before processing. +```