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

synchronize BIP 77 with code #458

Open
7 of 11 tasks
nothingmuch opened this issue Jan 3, 2025 · 11 comments
Open
7 of 11 tasks

synchronize BIP 77 with code #458

nothingmuch opened this issue Jan 3, 2025 · 11 comments
Milestone

Comments

@nothingmuch
Copy link
Collaborator

nothingmuch commented Jan 3, 2025

The BIP 77 spec is not fully up to date with these changes:

  • short IDs
  • uppercase bech32 fragment parameters
  • RK1 fragment parameter
  • ellswift encoding in DHKEM, bit uniformity of payload
  • ensure DHKEM is sufficiently documented in BIP 77 in an unambiguous spec, since it's a draft RFC now
  • padding rules for inner and outer messages
  • incl. pub const INFO_A: &[u8; 8] = b"PjV2MsgA";, pub const INFO_B: &[u8; 8] = b"PjV2MsgB";

Additionally the document can be improved:

  • UI/UX and privacy tradeoffs of relay and directory choices
  • diagrams for byte representations of the payloads
  • ascii sequence diagram to plantuml?
  • Describe bip21 as a potential bootstrap mechanism where TLS is unavailable
@DanGould
Copy link
Contributor

DanGould commented Jan 6, 2025

I started pushing these changes to a secondary branch so I don't interrupt the reviewers until we're ready for a review. FYI

Note that Short IDs likely also need to be uppercase since they're parsed as having a case-sensitive bech32 HRP in our code, too

@nothingmuch
Copy link
Collaborator Author

Note that Short IDs likely also need to be uppercase since they're parsed as having a case-sensitive bech32 HRP in our code, too

Yes, with the same rationale as fragment parameter, although the # character is not in the QR alphanumeric set, since it's % escaped according to RFC 3986 reccomendation to make the hex encoding uppercase, the run of alphanumeric QR code characters potentially starts at the beginning of the entire parameter, and lowercase shortid would break that

@DanGould
Copy link
Contributor

ensure DHKEM is sufficiently documented in BIP 77 in an unambiguous spec, since it's a draft RFC now

77 links to the draft RFC. What more would make it sufficiently unambiguous?

====Secp256k1-based DHKEM====

[[https://www.ietf.org/archive/id/draft-wahby-cfrg-hpke-kem-secp256k1-01.html| Secp256k1-based DHKEM for HPKE]] is most appropriate because of secp256k1's availability in bitcoin contexts.

@DanGould
Copy link
Contributor

Pushed changes to the secondary branch

@DanGould
Copy link
Contributor

#480 must be addressed for our implementation to be in sync with spec, which BIP 77 relies on, as must all existing Payjoin implementations

@DanGould
Copy link
Contributor

Was it a mistake to use different INFO strings for HPKE?

All the algorithms also take an info parameter that can be used to influence the generation of keys (e.g., to fold in identity information) and an aad parameter that provides additional authenticated data to the AEAD algorithm in use.

The default info value is "", did we make a mistake by using something non-default? Further, was it a mistake to use TWO separate info strings pub const INFO_A: &[u8; 8] = b"PjV2MsgA";, pub const INFO_B: &[u8; 8] = b"PjV2MsgB";

According to RFC 9180, the info parameter serves as application-supplied information that can be used to influence key derivation. It's used in both the key schedule context and in the key derivation process. Do we even need it? Is it worth removing it to reduce complexity?

@DanGould
Copy link
Contributor

I'm going to mark DHKEM on the draft RFC as unambiguous unless you raise a further concern.

@nothingmuch
Copy link
Collaborator Author

nothingmuch commented Jan 23, 2025

I believe these additional strings are appropriate, both in general (domain separators considered a good practice) and in particular, because although this might be superfluous in BIP 77 due to the use of ephemeral keys in both messages, in the context of a backwards compatible extension where the same pubkey might be used for two different protocols (e.g. the multiparty payjoin experiments) this can help clients reliably distinguish versions.

@DanGould
Copy link
Contributor

Do you think it's also appropriate to have 2 separate info strings, one for a and one for b?

@nothingmuch
Copy link
Collaborator Author

Yes, again both in general and in particular, especially now that both messages appear in the same namespace (i.e. the directory is a key value store where short IDs are keys and values are the union of both message types, and potentially arbitrary future message types)

@DanGould
Copy link
Contributor

I pushed to the bips repo. We're synced now, but missing the yet-to-be-defined-or-implemented ohttp relay payjoin directory relationship

re: https://github.com/orgs/payjoin/discussions/486

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: No status
Development

No branches or pull requests

2 participants