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

Tolerance to Base64URLSafe encoding padding (or lack thereof) in Connection Protocol Signatures #1053

Closed
gmulhearn-anonyome opened this issue Nov 7, 2023 · 0 comments · Fixed by #1083
Assignees

Comments

@gmulhearn-anonyome
Copy link
Contributor

During the Connection Response of Connection Protocol, a signature is attached within a connection~sig decorator/field. https://github.com/hyperledger/aries-rfcs/tree/main/features/0160-connection-protocol#2-connection-response .

  "connection~sig": {
    "@type": "https://didcomm.org/signature/1.0/ed25519Sha512_single",
    "signature": "<digital signature function output>",
    "sig_data": "<base64URL(64bit_integer_from_unix_epoch||connection_attribute)>",
    "signer": "<signing_verkey>"
  }

The spec describes it as so, but does not specify whether base64URL is with or without padding.

There is a note elsewhere on the page which says:

NOTE: In receiving the invitation, the base64url decode implementation used MUST correctly decode padded and unpadded base64URL encoded data.

This makes me believe that this sentiment would apply for sig_data too. As such, we may want to make our code handle both. Currently common/signing.rs is hardcoded to use URL_SAFE (expects padding). It has been found that the animo.demo.id DEMO (which presumably uses AFJ) can produce sig_data which does not have padding. This causes common/signing.rs to throw an InvalidPadding error.

payload from animo:

{"@type":"did:sov:BzCbsNYhMrjHiqZDTUASHg;spec/connections/1.0/response","@id":"5a557835-13cd-4cbf-b165-96242f138936","connection~sig":{"@type":"https://didcomm.org/signature/1.0/ed25519Sha512_single","sig_data":"AAABi6h0IMJ7IkRJRCI6IjJWMktCQUFGRlpiM2FlelVMTFRKWEUiLCJESUREb2MiOnsiQGNvbnRleHQiOiJodHRwczovL3czaWQub3JnL2RpZC92MSIsInB1YmxpY0tleSI6W3siaWQiOiIyVjJLQkFBRkZaYjNhZXpVTExUSlhFIzEiLCJjb250cm9sbGVyIjoiMlYyS0JBQUZGWmIzYWV6VUxMVEpYRSIsInR5cGUiOiJFZDI1NTE5VmVyaWZpY2F0aW9uS2V5MjAxOCIsInB1YmxpY0tleUJhc2U1OCI6Im90TTJwTHVVM0NTQkdvZ2JHNXAyRUtLZ0ZobU01ZWdQMkxLcU1RdnZEdlMifV0sInNlcnZpY2UiOlt7ImlkIjoiMlYyS0JBQUZGWmIzYWV6VUxMVEpYRSNJbmR5QWdlbnRTZXJ2aWNlIiwic2VydmljZUVuZHBvaW50IjoiaHR0cHM6Ly9kaWRjb21tLmRlbW8uYW5pbW8uaWQiLCJ0eXBlIjoiSW5keUFnZW50IiwicHJpb3JpdHkiOjAsInJlY2lwaWVudEtleXMiOlsib3RNMnBMdVUzQ1NCR29nYkc1cDJFS0tnRmhtTTVlZ1AyTEtxTVF2dkR2UyJdLCJyb3V0aW5nS2V5cyI6W119XSwiYXV0aGVudGljYXRpb24iOlt7InB1YmxpY0tleSI6IjJWMktCQUFGRlpiM2FlelVMTFRKWEUjMSIsInR5cGUiOiJFZDI1NTE5U2lnbmF0dXJlQXV0aGVudGljYXRpb24yMDE4In1dLCJpZCI6IjJWMktCQUFGRlpiM2FlelVMTFRKWEUifX0","signer":"otM2pLuU3CSBGogbG5p2EKKgFhmM5egP2LKqMQvvDvS","signature":"pctzlHij_A2a9RLBqMDQS7iz8J0fsIESSZWV9JW5tOuZcS6VcqN2visj2FjL6Lw7QelVNjY0jbALbbckh0y4Bw"},"~thread":{"thid":"151b33e9-0259-4d01-8b79-341b2a08b36a"}}

reproducable error:

general_purpose::URL_SAFE.decode("pctzlHij_A2a9RLBqMDQS7iz8J0fsIESSZWV9JW5tOuZcS6VcqN2visj2FjL6Lw7QelVNjY0jbALbbckh0y4Bw").unwrap();
@gmulhearn-anonyome gmulhearn-anonyome self-assigned this Nov 8, 2023
gmulhearn pushed a commit that referenced this issue Dec 19, 2023
* lli - Allow decoding both padded and unpadded Base64 data

Signed-off-by: lli <lli@anonyome.com>

* lli - Running cargo fmt

Signed-off-by: lli <lli@anonyome.com>

* lli - improve testing rigor

Signed-off-by: lli <lli@anonyome.com>

* lli - custom GeneralPurpose engine instance for indifferent decode

Signed-off-by: lli <lli@anonyome.com>

* lli - move custom engine to utils

Signed-off-by: lli <lli@anonyome.com>

---------

Signed-off-by: lli <lli@anonyome.com>
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 a pull request may close this issue.

1 participant