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

Implement a single type to provide different forms of encryption keys #4273

Open
igor-sirotin opened this issue Nov 7, 2023 · 3 comments
Open

Comments

@igor-sirotin
Copy link
Collaborator

igor-sirotin commented Nov 7, 2023

Problem

NOTE: It might be that I'm just stupid, but it seems to me from the codebase that we do have a lot of confusion about the topic.

This is all the same public key:

Description Used for Value
ecdsa.PublicKey curve = secp256k1.S256()
X = 65048851631837384047378089819319437426496905877495594931948125317595069084841
Y = 54855369826654862910415609527800597498500226949009120841462260707132436223130
compressed bytes protobuf, database 02 8f d0 58 65 ad fd 5d f7 00 be f8 16 09 a5 ed 38 89 ee 4c 82 c4 5a 74 b1 05 ca ca 8c 3a 62 f4 a9
uncompressed bytes 04 8f d0 58 65 ad fd 5d f7 00 be f8 16 09 a5 ed 38 89 ee 4c 82 c4 5a 74 b1 05 ca ca 8c 3a 62 f4 a9 79 47 09 ff 97 b5 8e 0d 38 4a 11 90 57 b7 f5 b4 70 a2 df 9f 13 37 ed 54 73 c1 58 fc c5 5d 94 9a
compressed,
hex-encoded
0x028fd05865adfd5df700bef81609a5ed3889ee4c82c45a74b105caca8c3a62f4a9
uncompressed,
hex-encoded
maps indexing 0x048fd05865adfd5df700bef81609a5ed3889ee4c82c45a74b105caca8c3a62f4a9794709ff97b58e0d384a119057b7f5b470a2df9f1337ed5473c158fcc55d949a
"serialized" UI and shared URLs zQ3shX6BKTi1CpWcqEiQLTLRmvGSzktyg14eMcfjYZfHzFzVE
"deserialized" fe701048fd05865adfd5df700bef81609a5ed3889ee4c82c45a74b105caca8c3a62f4a9794709ff97b58e0d384a119057b7f5b470a2df9f1337ed5473c158fcc55d949a

