luxolus
/
open-consul
2
0
Fork 0
Code Issues 1 Pull Requests Packages Releases Wiki Activity

Merge pull request #12267 from hashicorp/dnephin/ca-relax-key-bit-validation 

ca: change the PrivateKey type/bits validation
This commit is contained in:
Daniel Nephin 2022-02-04 12:44:08 -05:00 committed by GitHub
parent 173a238eba 7b466a024b
commit f81d076129
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 111 additions and 87 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
.changelog/12267.txt Normal file
View File

@ -0,0 +1,3 @@
```release-note:bug
ca: adjust validation of PrivateKeyType/Bits with the Vault provider, to remove the error when the cert is created manually in Vault.
```

53
agent/connect/ca/provider_vault.go
View File

@ -160,6 +160,34 @@ func (v *VaultProvider) Configure(cfg ProviderConfig) error {
return nil
}
func (v *VaultProvider) ValidateConfigUpdate(prevRaw, nextRaw map[string]interface{}) error {
prev, err := ParseVaultCAConfig(prevRaw)
if err != nil {
return fmt.Errorf("failed to parse existing CA config: %w", err)
}
next, err := ParseVaultCAConfig(nextRaw)
if err != nil {
return fmt.Errorf("failed to parse new CA config: %w", err)
}
if prev.RootPKIPath != next.RootPKIPath {
return nil
}
if prev.PrivateKeyType != "" && prev.PrivateKeyType != connect.DefaultPrivateKeyType {
if prev.PrivateKeyType != next.PrivateKeyType {
return fmt.Errorf("cannot update the PrivateKeyType field without changing RootPKIPath")
}
}
if prev.PrivateKeyBits != 0 && prev.PrivateKeyBits != connect.DefaultPrivateKeyBits {
if prev.PrivateKeyBits != next.PrivateKeyBits {
return fmt.Errorf("cannot update the PrivateKeyBits field without changing RootPKIPath")
}
}
return nil
}
// renewToken uses a vaultapi.LifetimeWatcher to repeatedly renew our token's lease.
// If the token can no longer be renewed and auth method is set,
// it will re-authenticate to Vault using the auth method and restart the renewer with the new token.
@ -272,31 +300,6 @@ func (v *VaultProvider) GenerateRoot() (RootResult, error) {
if err != nil {
return RootResult{}, err
}
if rootPEM != "" {
rootCert, err := connect.ParseCert(rootPEM)
if err != nil {
return RootResult{}, err
}
// Vault PKI doesn't allow in-place cert/key regeneration. That
// means if you need to change either the key type or key bits then
// you also need to provide new mount points.
// https://www.vaultproject.io/api-docs/secret/pki#generate-root
//
// A separate bug in vault likely also requires that you use the
// ForceWithoutCrossSigning option when changing key types.
foundKeyType, foundKeyBits, err := connect.KeyInfoFromCert(rootCert)
if err != nil {
return RootResult{}, err
}
if v.config.PrivateKeyType != foundKeyType {
return RootResult{}, fmt.Errorf("cannot update the PrivateKeyType field without choosing a new PKI mount for the root CA")
}
if v.config.PrivateKeyBits != foundKeyBits {
return RootResult{}, fmt.Errorf("cannot update the PrivateKeyBits field without choosing a new PKI mount for the root CA")
}
}
}
return RootResult{PEM: rootPEM}, nil

118
agent/consul/connect_ca_endpoint_test.go
View File

