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

Add method for deriving an HMAC key from the private key. #221

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

cmdruid
Copy link

@cmdruid cmdruid commented Feb 3, 2023

I ran into an issue where I want to derive child-keys from the parent private key, but there is currently no way to do this with the current spec.

I propose adding a window.nostr.getDerivedKey(key: string): string method to the spec, which is a simple HMAC method using the private key and a user supplied key. HMAC has wide-spread support in the WebCrypto spec, and is easy to implement.

https://developer.mozilla.org/en-US/docs/Web/API/SubtleCrypto

Here is some reference code for performing a simple HMAC operation using WebCrypto API:

async function hmac (
  // Perform an HMAC signing operation.
  key   : Uint8Array,
  data : Uint8Array,
  fmt   : string = 'SHA-256'
) : Promise<Uint8Array> {
  const cryptoKey = await importKey(key, fmt)
  return crypto.subtle
    .sign('HMAC', cryptoKey, data)
    .then((buffer) => new Uint8Array(buffer))
}
async function importKey (
  // Create a CryptoKey from the 
  // supplied key and format string.
  key : Uint8Array,
  fmt : string = 'SHA-256'
) : Promise<CryptoKey> {
  const config = { name: 'HMAC', hash: fmt }
  return crypto.subtle.importKey(
    'raw', key, config, false, ['sign', 'verify']
  )
}

An example use-case would be generating key material, in order to create key-pairs that are derived from the master key in a deterministic way.

This would be useful to create sub-accounts, delegated accounts, or key-pairs used to create channels and group chats (where the key-pair is used to publish updates to the state of the group).

This proposal does not involve key-tweaking or HD-wallets, although you could easily create such a scheme using key material from an HMAC-512 operation to initiate the wallet.

I ran into an issue where I want to derive child-keys from the parent private key, but there is currently no way to do this with the current spec.

I propose adding a `window.nostr.getDerivedKey(key: string): string` method to the spec, which is a simple HMAC method using the private key and a user supplied key. HMAC has wide-spread support in the WebCrypto spec, and is easy to implement. 

https://developer.mozilla.org/en-US/docs/Web/API/SubtleCrypto

Here is some reference code for performing a simple HMAC operation using WebCrypto API:
```ts
async function hmac (
  // Perform an HMAC signing operation.
  key   : Uint8Array,
  data : Uint8Array,
  fmt   : string = 'SHA-256'
) : Promise<Uint8Array> {
  const cryptoKey = await importKey(key, fmt)
  return crypto.subtle
    .sign('HMAC', cryptoKey, data)
    .then((buffer) => new Uint8Array(buffer))
}
async function importKey (
  // Create a CryptoKey from the 
  // supplied key and format string.
  key : Uint8Array,
  fmt : string = 'SHA-256'
) : Promise<CryptoKey> {
  const config = { name: 'HMAC', hash: fmt }
  return crypto.subtle.importKey(
    'raw', key, config, false, ['sign', 'verify']
  )
}
```
@fiatjaf
Copy link
Member

fiatjaf commented Feb 6, 2023

I don't know if I like this approach. I would prefer something that used NIP-06, but that would complicate the extension. It is probably better to generate these keys externally and use an extension that supports multiple accounts within the same NIP-07 standard, like Alby is going to do now.

Or until we see a really compelling use case.

@gkbrk
Copy link

gkbrk commented Feb 6, 2023

NIP-07 extensions are good because if the extension doesn't sign an event for the app, the app cannot produce valid signatures itself. It also cannot read the private key. It can only ask the extension to do things.

Keys derived with this method would be available to the application with no way to retract access later. This doesn't fit too well with the rest of NIP-07, where every single action can be subject to user approval, and once the extension says "no", nothing can be done.

@gkbrk
Copy link

gkbrk commented Feb 6, 2023

Perhaps a decent way to make this HMAC key derivation work like the rest of the NIP-07 spec could be making the getDerivedKey method return a pubkey, and adding a signWithDerivedKey(event, context) -> signed event method to actually sign things with it.

@cmdruid
Copy link
Author

cmdruid commented Feb 6, 2023

Yes that will be the next step.

This addition is just for reliably generating key material for a custom protocol. It is not meant to custody those keys.

The next step would be key tweaking, which I will be adding to this soon.

@cmdruid
Copy link
Author

cmdruid commented Feb 7, 2023

Perhaps a decent way to make this HMAC key derivation work like the rest of the NIP-07 spec could be making the getDerivedKey method return a pubkey, and adding a signWithDerivedKey(event, context) -> signed event method to actually sign things with it.

Please see fiatjaf/nos2x#26

@fiatjaf
Copy link
Member

fiatjaf commented Feb 7, 2023

Instead of adding new methods this should just reuse the existing methods, but on the prompt popup the user can choose to use a new derived key.

Once a user uses a new key for a given domain, that key becomes associated with that domain.

This would be a hundred times better since everything can keep working and it's a user's choice now. What do you think?

I still don't see a compelling use case so I can't promise I would merge this on nos2x though.

@cmdruid
Copy link
Author

cmdruid commented Feb 7, 2023

Instead of adding new methods this should just reuse the existing methods, but on the prompt popup the user can choose to use a new derived key.

I agree that this would be a better method. I could update the exiting methods to take a second optional parameter (the tweak), and it would essentially be the same method, but with a derived key.

Once a user uses a new key for a given domain, that key becomes associated with that domain.

That is definitely a good use case, similar to LNURL auth. The key tweak could be prefixed with the domain.

I am building applications that need key derivation but do not have a domains though, so I need the option to submit a non-prefixed string as a tweak.

This would be a hundred times better since everything can keep working and it's a user's choice now. What do you think?

I think it is a great idea and an improvement over the current PR. If you agree, I will update the PR so that the existing methods have an optional tweak parameter.

I still don't see a compelling use case so I can't promise I would merge this on nos2x though.

The biggest use-case I need this for is pubkey-based channels. I need the user to derive key pairs for the channels, so the user can sign and update the channel profile.

If this goes beyond nos2x then I can fork and make an extended version. But I think key derivation is essential enough that it should be a part of the windows.nostr spec in some form.

Thank you for your work and feedback on this, I greatly appreciate it!

@cmdruid
Copy link
Author

cmdruid commented Feb 20, 2023

I have realized that there is currently no easy way to check if an existing method supports a new parameter, so that I can fall-back to a polyfill if needed.

To avoid this problem, it may work out better if new features are wrapped within their own method, instead of extending an existing one.

However it has made me realize that this NIP currently has no spec for version control, which is not good.

I think that key tweaking is an obvious need. Any spec that makes use of multiple key-pairs for an identity needs to be able to sign and verify with a tweaked key.

I'll leave this PR in place for now, but it may make more sense to provide key tweaking as separate methods, and figure out the version control issue.

I have went ahead and created a separate issue for that discussion here:
#279

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants