Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

connect: Leaf Cert ModifyIndex not being incremented properly #4463

Closed
kyhavlov opened this issue Jul 27, 2018 · 1 comment
Closed

connect: Leaf Cert ModifyIndex not being incremented properly #4463

kyhavlov opened this issue Jul 27, 2018 · 1 comment
Assignees
Labels
theme/connect Anything related to Consul Connect, Service Mesh, Side Car Proxies type/bug Feature does not function as expected
Milestone

Comments

@kyhavlov
Copy link
Contributor

The ModifyIndex of leaf certs is being set based on the CAConfig table rather than something that increments with every new leaf cert signed. This means a proxy can request a new cert for a refresh and get back a cert with the same ModifyIndex, which could cause an edge case where a proxy doesn't get its certificate updated properly.

@kyhavlov kyhavlov added the type/bug Feature does not function as expected label Jul 27, 2018
@banks banks added the theme/connect Anything related to Consul Connect, Service Mesh, Side Car Proxies label Jul 30, 2018
@banks
Copy link
Member

banks commented Jul 30, 2018

In addition, the current approach of relying on the built-in CA to touch the CAProvider table on each sign to increase index (even if we implemented it right) doesn't work for Vault CA.

Once we build metadata support for certificates (planned for later in the year) all the problems go away but I suggest for now we use an explicit counter in the indexes table or something to ensure blocking and cache invalidation happens correctly.

Concretely it mostly works right now because if the proxy is actually blocking for a new leaf at the time the leaf rotates, the cache layer will deliver the new cert and cache it regardless of the index being the same.

Where it would fail now is it makes rotation racey. The following is possible (but never reproduced in practice yet):

  1. leaf rotation triggered by something (CA change or leaf near expiry)
  2. CSR Sign RPC sent to server
  3. Proxy gets to the end of blocking leaf query timeout and returns current leaf
  4. Sign RPC returns the new cert (with same ModifyIndex as the previous one due to this bug)
  5. Cache framework updates the cert in cache but still with same index
  6. Proxy issues next blocking long poll with same index it had before
  7. cache sees that the cert "hasn't changed" since indexes match and blocks waiting on next rotation

In this case the proxy will miss the certificate rotation and may end up loosing connectivity if the next rotation doesn't happen until it's current certificate has expired.

@pearkes pearkes added this to the 1.2.3 milestone Aug 7, 2018
@pearkes pearkes modified the milestones: 1.2.3, 1.3.0 Sep 11, 2018
@mkeeler mkeeler modified the milestones: 1.3.0, 1.4.0 Oct 10, 2018
@banks banks modified the milestones: 1.4.0, Upcoming Oct 15, 2018
@banks banks modified the milestones: Upcoming, 1.4.1 Nov 28, 2018
@mkeeler mkeeler self-assigned this Jan 8, 2019
mkeeler added a commit that referenced this issue Jan 10, 2019
…turned certs

This ensures that future certificate signings will have a strictly greater ModifyIndex than any previous certs signed.

Fixes #4463
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
theme/connect Anything related to Consul Connect, Service Mesh, Side Car Proxies type/bug Feature does not function as expected
Projects
None yet
Development

No branches or pull requests

4 participants