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

Issues with default Salty key generation in case of loss of KERIA #300

Open
iFergal opened this issue Jan 6, 2025 · 6 comments
Open

Issues with default Salty key generation in case of loss of KERIA #300

iFergal opened this issue Jan 6, 2025 · 6 comments
Labels
bug Something isn't working triage Needs assessment

Comments

@iFergal
Copy link
Contributor

iFergal commented Jan 6, 2025

Version

Latest main

Environment

No response

Expected behavior

In case of a malicious/lost KERIA instance, it would be good to be able to recover identifiers from just the bran/passcode.

Actual behavior

Recovery in case of KERIA DB loss:
Salty identifiers in Signify by default will generate a random salt for each new prefix incepted, and store this encrypted on KERIA. It's encrypted by a key generated from the bran/passcode passed to the SignifyClient.

identifiers().create() accepts a bran argument, so you could pass the same bran but you can't create more than one identifier because the path will be the same currently.pidx is the prefix identifier and is bumped every time you create an identifier. However, it's not used in the path.

            const path =
                this.stem == ''
                    ? pidx.toString(16)
                    : this.stem + ridx.toString(16) + (kidx + idx).toString(16);

Each managed identifier uses the same stem - signify:aid so pidx isn't used.

In general, I'm curious why by default we use separate salts for each identifier instead of a different path on the same bran. Perhaps I'm missing some security issue with using the same bran.

--

Furthermore, another issue with the path is there is no delimiter. e.g. signify:aid111 could be (signify:aid, 11, 1) or (signify:aid, 1, 11).

Steps to reproduce

Code inspection and debug logs

@iFergal iFergal added bug Something isn't working triage Needs assessment labels Jan 6, 2025
@iFergal
Copy link
Contributor Author

iFergal commented Jan 8, 2025

From dev call yesterday; main motivation for this design was so that passcode rotation on Signify doesn't mean every identifier needs to be rotated. So you should locally backup the encrypted salts too.

Will think about this a bit more.

@daidoji
Copy link
Contributor

daidoji commented Jan 9, 2025

If the bran isn't used for AID inception, is it used for key generation? I haven't been into that part of the code on keripy for a while but I thought it was used in the kdf for key generation there.

Also, I'm curious about

Furthermore, another issue with the path is there is no delimiter. e.g. signify:aid111 could be (signify:aid, 11, 1) or (signify:aid, 1, 11)

That seems like an issue to fix before we get lots of AIDs.

@iFergal
Copy link
Contributor Author

iFergal commented Jan 9, 2025

If the bran isn't used for AID inception, is it used for key generation?

What do you mean by this? You need to generate keys for inception.

Using identifiers().create(); with defaults on Signify-TS will create a salt, which is encrypted by a key from the main bran. That salt will be used for all keys generated for that particular managed identifier.

So the bran is used to generate the AID used to communicate with KERIA (and nothing else), and encrypt these salts. Perhaps other things but not user facing.

That seems like an issue to fix before we get lots of AIDs.

Yeah, only tricky thing is handling migration of already existing identifiers since the keys would change based on the path.

I would also opt for using the pidx instead of the stem signify:aid so that a single bran can be used for multiple identifiers. I would prefer this over separate salts, particularly on mobile because you kind of need to back it up somewhere other than the device which could be lost/stolen. And it loses the post quantum security being sent around - though I see an issue to resolve that WebOfTrust/keripy#344.

@daidoji
Copy link
Contributor

daidoji commented Jan 9, 2025

Inception and rotation require that you provide a set of keys, not how those keys are derived. I went back and looked and I could only find Salty and Randy keys so maybe I'm misremembering, but I thought it was discussed somewhere that using a KDF for all key construction from the bran so that you could deterministically recreate history for some use cases. I guess its not implemented anywhere I can find easily though.

The security context would def be different (like the pathed salter scheme) but similar to what exists now.

I'm with you on the path thing. Is there an issue already tracking that?

@iFergal
Copy link
Contributor Author

iFergal commented Jan 9, 2025

Well yes, the protocol only cares about the keys. I'm talking about the Salty keys implementation here. Given the salt for an identifier, you can deterministically recreate history. It's just that by default you have the bran, and a salt for each identifier, instead of just the one bran.

However, you can do something like: await client.identifiers().create({ bran: <bran> }); to use the same bran for everything - you'll just hit the duplicate path issue as signify:aid is used as the stem of the path instead of pidx (prefix index).

But you could manually set the stem, so with a small change to CreateIdentifierArgs you could do await client.identifiers().create({ bran: <bran>, stem: client.pidx }); and it resolves the first issue. And it wouldn't break existing deployments.

--

But the other issue on the delimiter would be breaking. No issue to track it, I guess this is it. We ran out of time in the dev call this week for me to get fully into it.

@kentbull
Copy link
Collaborator

The following discussion can be used to capture a lot of the back and forth on solving this issue including planning backwards compatibility: WebOfTrust/keripy#927

The following issue has been opened up in KERIpy to track this issue progress in KERIpy: WebOfTrust/keripy#928

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

3 participants