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.

Sign up for GitHub

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

Jump to bottom

PKI - Fix order of chain building writes #17772

Merged
merged 5 commits into from Nov 3, 2022
Merged

PKI - Fix order of chain building writes #17772

merged 5 commits into from Nov 3, 2022

Commits on Nov 2, 2022

  1. Ensure correct write ordering in rebuildIssuersChains 

    When troubleshooting a recent migration failure from 1.10->1.11, it was
noted that some PKI mounts had bad chain construction despite having
valid, chaining issuers. Due to the cluster's leadership trashing
between nodes, the migration logic was re-executed several times,
partially succeeding each time. While the legacy CA bundle migration
logic was written with this in mind, one shortcoming in the chain
building code lead us to truncate the ca_chain: by sorting the list of
issuers after including non-written issuers (with random IDs), these
issuers would occasionally be persisted prior to storage _prior_ to
existing CAs with modified chains.

The migration code carefully imported the active issuer prior to its
parents. However, due to this bug, there was a chance that, if write to
the pending parent succeeded but updating the active issuer didn't, the
active issuer's ca_chain field would only contain the self-reference and
not the parent's reference as well. Ultimately, a workaround of setting
and subsequently unsetting a manual chain would force a chain
regeneration.

In this patch, we simply fix the write ordering: because we need to
ensure a stable chain sorting, we leave the sort location in the same
place, but delay writing the provided referenceCert to the last
position. This is because the reference is meant to be the user-facing
action: without transactional write capabilities, other chains may
succeed, but if the last user-facing action fails, the user will
hopefully retry the action. This will also correct migration, by
ensuring the subsequent issuer import will be attempted again,
triggering another chain build and only persisting this issuer when
all other issuers have also been updated.

Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
    @cipherboy
    cipherboy committed Nov 2, 2022
    Configuration menu
    Copy the full SHA
    45f868c View commit details
    Browse the repository at this point in the history

  2. Remigrate ca_chains to fix any missing issuers 

    In the previous commit, we identified an issue that would occur on
legacy issuer migration to the new storage format. This is easy enough
to detect for any given mount (by an operator), but automating scanning
and remediating all PKI mounts in large deployments might be difficult.

Write a new storage migration version to regenerate all chains on
upgrade, once.

Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
    @cipherboy
    cipherboy committed Nov 2, 2022
    Configuration menu
    Copy the full SHA
    8892d14 View commit details
    Browse the repository at this point in the history

  3. Add changelog entry 

    Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
    @cipherboy
    cipherboy committed Nov 2, 2022
    Configuration menu
    Copy the full SHA
    ee7c4cf View commit details
    Browse the repository at this point in the history

  4. Add issue to PKI considerations documentation 

    Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
    @cipherboy
    cipherboy committed Nov 2, 2022
    Configuration menu
    Copy the full SHA
    6f20e65 View commit details
    Browse the repository at this point in the history

  5. Correct %v -> %w in chain building errs 

    Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
    @cipherboy
    cipherboy committed Nov 2, 2022
    Configuration menu
    Copy the full SHA
    b964e77 View commit details
    Browse the repository at this point in the history