aries-cloudagent-python icon indicating copy to clipboard operation
aries-cloudagent-python copied to clipboard

Verkey validation only supports ed25519 key type

Open ff137 opened this issue 2 years ago • 7 comments

Working with the aries-cloudcontroller (which is a client generated from the ACA-Py openapi spec), creating a DID with Bbs bls12381g2 key type, yields a 422 unprocessable entity, due to the verkey not passing the regex defined in the DIDSchema:

class DIDSchema(OpenAPISchema):
    verkey = fields.Str(
        validate=INDY_RAW_PUBLIC_KEY_VALIDATE,
...

INDY_RAW_PUBLIC_KEY_VALIDATE = IndyRawPublicKey()

class IndyRawPublicKey(Regexp):
    PATTERN = rf"^[{B58}]{{43,44}}$"

so, INDY_RAW_PUBLIC_KEY_VALIDATE is requiring it to be 43/44 base58 characters. This is only true for ed25519 key type.

Creating a DID with the Bbs key type does indeed work through ACA-Py, but doing so through an openapi generated client, will fail due to the verkey not passing validation defined for the schema.

We patched it in cloudcontroller just by removing the validation, but naturally we'd want to add it with support for Bbs. Verkey validation is happening in a few other schemas too, so they'll all need to be updated.

I can't find documentation re what the regex pattern should be for a Bbs generated verkey ... it seems to be 132 base58 characters, so it might be as simple as changing the {43,44} to {43,44,132}. But of course I just wanna make sure of that before patching it.

Are these requirements (re Bbs key type support) on the radar yet? And does anyone know what the Bbs verkey pattern should be?

ff137 avatar Aug 21 '23 16:08 ff137

@andrewwhitehead — can you please answer the BBS+ related questions above?

swcurran avatar Aug 21 '23 17:08 swcurran

I looked at some repos for how BBS signatures are implemented -- mattrglobal/bbs-signatures and jsonld-signatures-bbs -- afaict, the BBS verkey is base64 with at least 128 characters.

There's a table in the bbs-signatures README which says the public key is 96 bytes. GPT explains to me that 96 bytes will always translate to 128 base64 characters, because "The base64 encoding scheme takes 3 bytes of binary data and turns them into 4 characters of ASCII text".

It also mentions the binary data will be padded if it's not a multiple of 3 bytes ... so if the input had 1-3 bytes of extra information, it would yield a 132 character public key instead. And, that's precisely what I got from my initial testing, calling the create-did route with BBS signature type.

So, I really have no idea -- is it meant to always be 128? Or is it sometimes 132?

If I had to take a shot in the dark, maybe, due to the way default python json.dumps includes spaces between keys and values, somehow that's adding an extra byte to a payload somewhere, giving the BBS verkey to be 132 base64 characters instead of 128? Idk I'm probably wrong, but hopefully spurs some discussion! @andrewwhitehead or anyone else who knows the inner workings?

ff137 avatar Aug 28 '23 18:08 ff137

@ff137 A raw BBS (BLS12381 G2) public key should indeed be 96 bytes. It looks like you might be decoding the base58-encoded key as base64, which does return a 98 byte value. Otherwise, in some contexts (like did:key) the key becomes longer as it has a multicodec prefix added.

andrewwhitehead avatar Aug 28 '23 19:08 andrewwhitehead

@andrewwhitehead Thanks! But, how do you mean I'm "decoding the base58-encoded key as base64"?

The response we're getting from the wallet_create_did route, requesting did:key + BBS, returns a 132 character string. i.e.:

POST /wallet/did/create

{
  "method": "key",
  "options": {
    "key_type": "bls12381g2"
  }
}

Response:

{
  "result": {
    "did": "did:key:zUC73B9FzXEUrM8fhsB2s8r6X2jQhWW1uweXp2FABrWEvj8nxdWqf3d4yQJG7SfhXB8tWP7mpZ3apqJ1GfeV7HxVnjMfjaDjnUZ5oSb63AtiyUvZBha5gyaahJ2c4FVy24R8zL5",
    "verkey": "oLbNvqaFVjGTHvWBhC9tDjNVTEBosy6bwdfLi13SBqmzZ2ZCxPDvAxLaxTKSzsbFNAhtn9BmSwk3KnvMx2cmxmpwwEKdpqRbXnth8xFrVXngpoEUc3QUXVcdb4m5SWGGVim",
    "posture": "wallet_only",
    "key_type": "bls12381g2",
    "method": "key"
  }
}

So, correction: the verkey len is most often 131 characters:

>>> len("oLbNvqaFVjGTHvWBhC9tDjNVTEBosy6bwdfLi13SBqmzZ2ZCxPDvAxLaxTKSzsbFNAhtn9BmSwk3KnvMx2cmxmpwwEKdpqRbXnth8xFrVXngpoEUc3QUXVcdb4m5SWGGVim")
131

If I test multiple times, it's most often 131, and sometimes 132 characters.

The question here is in line with how the validation should be updated, as currently the openapi schema validates that the verkey is 43/44 base58 characters. I'm guessing that can be updated to include 131/132 base64 characters?

If the BBS public key is 96 bytes, which equals 128 base64 characters, doesn't it suggest something is off if it's returning 131/132 characters? Or, is that explained by (as you mentioned) the multicodec prefix being added?

ff137 avatar Aug 28 '23 19:08 ff137

No, I think you're just confusing base58 and base64:

>>> len(base58.b58decode("oLbNvqaFVjGTHvWBhC9tDjNVTEBosy6bwdfLi13SBqmzZ2ZCxPDvAxLaxTKSzsbFNAhtn9BmSwk3KnvMx2cmxmpwwEKdpqRbXnth8xFrVXngpoEUc3QUXVcdb4m5SWGGVim"))
96

andrewwhitehead avatar Sep 05 '23 22:09 andrewwhitehead

Yes, I indeed confused base58 and base64, because until now I wasn't told that the response is always in base58 :-)

I could only tell that the indy verkey was base58. In my message before last: "afaict [as far as I can tell], the BBS verkey is base64 with at least 128 characters." There I was incorrect. To which you said: "you might be decoding the base58-encoded key as base64", which indeed confused me, because I wasn't doing any decoding. I was just getting the response, and trying to infer what the correct regex validation should be.

So, to wrap things up: the DIDSchema validation:

class DIDSchema(OpenAPISchema):
    verkey = fields.Str(
        validate=INDY_RAW_PUBLIC_KEY_VALIDATE,
...
    PATTERN = rf"^[{B58}]{{43,44}}$"

That needs to be updated to support the did:key pattern, which, afaict :-), means accepting 131/132 base58 characters.

ff137 avatar Sep 06 '23 07:09 ff137

There are many models with fields that require the INDY_RAW_PUBLIC_KEY_VALIDATE pattern. I've started with adding support for did:key + BBS in the following PR: #2470 Please review to see the models affected, and please suggest changes or additional consideration.

ff137 avatar Sep 06 '23 07:09 ff137