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

Explicitly type RFQ private fields #202

Closed
wants to merge 1 commit into from
Closed

Explicitly type RFQ private fields #202

wants to merge 1 commit into from

Conversation

diehuxx
Copy link

@diehuxx diehuxx commented Dec 8, 2023

It's not clear from the documentation if certain fields that CAN be private MUST be private, nor is it explicitly laid out how to type these private fields. A thorough reading of the docs implies the type structure, but I'd rather be explicit.

After discussion at office hours, we've decided that an Offering specifies which RFQ fields are private.

@michaelneale
Copy link
Contributor

not a comment on the content, but I thoroughly agree with intent to tighten thing up if we can. "CAN" is always less desirable in a spec than other things.

@tomdaffurn
Copy link
Contributor

It was never clear to me how we'd parse the message when a field could be either a hash/string or a JSON object. It would be a 2-step approach, and probably look hacky in the code.

The downside of making private mandatory is that sometimes there won't be any sensitive data in the field, and it's complexity for no benefit. But the simplicity of having one approach seems worth that price.

#### `RfqPrivate`
| field | data type | required | description |
| --------------- | ------------------------------------------------- | -------- | ------------------------------------------------------------------------------------- |
| `claims` | string[] | Y | An array of claims that fulfill the requirements declared in an [Offering](#offering) |
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this be an array of PresentationSubmission objects?

Additionally, is it worth clarifying that the number of hashes in claims MUST be equal to the number of submission objects?

@mistermoe
Copy link
Member

@amika-sq and i discussed this a few weeks back and ended up thinking about whether an offering should state which fields should be private. Thoughts?

@mistermoe
Copy link
Member

notes from tbdex office hours:

  • decided that an offering should include which fields should be added to private when submitting an RFQ for said offering

@diehuxx
Copy link
Author

diehuxx commented Dec 21, 2023

Attempting to implement this and work out the kinks. There isn't a clear way in the Offering to denote which fields of the RFQ should be private. Here's a braindump of my thoughts so far.

Approach 1: Separate private-denoting fields in the Offering

The offering's PaymentMethod will still have JSON Schema to describe the shape of RFQ paymentDetails and a boolean denoting whether it goes in private or public

export type PaymentMethod = {
  /** The type of payment method. e.g. BITCOIN_ADDRESS, DEBIT_CARD etc */
  kind: string
  /** A JSON Schema containing the fields that need to be collected in the public `data.*.paymentDetails` section of the RFQ to use this payment method */
  requiredPaymentDetails: JsonSchema
  /**
    * If true, paymentDetails must be in the RFQ `private` field. `data.*.paymentDetails` must be a
    * string hash of the corresponding object in `private.*.paymentDetails`
    */
  privatePaymentDetails: boolean
}

The Offering will have a new requireClaimsPrivate field

export type OfferingData = {
...
  /** Articulates the claim(s) required when submitting an RFQ for this offering. */
  requiredClaims: PresentationDefinitionV2
  /**
   * If true, the RFQ must put the required claims in the `private.claims` field, with `data.claims` containing an array of hashes of the claims.
   * Otherwise, the RFQ must put the require claims in the public `data.claims` field.
   */
  requireClaimsPrivate: boolean
}

This results in an overall restriction that RFQ private field will look like so:

type RfqPrivate = {
    claims?: string[]
    payinMethod?: {
      paymentDetails?: Record<string, any>
    }
    payoutMethod?: {
      paymentDetails?: Record<string, any>
    }
  }

Approach 2: rfqPrivate

We add a rfqPrivate field to Offering.

type OfferingData = {
  /** The shape of the RFQ `private` section */
  rfqPrivate?: JsonSchema
}

This seems simpler, but I'm afraid it may be more difficult to validate an RFQ against. E.g. Offering.pay*Methods contains an array of JSON schema to make validation easier. A single rfqPrivate field on the other hand may need to have a big JSON schema with several oneOf fields, which are difficult to read.

Approach 3: private is a superset of data

This isn't mutually exclusive with Approaches 1 and 2, but feels like a distinct enough idea to deserve its own section.

We restructure RFQ private like so:

  • private contains ALL data, not JUST private data.
  • data contains public data, without any of the hashes of private stuff. E.g. if there are claims in private, then data omits the claims field entirely.
  • We add a field to metadata called privateHash (help me pick a better name) which is a hash of private. This still gives us some integrity when the private field is stripped away.

@andresuribe87
Copy link
Contributor

This is starting to look similar to the API used for creating an SD-JWT by an issuer. I thought I might share that here hoping that it's insightful. SD-JWTs are created by issuers. An issuer starts from a regular JWT claimset, and chooses which claims within the claimset are meant to be "blinded". Pseudo-code roughly looks like below:

sdJwt = blind(regulatJwtClaims, claimsToBlind)

An actual implementation is available in https://github.com/TBD54566975/web5-kt/blob/c7ef78d28330212247d16170e60c303c8ea6bfdc/sdjwt/src/test/kotlin/web5/security/SdJwtTest.kt#L77 which uses a map to indicate which claims should be made private.

Another implementation is available in https://github.com/openwallet-foundation-labs/sd-jwt-python/tree/main?tab=readme-ov-file#input-data-user_claims which uses a yaml map with tags to indicate which claims should be made private.

So with the above in mind, I'd advocate for a version of approach 2, where the type of rfqPrivate is an object, instead of a schema. I think that could alleviate some of the concerns of having to do schema processing on top of schema processing.

Happy to share more info about sd-jwts, if you think that would be helpful!

@diehuxx
Copy link
Author

diehuxx commented Dec 22, 2023

So with the above in mind, I'd advocate for a version of approach 2, where the type of rfqPrivate is an object

I'm open to this, but can't picture what that object would look like. Can you elaborate?

Happy to share more info about sd-jwts, if you think that would be helpful!

Yes pls. In particular, what's the structure of blinded vs non-blinded claims? Is it similar to the data vs private distinction we have, where private info appears hashed in the public data?

@andresuribe87
Copy link
Contributor

So with the above in mind, I'd advocate for a version of approach 2, where the type of rfqPrivate is an object

I'm open to this, but can't picture what that object would look like. Can you elaborate?

An example below, based off of the example in the spec.

Offering

{
  "metadata": {
    "from": "did:ex:pfi",
    "kind": "offering",
    "id": "offering_01ha82y8d0fhstg95hhfjwmgxf",
    "createdAt": "2023-09-13T20:15:22.528Z"
  },
  "data": {
    "description": "Selling BTC for USD",
    "payinCurrency": {
      "currencyCode": "USD"
    },
    "payoutCurrency": {
      "currencyCode": "BTC",
      "maxSubunits": "99952611"
    },
    "payoutUnitsPerPayinUnit": "0.00003826",
    "payinMethods": [
      {
        "kind": "DEBIT_CARD",
        "requiredPaymentDetails": {
          "$schema": "http://json-schema.org/draft-07/schema",
          "type": "object",
          "properties": {
            "cardNumber": {
              "type": "string",
              "description": "The 16-digit debit card number",
              "minLength": 16,
              "maxLength": 16
            },
            "expiryDate": {
              "type": "string",
              "description": "The expiry date of the card in MM/YY format",
              "pattern": "^(0[1-9]|1[0-2])\\/([0-9]{2})$"
            },
            "cardHolderName": {
              "type": "string",
              "description": "Name of the cardholder as it appears on the card"
            },
            "cvv": {
              "type": "string",
              "description": "The 3-digit CVV code",
              "minLength": 3,
              "maxLength": 3
            }
          },
          "required": [
            "cardNumber",
            "expiryDate",
            "cardHolderName",
            "cvv"
          ],
          "additionalProperties": false
        }
      }
    ],
    "payoutMethods": [
      {
        "kind": "BTC_ADDRESS",
        "requiredPaymentDetails": {
          "$schema": "http://json-schema.org/draft-07/schema",
          "type": "object",
          "properties": {
            "btcAddress": {
              "type": "string",
              "description": "your Bitcoin wallet address"
            }
          },
          "required": [
            "btcAddress"
          ],
          "additionalProperties": false
        }
      }
    ],
    "requiredClaims": {
      "id": "7ce4004c-3c38-4853-968b-e411bafcd945",
      "input_descriptors": [
        {
          "id": "bbdb9b7c-5754-4f46-b63b-590bada959e0",
          "constraints": {
            "fields": [
              {
                "path": [
                  "$.type"
                ],
                "filter": {
                  "type": "string",
                  "const": "YoloCredential"
                }
              }
            ]
          }
        }
      ]
    },
    "rfqPrivate": {
      "payinMethod":{
        "paymentDetails": {}
      }
    }
  },
  "signature": "eyJhbGciOiJFZERTQSIsImtpZCI6ImRpZDprZXk6ejZNa2syc1QyZUtvQWdUUTdzWjY3YTdmRDMzR21jYzZ1UXdaYmlxeWF5Rk1hYkhHI3o2TWtrMnNUMmVLb0FnVFE3c1o2N2E3ZkQzM0dtY2M2dVF3WmJpcXlheUZNYWJIRyJ9..9EBTL3VcajsQzSNOm8GElhcwvYcFGaRp24FTwmC845RCF84Md-ZB-CxdCo7kEjzsAY8OaB55XFSH_8K9vedhAw"
}

Giving an emphasis on the rfqPrivate field below for clarity:

    "rfqPrivate": {
      "payinMethod":{
        "paymentDetails": {}
      }
    }

Such an example is instructing the client to create a hash for the Rfq.data.payinMethod.paymentDetails object, and put the actual values inside the Rfq.private.payinMethod.paymentDetails field.

In order to validate the Rfq, the PFI would do the steps:

  1. Merge the private and data objects into a single object, by using the Offering.data.rfqPrivate object as a guide to replace the hashes in Rfq.data by their equivalent in the Rfq.private object.
  2. Run json schema validation that the resulting object from 1 satisfies the schema specified in Offering.data.payinMethods[kind == Rfq.data.payinMethod.kind].requiredPaymentDetails

Happy to share more info about sd-jwts, if you think that would be helpful!

Yes pls. In particular, what's the structure of blinded vs non-blinded claims?

It's best to illustrate this with an example SD-JWT (adapted from https://www.ietf.org/archive/id/draft-ietf-oauth-selective-disclosure-jwt-05.html#section-5.5):

Let's say you have the following set of non-blinded claims:

{
  "sub": "user_42",
  "given_name": "John",
  "family_name": "Doe",
  "email": "johndoe@example.com",
  "phone_number": "+1-202-555-0101",
  "phone_number_verified": true,
  "address": {
    "street_address": "123 Main St",
    "locality": "Anytown",
    "region": "Anystate",
    "country": "US"
  },
  "birthdate": "1940-01-01",
  "updated_at": 1570000000,
  "nationalities": [
    "US",
    "DE"
  ]
}

Then, the issuer (or whomever is going to sign the payload) decides to blind (i.e. to make private), among other things, the "address" claim. The resulting "blinded" payload would then be the following:

{
  "_sd": [
    "CrQe7S5kqBAHt-nMYXgc6bdt2SH5aTY1sU_M-PgkjPI",
    "JzYjH4svliH0R3PyEMfeZu6Jt69u5qehZo7F7EPYlSE",
    "PorFbpKuVu6xymJagvkFsFXAbRoc2JGlAUA2BA4o7cI",
    "TGf4oLbgwd5JQaHyKVQZU9UdGE0w5rtDsrZzfUaomLo",
    "XQ_3kPKt1XyX7KANkqVR6yZ2Va5NrPIvPYbyMvRKBMM",
    "XzFrzwscM6Gn6CJDc6vVK8BkMnfG8vOSKfpPIZdAfdE",
    "gbOsI4Edq2x2Kw-w5wPEzakob9hV1cRD0ATN3oQL9JM",
    "jsu9yVulwQQlhFlM_3JlzMaSFzglhQG0DpfayQwLUK4"
  ],
  "iss": "https://example.com/issuer",
  "iat": 1683000000,
  "exp": 1883000000,
  "sub": "user_42",
  "nationalities": [
    {
      "...": "pFndjkZ_VCzmyTa6UjlZo3dh-ko8aIKQc9DlGzhaVYo"
    },
    {
      "...": "7Cf6JkPudry3lcbwHgeZ8khAv1U1OSlerP0VkBJrWZ0"
    }
  ],
  "_sd_alg": "sha-256"
}

And when communicating the above "blinded" payload, a user also includes what's called a "disclosure". In particular, one for "address". The disclosure itself contains the private data. So in this case, the base64url decoded payload of the "address" disclosure contains the following:

["AJx-095VPrpTtN4QMOqROA", "address", {"street_address":
"123 Main St", "locality": "Anytown", "region": "Anystate",
"country": "US"}]

Is it similar to the data vs private distinction we have, where private info appears hashed in the public data?

Yes, it's similar, but not exactly the same. SD-JWT has stronger privacy guarantees to prevent correlating data. This is why there is a salt included in the disclosures, and why the "public" piece contains "decoy digests" (equivalent to our hashes).

The other similarity I see is that the issuer needs to programmatically communicate which fields they want to be made disclosable (i.e. which ones will be private).

I've attempted my best to communicate the important pieces of the SD-JWT design without all the details, and I may have failed... feel free to reach out with more questions. We can also set up some time to discuss during office hours!

@diehuxx
Copy link
Author

diehuxx commented Jan 29, 2024

Closing in favor of #232

@diehuxx diehuxx closed this Jan 29, 2024
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 this pull request may close these issues.

5 participants