TBD54566975 / tbdex

56 stars 25 forks source link

#274 Create salted hashes of privateData in Rfq #294

Closed diehuxx closed 5 months ago

diehuxx commented 5 months ago

Context

274

Previously, we considered the concept of a top-level unstructured private property in RFQ, which would optionally contain the private data specified by the Offering, where corresponding Rfq.data would be hashed. This PR lays out a simplified design compared to what we had before.

Motivation

Often times, an RFQ will contain PII or PCI data either within the claims being presented or within paymentDetails of payin or payout (e.g. card details, phone numbers, full names etc).

It is useful to be able to detach private fields from the RFQ payload and still be able to verify message integrity. So we create a separate section for private data.

Q&A

  1. Why call it privateData when we used to call it private? Since private is a reserved word in most programming languages, it was awkward to call the field private in some places and have to think of another name in other places.

  2. Why not use SD-JWT? SD-JWT is more complex than our use case requires. There are only a few fields that we need to selectively disclose in an RFQ. The design laid out in this PR takes some ideas from SD-JWT without needing to implement the rest of the kitchen sink.

Test vectors

This PR updates the existing parse-rfq test vectors because the new design constitutes a breaking change to RFQ message structure. This PR also adds a net new test vector related to RFQs called parse-rfq-omit-private-data.

TODO: Support updated test vectors in at least two tbdex implementations

mistermoe commented 5 months ago

we should move the privateData section under Rfq bguz longer applies to all message types.

mistermoe commented 5 months ago

@diehuxx just opened a PR that includes a handful of recommended extensions that make writing markdown subjectively easier (e.g. auto formatting tables, managing table of contents for you) https://github.com/TBD54566975/tbdex/pull/295. can you use Markdown All in One and Markdown Table to fix the ToC and table formatting?

mistermoe commented 5 months ago

what's the rationale behind claimsHashes over just claimsHash?

diehuxx commented 5 months ago

what's the rationale behind claimsHashes over just claimsHash?

I was thinking that in subsequent minor versions, we could add more nuance in validating each of the claims hashes to allow for a subset claims to be disclosed in privateData. I'm not married to this idea, since I don't have a concrete use case in mind that requires it. If you think it's not worth the added complexity, I'm open to changing it.

mistermoe commented 5 months ago

@diehuxx i think we'll probably want to lean on sd-jwt as much as possible wrt selective disclosure thangs. i think we should remove it.