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

Import compressed secp256k1 public key #2696

Closed
wemeetagain opened this issue Sep 11, 2024 · 2 comments · Fixed by #2697
Closed

Import compressed secp256k1 public key #2696

wemeetagain opened this issue Sep 11, 2024 · 2 comments · Fixed by #2697
Labels
need/triage Needs initial labeling and prioritization

Comments

@wemeetagain
Copy link
Member

A compressed secp256k1 public key is 33 bytes. I don't think 34 bytes is a thing anywhere.

const pubkeyRaw = Buffer.from('029b052e11390fec95d266e6d3208b7eaa9b92397b128915be02f1d0d54bfa2d02', 'hex');

But as it stands, the new @libp2p/crypto makes it very difficult turn this into a PublicKey instance.

import {publicKeyFromRaw} from '@libp2p/crypto/keys'
publicKeyFromRaw(pubkeyRaw) // blows up, expects secp256k1 pubkeys as 34 bytes

import {pb} from '@libp2p/crypto/keys' // library doesn't export protobuf
import {Secp256k1PublicKey} from '@libp2p/crypto/keys' // library doesn't export secp256k1 classes directly

leads to funny code

const magicPrefix = Buffer.from('08021221', 'hex'); // the first bytes of the pubkey protobuf

import {publicKeyFromProtobuf} from '@libp2p/crypto/keys'
const pubkey = publicKeyFromProtobuf(Buffer.concat([magicPrefix, pubkeyRaw]) // woohoo
@wemeetagain wemeetagain added the need/triage Needs initial labeling and prioritization label Sep 11, 2024
wemeetagain added a commit that referenced this issue Sep 11, 2024
@wemeetagain
Copy link
Member Author

Thinking more about it...

Its a little funky that we rely on the loose rule of byte length in *FromRaw fns. Would expect them to take a type param and look something like:

export function publicKeyFromRaw (type: KeyType, buf: Uint8Array): PublicKey {
  if (type === "Ed25519") {
    return unmarshalEd25519PublicKey(buf)
  } else if (type === "secp256k1") {
    return unmarshalSecp256k1PublicKey(buf)
  } else if (type === "RSA") {
    return pkixToRSAPublicKey(buf)
  }
  throw new UnsupportedKeyTypeError()
}

@achingbrain
Copy link
Member

achingbrain commented Sep 12, 2024

It does make me slightly nervous that we infer the key type by the length, all it would take is a new key type with the same length as one of the others to throw a spanner in the works.

An optional hint might be the way to go, we can overload it to specify the return type too:

export function publicKeyFromRaw (buf: Uint8Array, type: 'Ed25519'): Ed25519PublicKey
export function publicKeyFromRaw (buf: Uint8Array, type: 'secp256k1'): Secp256k1PublicKey
export function publicKeyFromRaw (buf: Uint8Array, type: 'RSA'): RSAPublicKey
export function publicKeyFromRaw (buf: Uint8Array): PublicKey
export function publicKeyFromRaw (buf: Uint8Array, type?: KeyType): PublicKey {
  //... implementation here
}

export function privateKeyFromRaw (buf: Uint8Array, type: 'Ed25519'): Ed25519PrivateKey
export function privateFromRaw (buf: Uint8Array, type: 'secp256k1'): Secp256k1PrivateKey
export function privateFromRaw (buf: Uint8Array, type: 'RSA'): RSAPrivateKey
export function privateFromRaw (buf: Uint8Array): PrivateKey
export function privateFromRaw (buf: Uint8Array, type?: KeyType): PrivateKey {
  //... implementation here
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
need/triage Needs initial labeling and prioritization
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants