Allow KeyStore to be configured with multiple keys #3335
Conversation
Fixes #3304 Add a new configuration structure for encryption keys. This allows a default key and algorithm to be specified, along with fallbacks. A backwards compatibility mechanism is provided which will allow the keystore to work with the existing configuration structure.
| @@ -297,3 +273,33 @@ func getDefaultValueForInt64(value string) (any, error) { | |||
| // Return the original error, not time.ParseDuration's error | |||
| return nil, err | |||
| } | |||
|
|
|||
| func getDefaultValue(field reflect.StructField, value string, valueName string) any { | |||
There was a problem hiding this comment.
Factored out into a function to avoid cyclomatic complexity limits
| defaultValue, err = strconv.ParseBool(value) | ||
| case reflect.Slice: | ||
| defaultValue = nil | ||
| case reflect.Map: |
There was a problem hiding this comment.
This is the new addition
| KeyIDs []string `mapstructure:"key_ids"` | ||
| Algorithms []string `mapstructure:"algorithms"` |
There was a problem hiding this comment.
Should this be a []struct{KeyID string, Algo string}? I'm wondering what happens if the two arrays are not the same length.
There was a problem hiding this comment.
It is expected that they will not always be the same length. For example, if we rotate a key, but do not change the algorithm, then we will add a fallback key but not a corresponding algorithm.
The selection of key/algorithm during decryption is based on the data we store in the database (see the EncryptedData struct in models.go) and the selection of the algorithm during encryption is based on the values in the DefaultCrypto struct. The fallback values state which algorithms and keys should be loaded at startup to support old data.
| } | ||
|
|
||
| var keystoreCfg serverconfig.LocalFileKeyStoreConfig | ||
| err := mapstructure.Decode(config.KeyStore.Config, &keystoreCfg) |
There was a problem hiding this comment.
This is because Config is a map[string]any, right?
(I'd like to minimize the total area of mapstructure in our code, though I'm willing to be convinced that I shouldn't)
There was a problem hiding this comment.
Yes, it's because the exact type of Config is not known until the Type field is read.
Perhaps this could be done as part of config parsing, but I felt it would be better to do at the point where we make use of the config.
There was a problem hiding this comment.
As I said, I'm okay with this pattern -- it would be nice if we could 👍 or 👎 the config at parsing time, but that's already not the case.
Co-authored-by: Evan Anderson <evan@stacklok.com>
Fixes #3304
Add a new configuration structure for encryption keys. This allows a default key and algorithm to be specified, along with fallbacks. A backwards compatibility mechanism is provided which will allow the keystore to work with the existing configuration structure.
Summary
Provide a brief overview of the changes and the issue being addressed.
Explain the rationale and any background necessary for understanding the changes.
List dependencies required by this change, if any.
Fixes #(related issue)
Change Type
Mark the type of change your PR introduces:
Testing
Outline how the changes were tested, including steps to reproduce and any relevant configurations.
Attach screenshots if helpful.
Review Checklist: