Conversation
nothingmuch
force-pushed
the
gateway-opt-in
branch
2 times, most recently
from
9bf7352 to
3d0d876
Compare
|
converting to draft because mokckito dev dep broke MSRV, i'll downgrade tomorrow this depends on RFC 9540 stuff anyway and another loose end, I guess the the GATEWAY_ORIGIN environment variable can be optional now, so i'll do that too |
|
mockito 0.32 and up require rust 1.65, that's when the async stuff was introduced and dev-dependencies can't be optional unfortunately so i think we have three choices:
|
|
Secondly, I screwed up and didn't realize the websockets handler does not work like CONNECT until yesterday, so I suspect the |
|
The reason we use WebSocket to bootstrap is so that it can be done from a browser, where WebSockets, where as far as my understanding extended when I wrote the ws-bootstrap feature, is the only way to make a tunnel streaming a logical TLS/TCP connection through a proxy because HTTP CONNECT is not available in that way. So it was built as a secondary mechanism to HTTP CONNECT (and we've come up with a third, to get keys from trusted bip21 pj= fragment input), where HTTP CONNECT is not available in web-first environments. This is the PR where I came up with it. Probably makes sense to write detailed mod-level docs after we resolve this conversation and you feel like you understand what I'm saying. edit: see Mutiny's WebSockets bootstrap here: https://github.com/MutinyWallet/mutiny-node/pull/820/files#diff-9bd1c92f85da5773c1cf3b66662d28004dd0c00ab719781af7d7a77aa712fb20R140-R209 |
DanGould
left a comment
DanGould
left a comment
There was a problem hiding this comment.
Beautiful changes.
I sense some unease about the encapsulation of Policy as Known vs InFlight but also a sense that the abstraction is best as implemented.
Other than what you documented in KnownGateways::insert, do you see this being influenced by another change to itself change? If so, what would that change be? It seems ok and well tested as-is.
test_inflight_deduplication was the hardest for me to understand even though step-by-step it was commented well. A docstring summary might have helped that. Curious also if you think unwrap() is the most appropriate to leave in the test there.
nothingmuch
force-pushed
the
gateway-opt-in
branch
from
3d0d876 to
ce2df2d
Compare
It's kinda lame that
It felt a bit awkward adding expect messages, e.g. Perhaps a more readable way of doing that is to bundle the condvar & mutex into a simpler synchronization helper that just has |
nothingmuch
force-pushed
the
gateway-opt-in
branch
from
ce2df2d to
c099b6d
Compare
|
Force pushed, addressing review comments, removing additional |
This change signals to relays (see payjoin/ohttp-relay#58) that even if the gateway is not known to the relay, requests are still allowed for the purposes of BIP 77. Because receivers choose the directory (which is its own gateway), but senders choose the relay, before introducing this functionality it this would have been the responsibility of the sender (or more realistically their wallet vendor) to choose an appropriate relay, one that accepts requests on behalf of the receiver's chosen directory.
These are the places I really like to bubble up |
I disagree in this case, since that code makes no sense as an example, so there's no benefit to avoiding unwrap() in case the code gets copied around, but if an error does occur in a test for some reason (afaik only EINVAL should be possible from underlying pthread_cond_wait, etc, which would suggest a bug) then bubbling as an error would make it harder to figure out where it originated. |
|
note that Cache-Control is not correct for service unavailable, it should be Retry-After |
This change signals to relays (see payjoin/ohttp-relay#58) that even if the gateway is not known to the relay, requests are still allowed for the purposes of BIP 77. Because receivers choose the directory (which is its own gateway), but senders choose the relay, before introducing this functionality it this would have been the responsibility of the sender (or more realistically their wallet vendor) to choose an appropriate relay, one that accepts requests on behalf of the receiver's chosen directory.
This change signals to relays (see payjoin/ohttp-relay#58) that even if the gateway is not known to the relay, requests are still allowed for the purposes of BIP 77. Because receivers choose the directory (which is its own gateway), but senders choose the relay, before introducing this functionality it this would have been the responsibility of the sender (or more realistically their wallet vendor) to choose an appropriate relay, one that accepts requests on behalf of the receiver's chosen directory.
This change signals to relays (see payjoin/ohttp-relay#58) that even if the gateway is not known to the relay, requests are still allowed for the purposes of BIP 77. Because receivers choose the directory (which is its own gateway), but senders choose the relay, before introducing this functionality it this would have been the responsibility of the sender (or more realistically their wallet vendor) to choose an appropriate relay, one that accepts requests on behalf of the receiver's chosen directory. Rationale for adding a UUID: although arguably 64 bits of randomness is more than enough, and even just the string "BIP 77" is likely to be unique and unambiguous, adding a UUID practically ensures an opt-in is really an opt-in as 128 random bits would not be repeated except intentionally. Rationale for format: we just need a set of opaque identifiers, encoding the same way as TLS ALPN does makes sense since that's well specified, binary safe, easy to parse and places readonable limits on string lengths.
nothingmuch
force-pushed
the
gateway-opt-in
branch
2 times, most recently
from
32b4349 to
d523a1f
Compare
This change signals to relays (see payjoin/ohttp-relay#58) that even if the gateway is not known to the relay, requests are still allowed for the purposes of BIP 77. Because receivers choose the directory (which is its own gateway), but senders choose the relay, before introducing this functionality it this would have been the responsibility of the sender (or more realistically their wallet vendor) to choose an appropriate relay, one that accepts requests on behalf of the receiver's chosen directory. Rationale for adding a UUID: although arguably 64 bits of randomness is more than enough, and even just the string "BIP 77" is likely to be unique and unambiguous, adding a UUID practically ensures an opt-in is really an opt-in as 128 random bits would not be repeated except intentionally. Rationale for format: we just need a set of opaque identifiers, encoding the same way as TLS ALPN does makes sense since that's well specified, binary safe, easy to parse and places readonable limits on string lengths.
This change signals to relays (see payjoin/ohttp-relay#58) that even if the gateway is not known to the relay, requests are still allowed for the purposes of BIP 77. Because receivers choose the directory (which is its own gateway), but senders choose the relay, before introducing this functionality it this would have been the responsibility of the sender (or more realistically their wallet vendor) to choose an appropriate relay, one that accepts requests on behalf of the receiver's chosen directory. Rationale for adding a UUID: although arguably 64 bits of randomness is more than enough, and even just the string "BIP 77" is likely to be unique and unambiguous, adding a UUID practically ensures an opt-in is really an opt-in as 128 random bits would not be repeated except intentionally. Rationale for format: we just need a set of opaque identifiers, encoding the same way as TLS ALPN does makes sense since that's well specified, binary safe, easy to parse and places readonable limits on string lengths.
This change signals to relays (see payjoin/ohttp-relay#58) that even if the gateway is not known to the relay, requests are still allowed for the purposes of BIP 77. Because receivers choose the directory (which is its own gateway), but senders choose the relay, before introducing this functionality it this would have been the responsibility of the sender (or more realistically their wallet vendor) to choose an appropriate relay, one that accepts requests on behalf of the receiver's chosen directory. Rationale for adding a UUID: although arguably 64 bits of randomness is more than enough, and even just the string "BIP 77" is likely to be unique and unambiguous, adding a UUID practically ensures an opt-in is really an opt-in as 128 random bits would not be repeated except intentionally. Rationale for format: we just need a set of opaque identifiers, encoding the same way as TLS ALPN does makes sense since that's well specified, binary safe, easy to parse and places readonable limits on string lengths.
nothingmuch
force-pushed
the
gateway-opt-in
branch
2 times, most recently
from
34cff4e to
79efb68
Compare
nothingmuch
force-pushed
the
gateway-opt-in
branch
from
79efb68 to
93ed428
Compare
With this change, the payjoin directory's RFC 9540 compatible OHTTP gateway additionally opts in to receiving OHTTP requests from arbitrary relays. See payjoin/ohttp-relay#58. Depends on - #549 - #572
nothingmuch
force-pushed
the
gateway-opt-in
branch
3 times, most recently
from
37c5d1c to
9322f65
Compare
RFC 9458 (OHTTP) specifies a 1:1 mapping between relays and gateways in order to preclude abuse. That presents an issue for payjoin UX, the receiver chooses the directory, which is its own OHTTP gateway, but then the sender must know a relay that will forward requests to that directory. In order to eliminate the need for maintaining up to date relay configurations, this commit adds a mechanism that allows OHTTP gateways to signal to relays that they will accept OHTTP requests for a specified purpose (i.e. BIP77), even if the relay is not configured to allow that specific gateway a priori. The new opt-in mechanism is extends RFC 9540: when querying the `/.well-known/ohttp-gateway` endpoint with query string `?allowed_purposes`, if the gateway responds with an `application/x-ohttp-allowed-purposes` response whose body is a list of bytestrings encoded in the same format as TLS ALPN (U16 big endian element count list, with U8 length prefixed bytestring), and this response contains the magic string `BIP77 454403bb-9f7b-4385-b31f-acd2dae20b7e` as one of the elements, this indicates that the relay may forward BIP77 OHTTP requests to it. The relay now expects POST requests to include the gateway in the request path. Only the scheme and authority are to be included (same as GatewayUri struct), the RFC 9540 path on the gateway's host is implied. When a gateway does not opt-in, this is treated as 404 Not Found. When the prober is over capacity, the relay will respond with 503 Service Unavailable whenever requests to unknown gateways are made. Retry-After will be set for these responses according to the TTL of the soonest expiring gateway status.
In order to allow the prober to share the same client as the proxy, the Request<Incoming> type needs to be boxed, as even an empty Incoming body type can't be created outside of the hyper crate. When converting the request, two changes in behavior were also implemented: No longer set Host header to 0.0.0.0. Instead, let the client set it from the request URL's authority. Also specify Content-Type as `message/ohttp-req`, this was previously omitted from the forwarded request.
nothingmuch
force-pushed
the
gateway-opt-in
branch
from
9322f65 to
eb67b2c
Compare
payjoin-test-util's Cargo.toml now refers to the merge commit of payjoin/ohttp-relay#58, so before the payjoin crate can be released ohttp-relay needs to be released and this pin removed.
payjoin-test-util's Cargo.toml now refers to the merge commit of payjoin/ohttp-relay#58, so before the payjoin crate can be released ohttp-relay needs to be released and this pin removed.
This change signals to relays (see payjoin/ohttp-relay#58) that even if the gateway is not known to the relay, requests are still allowed for the purposes of BIP 77. Because receivers choose the directory (which is its own gateway), but senders choose the relay, before introducing this functionality it this would have been the responsibility of the sender (or more realistically their wallet vendor) to choose an appropriate relay, one that accepts requests on behalf of the receiver's chosen directory. Rationale for adding a UUID: although arguably 64 bits of randomness is more than enough, and even just the string "BIP 77" is likely to be unique and unambiguous, adding a UUID practically ensures an opt-in is really an opt-in as 128 random bits would not be repeated except intentionally. Rationale for format: we just need a set of opaque identifiers, encoding the same way as TLS ALPN does makes sense since that's well specified, binary safe, easy to parse and places readonable limits on string lengths.
payjoin-test-util's Cargo.toml now refers to the merge commit of payjoin/ohttp-relay#58, so before the payjoin crate can be released ohttp-relay needs to be released and this pin removed.
This change signals to relays (see payjoin/ohttp-relay#58) that even if the gateway is not known to the relay, requests are still allowed for the purposes of BIP 77. Because receivers choose the directory (which is its own gateway), but senders choose the relay, before introducing this functionality it this would have been the responsibility of the sender (or more realistically their wallet vendor) to choose an appropriate relay, one that accepts requests on behalf of the receiver's chosen directory. Rationale for adding a UUID: although arguably 64 bits of randomness is more than enough, and even just the string "BIP 77" is likely to be unique and unambiguous, adding a UUID practically ensures an opt-in is really an opt-in as 128 random bits would not be repeated except intentionally. Rationale for format: we just need a set of opaque identifiers, encoding the same way as TLS ALPN does makes sense since that's well specified, binary safe, easy to parse and places readonable limits on string lengths.
With this change, the payjoin directory's RFC 9540 compatible OHTTP gateway additionally opts in to receiving OHTTP requests from arbitrary relays. See payjoin/ohttp-relay#58. Depends on - #549 - #572
payjoin-test-util's Cargo.toml now refers to the merge commit of payjoin/ohttp-relay#58, so before the payjoin crate can be released ohttp-relay needs to be released and this pin removed.
2 participants
This adds a new
gateway_probermodule, whose main typeProberallows checking if aGatewayUriis allowed to be queried by the relay.This is done by performing a
GET /.well-known/ohttp-relay?allowed_purposes. If the response is anapplication/x-ohttp-allowed-purposes, which is a list of values encoded as in TLS ALPN (U16BE for number of elements, U8 length prefix for each binary string value), and this is found to contain the magic stringBIP77 454403bb-9f7b-4385-b31f-acd2dae20b7e, then the gateway is considered to have opted in to receiving OHTTP requests for this purpose.Although appropriate in principle, the
Cache-Controland/orRetry-Afterheaders given by the (potential/alleged) gateways are not taken into consideration as no such functionality exists in the payjoin directory's OHTTP gateway implementation, and such functionality would require parsing logic (a crate exists for this FWIW).When the prober is over capacity (too many cached or inflight gateway probes), a
Cache-Control max-age=...header is provided for the 503 Service Unavailable response, with the TTL of the earliest expiring entry.Other negative responses do not set
Cache-Control, doing so would require a secondError::NotFoundvariant or some other way of setting that, this doesn't really seem useful for clients or service operators, so that is also omitted for simplicity.This should probably be squashed before merging since the first commit does not past lints due to dead code.
Depends on #47