Proposals

  1. Make a strict terminology:
    • COMPRESSED should only be used for "using the X coordinate of ECDSA Public Key".
      It has (almost) nothing to do with zQ3sh... base58-encoding.
      E.g. in status-desktop we have decompressPk function that wraps deserializePublicKey call.
    • SERIALIZED == base58-encoded multibase-identified uncompressed public key
    • DESERIALIZED == hex-encoded multibase-identified compressed public key
  2. Make the code independent of the passed form
    • Draw strict rules for where and which variation of key should be used
  3. Consider migrating the database
    communities_communities table use "33-bytes compressed public key" as BLOB type,
    while contacts table use "full uncompressed public key" as STRING.
  4. Remove any code duplications
  • 0x0701
  • z, f encoding identifiers
  1. Remove manual prepending/stripping 0x.
    Though we have special types.EncodeHex and typesDecodeHex` functions, there're still so many places that do this manually. And actually, should we even use this form of the public key? Even if we don't want to migrate the database, we could convert it to smth right after scanning the database.
  2. Create a single module with all required features.
    One of the confusion reasons is that currently we use a bunch of modules for converting purposes:
    hex, types, utils, secp256k1, ecdsa, elliptic, crypto, gethcrypto, multiformat, multibase

Bad code references

  1. status-desktop: decompressPk
    https://github.com/status-im/status-desktop/blob/94953bb925a3a3cbac1508eb6baf7aec053eeae3/src/backend/accounts.nim#L148-L163

Problems here:
- at first check it the input is not uncompressed already, this seems correct
- then mistake: "decompress" wraps "deserialize" procedure and actually return it as result
- and in case of JsonParsingError it simply does the things manually in nim 😄

proc decompressPk*(publicKey: string): RpcResponse[string] =
  discard
  if publicKey.startsWith("0x04") and publicKey.len == PK_LENGTH_0X_INCLUDED:
    # already decompressed
    result.result = publicKey
    return

  var response = status_go.multiformatDeserializePublicKey(publicKey, "f")
  # json response indicates error
  try:
    let jsonReponse = parseJson(response)
    result.error = RpcError(message: jsonReponse["error"].getStr())
  except JsonParsingError as e:
    let secp256k1Code = "fe701"
    response.removePrefix(secp256k1Code)
    result.result = "0x" & response
  1. SerializePublicKey and DeserializePublicKey custom wrappers:

This is a very good example of confusion and lack of convenient functions.
It's really not obvious how to convert zQ3sh... string to 33-bytes compressed bytes. So we end up with such functions in the codebase:

func DeserializePublicKey(compressedKey string) (types.HexBytes, error) {
rawKey, err := multiformat.DeserializePublicKey(compressedKey, "f")
if err != nil {
return nil, err
}
secp256k1Code := "fe701"
pubKeyBytes := "0x" + strings.TrimPrefix(rawKey, secp256k1Code)
pubKey, err := prot_common.HexToPubkey(pubKeyBytes)
if err != nil {
return nil, err
}
return crypto.CompressPubkey(pubKey), nil
}
func SerializePublicKey(compressedKey types.HexBytes) (string, error) {
rawKey, err := crypto.DecompressPubkey(compressedKey)
if err != nil {
return "", err
}
pubKey := types.EncodeHex(crypto.FromECDSAPub(rawKey))
secp256k1Code := "0xe701"
base58btc := "z"
multiCodecKey := secp256k1Code + strings.TrimPrefix(pubKey, "0x")
cpk, err := multiformat.SerializePublicKey(multiCodecKey, base58btc)
if err != nil {
return "", err
}
return cpk, nil
}

Implementation

I also came up with an idea that could help us.
This has it's pros and cons, it to be discussed ofc, but I'm sharing it here anyway.

The idea is to implement a "smart class" for keys.
We could pass it around within status-go and each consumer can take the key in whatever format is needed. We can also cache some popular values inside, e.g. compressed/uncompressed versions, z and f string encodings and ecdsa.PublicKey. And I suppose that with due implementation it can even be used for maps indexing.

protocol common

type PublicKey struct {
    // cache popular values
    raw                               ecdsa.PublicKey
    compressedBytes                   []byte
    decompressedHexEncoded            string
    compressedSerializedBase58Encoded string // a.k.a. "short string for UI"
}

// Smart functions that check all possible cases of input,
// considering different encodings, compressed/uncompressed, seialised/non-serialised, etc.
func ParseBytes(input []byte) PublicKey
func ParseString(input string) PublicKey

// Convert functions that can be controlled with arguments
func (pk *PublicKey) GetString(serialize bool, compress bool, encodeBase Encoding)
func (pk *PublicKey) GetShortString(serialize bool, compress bool) {
    return GetString(serialize, compress, multibase.Base58BTC)
}
func (pk *PublicKey) GetCompressedBytes() bytes

// Smart functions that know the format of `ContactID` and `CommunityID`  for different use-cases
func (pk *PublicKey) ToContactIdForDatabase() 
func (pk *PublicKey) ToContactIdForUI()
@igor-sirotin
Copy link
Collaborator Author

cc @Samyoul @cammellos

@Samyoul
Copy link
Member

Samyoul commented Nov 7, 2023

I think that this approach will encapsulate a solution for each use case we have for public keys.

@igor-sirotin
Copy link
Collaborator Author

just in case, here's the code I used to collect different formats:

func TestMessengerKeys(t *testing.T) {
	suite.Run(t, new(MessengerKeysTestSuite))
}

type MessengerKeysTestSuite struct {
	suite.Suite
}

func (s *MessengerKeysTestSuite) Test_1() {
	privateKey, err := gethcrypto.GenerateKey()
	s.Require().NoError(err)

	publicKey := privateKey.PublicKey

	// 1. X/Y
	fmt.Println(fmt.Sprintf("raw curve point: X = %s, Y = %s", publicKey.X.String(), publicKey.Y.String()))

	// 2. 33-bytes compressed
	compressed := crypto.CompressPubkey(&publicKey)
	fmt.Println("compressed bytes:", hex.EncodeToString(compressed))

	// 3. 64-bytes uncompressed
	uncompressed := elliptic.Marshal(secp256k1.S256(), publicKey.X, publicKey.Y)
	fmt.Println("uncompressed bytes", hex.EncodeToString(uncompressed))

	// 4. hex-encoded 64-bytes uncompressed
	hexEncodedCompressed := types.EncodeHex(compressed)
	fmt.Println("compressed hex-encoded", hexEncodedCompressed)

	// 5. hex-encoded 64-bytes uncompressed
	hexEncodedUncompressed := types.EncodeHex(uncompressed)
	fmt.Println("uncompressed hex-encoded", hexEncodedUncompressed)

	// 6. serialised: compressed + multibase-identified + base58-encoded
	serialized, err := multiformat.SerializeLegacyKey(hexEncodedUncompressed)
	s.Require().NoError(err)
	fmt.Println("serialized", serialized)

	// 5. deserialized: decompressed + multibase-identified + hex-encoded
	deserialized, err := multiformat.DeserializePublicKey(serialized, "f")
	s.Require().NoError(err)
	fmt.Println("deserialized", deserialized)
}

@igor-sirotin igor-sirotin changed the title Rationale: refactoring different forms of public keys Implement a single type to provide different forms of encryption keys Aug 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants