diff --git a/.changelog/9240.txt b/.changelog/9240.txt new file mode 100644 index 0000000000..5e665e6bcc --- /dev/null +++ b/.changelog/9240.txt @@ -0,0 +1,3 @@ +```release-note:security +Increase the permissions to read from the `/connect/ca/configuration` endpoint to `operator:write`. Previously Connect CA configuration, including the private key, set via this endpoint could be read back by an operator with `operator:read` privileges. [CVE-2020-28053](https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2020-28053) +``` \ No newline at end of file diff --git a/agent/consul/connect_ca_endpoint.go b/agent/consul/connect_ca_endpoint.go index 2198c0ec65..e7ae0c7237 100644 --- a/agent/consul/connect_ca_endpoint.go +++ b/agent/consul/connect_ca_endpoint.go @@ -67,7 +67,7 @@ func (s *ConnectCA) ConfigurationGet( if err != nil { return err } - if rule != nil && rule.OperatorRead(nil) != acl.Allow { + if rule != nil && rule.OperatorWrite(nil) != acl.Allow { return acl.ErrPermissionDenied } diff --git a/agent/consul/connect_ca_endpoint_test.go b/agent/consul/connect_ca_endpoint_test.go index d55c7e4b67..4e2885e473 100644 --- a/agent/consul/connect_ca_endpoint_test.go +++ b/agent/consul/connect_ca_endpoint_test.go @@ -9,8 +9,7 @@ import ( "testing" "time" - "github.com/stretchr/testify/require" - + "github.com/hashicorp/consul/acl" "github.com/hashicorp/consul/agent/connect" ca "github.com/hashicorp/consul/agent/connect/ca" "github.com/hashicorp/consul/agent/structs" @@ -18,6 +17,7 @@ import ( "github.com/hashicorp/consul/testrpc" msgpackrpc "github.com/hashicorp/net-rpc-msgpackrpc" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func testParseCert(t *testing.T, pemValue string) *x509.Certificate { @@ -144,6 +144,93 @@ func TestConnectCAConfig_GetSet(t *testing.T) { } } +func TestConnectCAConfig_GetSet_ACLDeny(t *testing.T) { + t.Parallel() + + dir1, s1 := testServerWithConfig(t, func(c *Config) { + c.ACLDatacenter = "dc1" + c.ACLsEnabled = true + c.ACLMasterToken = TestDefaultMasterToken + c.ACLDefaultPolicy = "deny" + }) + defer os.RemoveAll(dir1) + defer s1.Shutdown() + + codec := rpcClient(t, s1) + defer codec.Close() + + testrpc.WaitForLeader(t, s1.RPC, "dc1") + + opReadToken, err := upsertTestTokenWithPolicyRules( + codec, TestDefaultMasterToken, "dc1", `operator = "read"`) + require.NoError(t, err) + + opWriteToken, err := upsertTestTokenWithPolicyRules( + codec, TestDefaultMasterToken, "dc1", `operator = "write"`) + require.NoError(t, err) + + // Update a config value + newConfig := &structs.CAConfiguration{ + Provider: "consul", + Config: map[string]interface{}{ + "PrivateKey": ` +-----BEGIN EC PRIVATE KEY----- +MHcCAQEEIMoTkpRggp3fqZzFKh82yS4LjtJI+XY+qX/7DefHFrtdoAoGCCqGSM49 +AwEHoUQDQgAEADPv1RHVNRfa2VKRAB16b6rZnEt7tuhaxCFpQXPj7M2omb0B9Fav +q5E0ivpNtv1QnFhxtPd7d5k4e+T7SkW1TQ== +-----END EC PRIVATE KEY-----`, + "RootCert": ` +-----BEGIN CERTIFICATE----- +MIICjDCCAjKgAwIBAgIIC5llxGV1gB8wCgYIKoZIzj0EAwIwFDESMBAGA1UEAxMJ +VGVzdCBDQSAyMB4XDTE5MDMyMjEzNTgyNloXDTI5MDMyMjEzNTgyNlowDjEMMAoG +A1UEAxMDd2ViMFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAEADPv1RHVNRfa2VKR +AB16b6rZnEt7tuhaxCFpQXPj7M2omb0B9Favq5E0ivpNtv1QnFhxtPd7d5k4e+T7 +SkW1TaOCAXIwggFuMA4GA1UdDwEB/wQEAwIDuDAdBgNVHSUEFjAUBggrBgEFBQcD +AgYIKwYBBQUHAwEwDAYDVR0TAQH/BAIwADBoBgNVHQ4EYQRfN2Q6MDc6ODc6M2E6 +NDA6MTk6NDc6YzM6NWE6YzA6YmE6NjI6ZGY6YWY6NGI6ZDQ6MDU6MjU6NzY6M2Q6 +NWE6OGQ6MTY6OGQ6Njc6NWU6MmU6YTA6MzQ6N2Q6ZGM6ZmYwagYDVR0jBGMwYYBf +ZDE6MTE6MTE6YWM6MmE6YmE6OTc6YjI6M2Y6YWM6N2I6YmQ6ZGE6YmU6YjE6OGE6 +ZmM6OWE6YmE6YjU6YmM6ODM6ZTc6NWU6NDE6NmY6ZjI6NzM6OTU6NTg6MGM6ZGIw +WQYDVR0RBFIwUIZOc3BpZmZlOi8vMTExMTExMTEtMjIyMi0zMzMzLTQ0NDQtNTU1 +NTU1NTU1NTU1LmNvbnN1bC9ucy9kZWZhdWx0L2RjL2RjMS9zdmMvd2ViMAoGCCqG +SM49BAMCA0gAMEUCIGC3TTvvjj76KMrguVyFf4tjOqaSCRie3nmHMRNNRav7AiEA +pY0heYeK9A6iOLrzqxSerkXXQyj5e9bE4VgUnxgPU6g= +-----END CERTIFICATE-----`, + }, + } + + args := &structs.CARequest{ + Datacenter: "dc1", + Config: newConfig, + WriteRequest: structs.WriteRequest{Token: TestDefaultMasterToken}, + } + var reply interface{} + require.NoError(t, msgpackrpc.CallWithCodec(codec, "ConnectCA.ConfigurationSet", args, &reply)) + + t.Run("deny get with operator:read", func(t *testing.T) { + args := &structs.DCSpecificRequest{ + Datacenter: "dc1", + QueryOptions: structs.QueryOptions{Token: opReadToken.SecretID}, + } + + var reply structs.CAConfiguration + err = msgpackrpc.CallWithCodec(codec, "ConnectCA.ConfigurationGet", args, &reply) + assert.True(t, acl.IsErrPermissionDenied(err)) + }) + + t.Run("allow get with operator:write", func(t *testing.T) { + args := &structs.DCSpecificRequest{ + Datacenter: "dc1", + QueryOptions: structs.QueryOptions{Token: opWriteToken.SecretID}, + } + + var reply structs.CAConfiguration + err = msgpackrpc.CallWithCodec(codec, "ConnectCA.ConfigurationGet", args, &reply) + assert.False(t, acl.IsErrPermissionDenied(err)) + assert.Equal(t, newConfig.Config, reply.Config) + }) +} + // This test case tests that the logic around forcing a rotation without cross // signing works when requested (and is denied when not requested). This occurs // if the current CA is not able to cross sign external CA certificates. diff --git a/website/pages/api-docs/connect/ca.mdx b/website/pages/api-docs/connect/ca.mdx index 02ba96fd66..434c45f5ae 100644 --- a/website/pages/api-docs/connect/ca.mdx +++ b/website/pages/api-docs/connect/ca.mdx @@ -121,7 +121,9 @@ The table below shows this endpoint's support for | Blocking Queries | Consistency Modes | Agent Caching | ACL Required | | ---------------- | ----------------- | ------------- | --------------- | -| `YES` | `all` | `none` | `operator:read` | +| `YES` | `all` | `none` | `operator:write` 1 | + +1 ACL required was operator:read prior to versions 1.8.6, 1.7.10, and 1.6.10. ### Sample Request