From 658e6a97bb37a7a02b7324b71796f99a86639fca Mon Sep 17 00:00:00 2001 From: Kyle Havlovitz Date: Thu, 11 Mar 2021 11:49:15 -0800 Subject: [PATCH] Merge pull request #9672 from hashicorp/ca-force-skip-xc connect/ca: Allow ForceWithoutCrossSigning for all providers --- .changelog/9672.txt | 4 + agent/consul/leader_connect_ca.go | 9 +- agent/consul/leader_connect_test.go | 127 +++++++++++++++++++++++ api/connect_ca.go | 8 ++ command/connect/ca/set/connect_ca_set.go | 10 +- website/pages/api-docs/connect/ca.mdx | 2 +- website/pages/commands/connect/ca.mdx | 7 ++ 7 files changed, 161 insertions(+), 6 deletions(-) create mode 100644 .changelog/9672.txt diff --git a/.changelog/9672.txt b/.changelog/9672.txt new file mode 100644 index 0000000000..a761fc0e4c --- /dev/null +++ b/.changelog/9672.txt @@ -0,0 +1,4 @@ +```release-note:improvement +cli: added a `-force-without-cross-signing` flag to the `ca set-config` command. +connect/ca: The ForceWithoutCrossSigning field will now work as expected for CA providers that support cross signing. +``` \ No newline at end of file diff --git a/agent/consul/leader_connect_ca.go b/agent/consul/leader_connect_ca.go index d7bcd42221..eb4189094e 100644 --- a/agent/consul/leader_connect_ca.go +++ b/agent/consul/leader_connect_ca.go @@ -887,12 +887,13 @@ func (c *CAManager) UpdateConfiguration(args *structs.CARequest) (reterr error) "You can try again with ForceWithoutCrossSigningSet but this may cause " + "disruption - see documentation for more.") } - if !canXSign && args.Config.ForceWithoutCrossSigning { - c.logger.Warn("current CA doesn't support cross signing but " + - "CA reconfiguration forced anyway with ForceWithoutCrossSigning") + if args.Config.ForceWithoutCrossSigning { + c.logger.Warn("ForceWithoutCrossSigning set, CA reconfiguration skipping cross-signing") } - if canXSign { + // If ForceWithoutCrossSigning wasn't set, attempt to have the old CA generate a + // cross-signed intermediate. + if canXSign && !args.Config.ForceWithoutCrossSigning { // Have the old provider cross-sign the new root xcCert, err := oldProvider.CrossSignCA(newRoot) if err != nil { diff --git a/agent/consul/leader_connect_test.go b/agent/consul/leader_connect_test.go index 3ea31f40b3..341600030d 100644 --- a/agent/consul/leader_connect_test.go +++ b/agent/consul/leader_connect_test.go @@ -1791,3 +1791,130 @@ func TestLeader_Consul_BadCAConfigShouldntPreventLeaderEstablishment(t *testing. require.NotEmpty(t, rootsList.Roots) require.NotNil(t, activeRoot) } + +func TestLeader_Consul_ForceWithoutCrossSigning(t *testing.T) { + require := require.New(t) + dir1, s1 := testServer(t) + defer os.RemoveAll(dir1) + defer s1.Shutdown() + codec := rpcClient(t, s1) + defer codec.Close() + + waitForLeaderEstablishment(t, s1) + + // Get the current root + rootReq := &structs.DCSpecificRequest{ + Datacenter: "dc1", + } + var rootList structs.IndexedCARoots + require.Nil(msgpackrpc.CallWithCodec(codec, "ConnectCA.Roots", rootReq, &rootList)) + require.Len(rootList.Roots, 1) + oldRoot := rootList.Roots[0] + + // Update the provider config to use a new private key, which should + // cause a rotation. + _, newKey, err := connect.GeneratePrivateKey() + require.NoError(err) + newConfig := &structs.CAConfiguration{ + Provider: "consul", + Config: map[string]interface{}{ + "LeafCertTTL": "500ms", + "PrivateKey": newKey, + "RootCert": "", + "RotationPeriod": "2160h", + "SkipValidate": true, + }, + ForceWithoutCrossSigning: true, + } + { + args := &structs.CARequest{ + Datacenter: "dc1", + Config: newConfig, + } + var reply interface{} + + require.NoError(msgpackrpc.CallWithCodec(codec, "ConnectCA.ConfigurationSet", args, &reply)) + } + + // Old root should no longer be active. + _, roots, err := s1.fsm.State().CARoots(nil) + require.NoError(err) + require.Len(roots, 2) + for _, r := range roots { + if r.ID == oldRoot.ID { + require.False(r.Active) + } else { + require.True(r.Active) + } + } +} + +func TestLeader_Vault_ForceWithoutCrossSigning(t *testing.T) { + ca.SkipIfVaultNotPresent(t) + + require := require.New(t) + testVault := ca.NewTestVaultServer(t) + defer testVault.Stop() + + _, s1 := testServerWithConfig(t, func(c *Config) { + c.Build = "1.9.1" + c.PrimaryDatacenter = "dc1" + c.CAConfig = &structs.CAConfiguration{ + Provider: "vault", + Config: map[string]interface{}{ + "Address": testVault.Addr, + "Token": testVault.RootToken, + "RootPKIPath": "pki-root/", + "IntermediatePKIPath": "pki-intermediate/", + }, + } + }) + defer s1.Shutdown() + codec := rpcClient(t, s1) + defer codec.Close() + + waitForLeaderEstablishment(t, s1) + + // Get the current root + rootReq := &structs.DCSpecificRequest{ + Datacenter: "dc1", + } + var rootList structs.IndexedCARoots + require.Nil(msgpackrpc.CallWithCodec(codec, "ConnectCA.Roots", rootReq, &rootList)) + require.Len(rootList.Roots, 1) + oldRoot := rootList.Roots[0] + + // Update the provider config to use a new PKI path, which should + // cause a rotation. + newConfig := &structs.CAConfiguration{ + Provider: "vault", + Config: map[string]interface{}{ + "Address": testVault.Addr, + "Token": testVault.RootToken, + "RootPKIPath": "pki-root-2/", + "IntermediatePKIPath": "pki-intermediate/", + }, + ForceWithoutCrossSigning: true, + } + { + args := &structs.CARequest{ + Datacenter: "dc1", + Config: newConfig, + } + var reply interface{} + + require.NoError(msgpackrpc.CallWithCodec(codec, "ConnectCA.ConfigurationSet", args, &reply)) + } + + // Old root should no longer be active. + _, roots, err := s1.fsm.State().CARoots(nil) + require.NoError(err) + require.Len(roots, 2) + for _, r := range roots { + if r.ID == oldRoot.ID { + require.False(r.Active) + } else { + require.True(r.Active) + } + } +} diff --git a/api/connect_ca.go b/api/connect_ca.go index 26a7bfb1df..9d2c266020 100644 --- a/api/connect_ca.go +++ b/api/connect_ca.go @@ -23,6 +23,14 @@ type CAConfig struct { // configuration is an error. State map[string]string + // ForceWithoutCrossSigning indicates that the CA reconfiguration should go + // ahead even if the current CA is unable to cross sign certificates. This + // risks temporary connection failures during the rollout as new leafs will be + // rejected by proxies that have not yet observed the new root cert but is the + // only option if a CA that doesn't support cross signing needs to be + // reconfigured or mirated away from. + ForceWithoutCrossSigning bool + CreateIndex uint64 ModifyIndex uint64 } diff --git a/command/connect/ca/set/connect_ca_set.go b/command/connect/ca/set/connect_ca_set.go index 696b894c01..29922b5b99 100644 --- a/command/connect/ca/set/connect_ca_set.go +++ b/command/connect/ca/set/connect_ca_set.go @@ -24,13 +24,20 @@ type cmd struct { help string // flags - configFile flags.StringValue + configFile flags.StringValue + forceWithoutCrossSigning bool } func (c *cmd) init() { c.flags = flag.NewFlagSet("", flag.ContinueOnError) c.flags.Var(&c.configFile, "config-file", "The path to the config file to use.") + c.flags.BoolVar(&c.forceWithoutCrossSigning, "force-without-cross-signing", false, + "Indicates that the CA reconfiguration should go ahead even if the current "+ + "CA is unable to cross sign certificates. This risks temporary connection "+ + "failures during the rollout as new leafs will be rejected by proxies that "+ + "have not yet observed the new root cert but is the only option if a CA that "+ + "doesn't support cross signing needs to be reconfigured or mirated away from.") c.http = &flags.HTTPFlags{} flags.Merge(c.flags, c.http.ClientFlags()) @@ -70,6 +77,7 @@ func (c *cmd) Run(args []string) int { c.UI.Error(fmt.Sprintf("Error parsing config file: %s", err)) return 1 } + config.ForceWithoutCrossSigning = c.forceWithoutCrossSigning // Set the new configuration. if _, err := client.Connect().CASetConfig(&config, nil); err != nil { diff --git a/website/pages/api-docs/connect/ca.mdx b/website/pages/api-docs/connect/ca.mdx index 434c45f5ae..900e0d963a 100644 --- a/website/pages/api-docs/connect/ca.mdx +++ b/website/pages/api-docs/connect/ca.mdx @@ -175,7 +175,7 @@ The table below shows this endpoint's support for providers, see [Provider Config](/docs/connect/ca). - `ForceWithoutCrossSigning` `(bool: )` - Indicates that the CA change - should be force to complete even if the current CA doesn't support cross + should be forced to complete even if the current CA doesn't support cross signing. Changing root without cross-signing may cause temporary connection failures until the rollout completes. See [Forced Rotation Without Cross-Signing](/docs/connect/ca#forced-rotation-without-cross-signing) diff --git a/website/pages/commands/connect/ca.mdx b/website/pages/commands/connect/ca.mdx index 8d82466841..a0f0a6d875 100644 --- a/website/pages/commands/connect/ca.mdx +++ b/website/pages/commands/connect/ca.mdx @@ -82,6 +82,13 @@ Usage: `consul connect ca set-config [options]` The format of this config file matches the request payload documented in the [Update CA Configuration API](/api/connect/ca#update-ca-configuration). +- `-force-without-cross-signing` `(bool: )` - Indicates that the CA change + should be forced to complete even if the current CA doesn't support cross + signing. Changing root without cross-signing may cause temporary connection + failures until the rollout completes. See [Forced Rotation Without + Cross-Signing](/docs/connect/ca#forced-rotation-without-cross-signing) + for more detail. + The output looks like this: ```