[ca/manager] Stop encrypting the raft root CA key based on env vars#2552
[ca/manager] Stop encrypting the raft root CA key based on env vars#2552cyli wants to merge 1 commit into
Conversation
cyli
changed the title
cyli
changed the title
Codecov Report
@@ Coverage Diff @@
## master #2552 +/- ##
==========================================
+ Coverage 61.4% 61.44% +0.04%
==========================================
Files 49 133 +84
Lines 6332 21729 +15397
==========================================
+ Hits 3888 13352 +9464
- Misses 2067 6938 +4871
- Partials 377 1439 +1062 |
cyli
force-pushed
the
remove-root-ca-key-encryption
branch
from
030fa00 to
e532f43
Compare
|
This will also make it easier and cleaner to implement the change where we remove the |
dperny
left a comment
dperny
left a comment
There was a problem hiding this comment.
Not your fault, but something about this diff makes it super hard to parse. I've reviewed as best as I can and it LGTM, but I would like some more comments in the noted place if you don't mind. Not a blocker though.
| } | ||
|
|
||
| func TestNewRootCA(t *testing.T) { | ||
| // even if root encryption passphrases are set, they are not used |
There was a problem hiding this comment.
what does this mean? can you add some more comments explaining what this accomplishes and tests?
|
@dperny Thanks for reviewing! Sorry about the confusion - would it help if I explained how the two different env vars used to work, in comments? |
|
Sure, yeah. It would be fine. Just something along the lines of
|
cyli
force-pushed
the
remove-root-ca-key-encryption
branch
from
e532f43 to
79964be
Compare
| // If we don't have a KEK, we won't need to, or can't, decrypt anything | ||
| strPassphrase := os.Getenv(ca.PassphraseENVVar) | ||
| strPassphrasePrev := os.Getenv(ca.PassphraseENVVarPrev) | ||
| if strPassphrase == "" && strPassphrasePrev == "" { |
There was a problem hiding this comment.
Should we just ignore this and try to resolve the KEK?
There was a problem hiding this comment.
Er, sorry, could you explain? Do you mean ignore ca.PassphraseENVVarPrev, and just expect someone to give us the current passphrase?
|
|
||
| // Tests manager rotates encryption of root key data in the raft store | ||
| func TestManagerEncryptsDecryptsRootKeyMaterial(t *testing.T) { | ||
| // If the root CA key was encrypted in raft, on startup, the manager would decrypt the key |
There was a problem hiding this comment.
// TestManagerDecryptsRootKeyMaterial ...
allow decrypting, since that feature was deprecated almost a year ago. Signed-off-by: cyli <ying.li@docker.com>
cyli
force-pushed
the
remove-root-ca-key-encryption
branch
from
79964be to
de11799
Compare
|
After IRL discussion with @stevvooe and @anshulpundir, closing this in favor of #2573 instead. Thanks for reviewing this and sorry for wasting your time @stevvooe and @dperny! |
3 participants
This feature was never documented nor advertised, and was deprecated almost a year ago. So remove support for encrypting and rotating the root CA key KEK (key-encrypting-key), and only support decrypting the root CA key, if it's encrypted.
This removes some useless code and extra complexity from the manager and the ca package.