@ -558,92 +558,88 @@ func TestConnectCAConfig_Vault_TriggerRotation_Fails(t *testing.T) {
t.Parallel()
testVault := ca.NewTestVaultServer(t)
defer testVault.Stop()
_, s1 := testServerWithConfig(t, func(c *Config) {
c.Build = "1.6.0"
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/",
},
newConfig := func(keyType string, keyBits int) map[string]interface{} {
return map[string]interface{}{
"Address": testVault.Addr,
"Token": testVault.RootToken,
"RootPKIPath": "pki-root/",
"IntermediatePKIPath": "pki-intermediate/",
"PrivateKeyType": keyType,
"PrivateKeyBits": keyBits,
}
})
defer s1.Shutdown()
codec := rpcClient(t, s1)
defer codec.Close()
testrpc.WaitForTestAgent(t, s1.RPC, "dc1")
// Capture the current root.
{
rootList, _, err := getTestRoots(s1, "dc1")
require.NoError(t, err)
require.Len(t, rootList.Roots, 1)
}
cases := []struct {
_, s1 := testServerWithConfig(t, func(c *Config) {
c.CAConfig = &structs.CAConfiguration{
Provider: "vault",
Config: newConfig(connect.DefaultPrivateKeyType, connect.DefaultPrivateKeyBits),
}
})
testrpc.WaitForTestAgent(t, s1.RPC, "dc1")
// note: unlike many table tests, the ordering of these cases does matter
// because any non-errored case will modify the CA config, and any subsequent
// tests will use the same agent with that new CA config.
testSteps := []struct {
name string
configFn func() (*structs.CAConfiguration, error)
configFn func() *structs.CAConfiguration
expectErr string
}{
{
name: "cannot edit key bits",
configFn: func() (*structs.CAConfiguration, error) {
name: "allow modifying key type and bits from default",
configFn: func() *structs.CAConfiguration {
return &structs.CAConfiguration{
Provider: "vault",
Config: map[string]interface{}{
"Address": testVault.Addr,
"Token": testVault.RootToken,
"RootPKIPath": "pki-root/",
"IntermediatePKIPath": "pki-intermediate/",
//
"PrivateKeyType": "ec",
"PrivateKeyBits": 384,
},
Provider: "vault",
Config: newConfig("rsa", 4096),
ForceWithoutCrossSigning: true,
}, nil
}
},
expectErr: `error generating CA root certificate: cannot update the PrivateKeyBits field without choosing a new PKI mount for the root CA`,
},
{
name: "cannot edit key type",
configFn: func() (*structs.CAConfiguration, error) {
name: "error when trying to modify key bits",
configFn: func() *structs.CAConfiguration {
return &structs.CAConfiguration{
Provider: "vault",
Config: map[string]interface{}{
"Address": testVault.Addr,
"Token": testVault.RootToken,
"RootPKIPath": "pki-root/",
"IntermediatePKIPath": "pki-intermediate/",
//
"PrivateKeyType": "rsa",
"PrivateKeyBits": 4096,
},
Provider: "vault",
Config: newConfig("rsa", 2048),
ForceWithoutCrossSigning: true,
}, nil
}
},
expectErr: `cannot update the PrivateKeyBits field without changing RootPKIPath`,
},
{
name: "error when trying to modify key type",
configFn: func() *structs.CAConfiguration {
return &structs.CAConfiguration{
Provider: "vault",
Config: newConfig("ec", 256),
ForceWithoutCrossSigning: true,
}
},
expectErr: `cannot update the PrivateKeyType field without changing RootPKIPath`,
},
{
name: "allow update that does not change key type or bits",
configFn: func() *structs.CAConfiguration {
return &structs.CAConfiguration{
Provider: "vault",
Config: newConfig("rsa", 4096),
ForceWithoutCrossSigning: true,
}
},
expectErr: `error generating CA root certificate: cannot update the PrivateKeyType field without choosing a new PKI mount for the root CA`,
},
}
for _, tc := range cases {
for _, tc := range testSteps {
t.Run(tc.name, func(t *testing.T) {
newConfig, err := tc.configFn()
require.NoError(t, err)
args := &structs.CARequest{
Datacenter: "dc1",
Config: newConfig,
Config: tc.configFn(),
}
var reply interface{}
err = msgpackrpc.CallWithCodec(codec, "ConnectCA.ConfigurationSet", args, &reply)
codec := rpcClient(t, s1)
err := msgpackrpc.CallWithCodec(codec, "ConnectCA.ConfigurationSet", args, &reply)
if tc.expectErr == "" {
require.NoError(t, err)
} else {

24
agent/consul/leader_connect_ca.go
View File

@ -781,7 +781,7 @@ func (c *CAManager) UpdateConfiguration(args *structs.CARequest) (reterr error)
}()
// Attempt to initialize the config if we failed to do so in Initialize for some reason
_, err = c.initializeCAConfig()
prevConfig, err := c.initializeCAConfig()
if err != nil {
return err
}
@ -832,6 +832,15 @@ func (c *CAManager) UpdateConfiguration(args *structs.CARequest) (reterr error)
RawConfig: args.Config.Config,
State: args.Config.State,
}
if args.Config.Provider == config.Provider {
if validator, ok := newProvider.(ValidateConfigUpdater); ok {
if err := validator.ValidateConfigUpdate(prevConfig.Config, args.Config.Config); err != nil {
return fmt.Errorf("new configuration is incompatible with previous configuration: %w", err)
}
}
}
if err := newProvider.Configure(pCfg); err != nil {
return fmt.Errorf("error configuring provider: %v", err)
}
@ -858,6 +867,19 @@ func (c *CAManager) UpdateConfiguration(args *structs.CARequest) (reterr error)
return nil
}
// ValidateConfigUpdater is an optional interface that may be implemented
// by a ca.Provider. If the provider implements this interface, the
// ValidateConfigurationUpdate will be called when a user attempts to change the
// CA configuration, and the provider type has not changed from the previous
// configuration.
type ValidateConfigUpdater interface {
// ValidateConfigUpdate should return an error if the next configuration is
// incompatible with the previous configuration.
//
// TODO: use better types after https://github.com/hashicorp/consul/issues/12238
ValidateConfigUpdate(previous, next map[string]interface{}) error
}
func (c *CAManager) primaryUpdateRootCA(newProvider ca.Provider, args *structs.CARequest, config *structs.CAConfiguration) error {
providerRoot, err := newProvider.GenerateRoot()
if err != nil {