ca: Vault provider creates a new intermediate on every leader rotation

[dnephin]

Currently we call Provider.GenerateIntermediate in the primary DC every time a new leader is elected (either on startup or leader election). This is not a problem for the Consul or AWS PCA providers because those don't use intermediate certs in the primary.

The Vault provider however, creates a new intermediate certificate every time. In a scenario where there are more frequent leader elections, this could lead to a very large list of IntermediateCerts being stored and sent to clients unnecessarily.

In the secondary we check ActiveIntermediate which prevents the creation of these extra intermediates.

We should do something similar in the primary DC. Instead of generating a new one each time, we should see if one already exists and use it.

I have not written a test to reproduce this behaviour.

[dnephin]

Seems worth mentioning that this issue will cause the "secondary CA roots watch" routine to run in all secondary DCs on every leader rotation in the primary DC. The logic seems correct right now in a secondary and it shouldn't also generate a new intermediate in the secondary as a result. But it's possible this was wrong in some versions and would have also triggered a new intermediate in secondary DCs.

[lstoll]

I don't think this is completely accurate - while a new intermediate is generated on leader election, the list does not grow. Immediately before the intermediate is generated the newCARoot function is called, which constructs a fresh copy of the stored state. The new intermediate is then appended on to this empty, freshly initialised root and persisted.

This results in only the active intermediate being passed around. While it could probably be generated less, in practice I don't think it has much impact.

[eikenb]

Thanks for the input @lstoll and well timed! I've been digging into this and have come to the same conclusion. After studying the code in use and narrowing the scope I've been trying to reproduce this all day and the list of intermediates never grows.

It creates new intermediates each time the related code is run, but the new intermediate always replaces the old.

I'm going to keep at it the rest of the day to be sure, but I'll probably be closing this as non-reproducible.

[eikenb]

Just to add a note on how I've tried to reproduce.

I was able to narrow all the CA related setup code under primaryInitialize and even more specifically I'm pretty sure that GenerateIntermediate contains all the relevant code for the intermediate CA generation.

I asked the Vault team to be sure how I was checking the correct thing in the tests. They specifically pointed me to listing issuers as the way to check. This value never grew no matter how many times I ran GenerateIntermidiate or primaryInitialize.

[eikenb]

I was playing around with generating mermaid charts from tracebacks, and while it wasn't super useful, I'll include the call stack diagram I generated while figuring out the call path just to get some more use out of it...

flowchart TB
	7[ca.VaultProvider.GenerateIntermediate]
	6[consul.CAManager.primaryInitialize]
	5[consul.CAManager.Initialize]
	4[consul.CAManager.Start]
	3[consul.Server.startConnectLeader]
	2[consul.Server.establishLeadership]
	1[consul.Server.leaderLoop]
	0[consul.Server.monitorLeadership.func1]

	0 --> 1 --> 2 --> 3 --> 4 --> 5 --> 6 --> 7

Loading

[eikenb]

Closing the issue as non-reproducible/non-existent.

The intermediate cert is recreated each time GenerateIntermediate is called and that intermediate cert becomes the 1 issuer of those certs replacing the old. The old cert will still be valid, but will only be used by whichever service uses it.

It creates a new cert each call which replaces the previous so there is no..

a very large list of IntermediateCerts being stored and sent to clients unnecessarily"

Which is the crux of this bug and it not happening means this issue is done.

[dnephin]

Thanks for looking into it @eikenb!

[jkirschner-hashicorp]

Re-opening this issue in light of hashicorp/vault#22980, which suggests that the number of non-expired issuers in the Vault PKI secrets engine is growing. That's still a problem, even if the list of intermediate certs being stored/passed around in Consul isn't growing.