hyperledger / aries-rfcs

Hyperledger Aries is infrastructure for blockchain-rooted, peer-to-peer interactions
https://hyperledger.github.io/aries-rfcs/
Apache License 2.0
326 stars 217 forks source link

fix: example oob url encoding #849

Closed JamesKEbert closed 2 months ago

JamesKEbert commented 3 months ago

This is a fairly pedantic fix to the URL encoding of the example OOB invitation in the OOB RFC.

Given in the OOB Aries RFC 0434 it references RFC 4648 for Base64URL encoding, we should be using padding (as the invitation length is not implicitly known) and the trailing '=' should be percent-encoded as '%3D', as described here and here.

From RFC 4648:

The pad character "=" is typically percent-encoded when used in an URI [9], but if the data length is known implicitly, this can be avoided by skipping the padding; see section 3.2.

Is additional language in RFC 0434 describing padding and formatting of the URI beneficial?

TimoGlastra commented 3 months ago

Does this reflect what is being done in pratice? Otherwise it might be beneficial to use unpadded (I think Credo uses unpadded when doing base64 url by default)

swcurran commented 3 months ago

I believe the guidance that we’ve given in the past is that the sender should NOT add padding, but the receiver should accept and process with and without padding (e.g., assuming the sender doesn’t follow rules). That last part is in the RFC.

That said, I’m not sure how that impacts this PR.

JamesKEbert commented 3 months ago

The default behavior for ACA-Py is for the sender to use padding (but not percent encoded), I spun up a basic agent and used essentially the example api request (note the trailing '='): https://present-bengal-logical.ngrok-free.app?oob=eyJAdHlwZSI6ICJkaWQ6c292OkJ6Q2JzTlloTXJqSGlxWkRUVUFTSGc7c3BlYy9vdXQtb2YtYmFuZC8xLjEvaW52aXRhdGlvbiIsICJAaWQiOiAiZTM5ODg0Y2YtMGYwMS00YTI5LTliNzUtOWJmMzg5MDIzNjgzIiwgInNlcnZpY2VzIjogW3siaWQiOiAiI2lubGluZSIsICJ0eXBlIjogImRpZC1jb21tdW5pY2F0aW9uIiwgInJlY2lwaWVudEtleXMiOiBbImRpZDprZXk6ejZNa3RmdkNqOFZ1Wnl3Nm5pbmhRUFpxUUtGM3dwUno5eGU3Yld1NEp1bTVYdEo2Il0sICJzZXJ2aWNlRW5kcG9pbnQiOiAiaHR0cHM6Ly9wcmVzZW50LWJlbmdhbC1sb2dpY2FsLm5ncm9rLWZyZWUuYXBwIn1dLCAiYWNjZXB0IjogWyJkaWRjb21tL2FpcDEiLCAiZGlkY29tbS9haXAyO2Vudj1yZmMxOSJdLCAiaGFuZHNoYWtlX3Byb3RvY29scyI6IFsiZGlkOnNvdjpCekNic05ZaE1yakhpcVpEVFVBU0hnO3NwZWMvZGlkZXhjaGFuZ2UvMS4wIl0sICJsYWJlbCI6ICJJbnZpdGF0aW9uIHRvIEJhcnJ5In0=

Credo is not using padding, as it is manually removing it after doing its base64 conversion: return base64.replace(/\+/g, '-').replace(/\//g, '_').replace(/=/g, '') https://github.com/openwallet-foundation/credo-ts/blob/main/packages/core/src/utils/base64.ts

Default Credo toUrl() URL (note the absent trailing '=' or '%3D'): https://example.com/ssi?oob=eyJAdHlwZSI6Imh0dHBzOi8vZGlkY29tbS5vcmcvb3V0LW9mLWJhbmQvMS4xL2ludml0YXRpb24iLCJzZXJ2aWNlcyI6WyJkaWQ6c292OkxqZ3BTVDJyanNveFllZ1FEUm03RUwiXSwiQGlkIjoiNjkyMTJhM2EtZDA2OC00ZjlkLWEyZGQtNDc0MWJjYTg5YWYzIiwibGFiZWwiOiJGYWJlciBDb2xsZWdlIiwiZ29hbF9jb2RlIjoiaXNzdWUtdmMiLCJnb2FsIjoiVG8gaXNzdWUgYSBGYWJlciBDb2xsZWdlIEdyYWR1YXRlIGNyZWRlbnRpYWwiLCJoYW5kc2hha2VfcHJvdG9jb2xzIjpbImh0dHBzOi8vZGlkY29tbS5vcmcvZGlkZXhjaGFuZ2UvMS4wIiwiaHR0cHM6Ly9kaWRjb21tLm9yZy9jb25uZWN0aW9ucy8xLjAiXX0

I believe the guidance that we’ve given in the past is that the sender should NOT add padding, but the receiver should accept and process with and without padding (e.g., assuming the sender doesn’t follow rules). That last part is in the RFC.

That said, I’m not sure how that impacts this PR.

I'll update this PR as per that guidance :+1:

swcurran commented 3 months ago

@TelegramSam — can you add this to the agenda for the next Aries Working Group Meeting? Or @JamesKEbert — add it to the agenda when the call for topics is made at the start of the call?

Thanks!

swcurran commented 2 months ago

ACA-Py to be fixed to remove sending a padding character.