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

Clean up stale SignifyClient key state after rotate by Re-GETting managed identifier keystate #306

Open
kentbull opened this issue Jan 22, 2025 · 1 comment
Labels
bug Something isn't working triage Needs assessment

Comments

@kentbull
Copy link
Collaborator

Version

0.3.0-rc1

Environment

NodeJS, MacOS Sequoia

Expected behavior

The Keeper kidx should update to the latest keystate after rotating a managed identifier (sub-identifier of Client AID).

Actual behavior

Currently the kidx becomes stale in any active instances of a SignifyClient so that when a SignifyClient.manager.get(HabState) invocation happens then the returned Keeper has an old, stale kidx.

Steps to reproduce

  1. Initialize three separate SignifyClient instances with three separate salts. Use these same clients for all below steps.
  2. Create a multisig AID with three participants, Q1, Q2, and Q3.
  3. Get the AID state of each participant with SignifyClient.identifiers.get(name). Use this same client for the individual single sig rotation as well as the multisig rotation operations. Do NOT re-execute SignifyClient.identifiers.get(name)because that will refresh thekidx. The bug is that kidx` does not automatically update upon rotating a managed AID.
  4. Rotate each of the participating identifiers and refresh keystate between all three identifiers.
  5. Begin a multisig rotation by creating a drt event with SignifyClient.identifiers().rotate(multisigName, kargs).
  6. Create the exchange message for this multisig and send it to KERIA
  7. KERIA will fail signature validation when ExchangeCollectionEnd.on_post makes the call to:
agent.hby.psr.parseOne(ims=bytearray(ims))

after it gets through

# parseOne -> 
#   onceParsator -> 
#     msgParsator ->
#       Exchanger.processEvent ->
#         ...
        _, eventing.verifySigs(serder.raw, sigers, verfers) ->
# eventing.verifySigs
          ...
          if siger.verfer.verify(siger.raw, raw):  # <-- fails signature verification here because the stale `kidx` was used to select the signing key in the SignifyClient
@kentbull kentbull added bug Something isn't working triage Needs assessment labels Jan 22, 2025
@kentbull
Copy link
Collaborator Author

I'm seeing that it's actually we can narrow the problem further to the HabState returned from the SignifyClient.identifiers().get(name) call, which would make even more sense. The HabState has a SaltyState, and one per algo type, that is a point-in-time snapshot of keystate, including kidx, at the time the Identifiers.get(name) call happens.

So, really, it isn't a bug to hold on to a reference to stale keystate. SignifyTS was doing exactly what I told it to do, using an old, stale view of keystate to look up a particular kidx. I'll do a double take on the bug report I filed after I finish my current end-to-end workflow task and determine if I should close the bug report. I'm leaning towards closing it.
This is definitely something I will add usage documentation about. People will definitely get tripped up on this as they learn to be disciplined in thinking about keystate and SignifyTS usage.
If there is a bug then it is in the fact that SignifyClient.identifiers.rotate() allows rotating with old keys when newer keys exist. We should probably throw an error and then add an override parameter force=true or something like that to force API consumers to be very aware and explicit about using old keys to sign events.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working triage Needs assessment
Projects
None yet
Development

No branches or pull requests

1 participant