Clients that know the `LSPS2OpeningFeeParams` can bypass the token requirement in LSPS2

[TheBlueMatt]

I have no idea why the spec requires the token only in GetInfo and not in BuyRequest, but we need to ignore the spec and do something sensible here. We should require the token in BuyRequest and bubble that up to the driver via the event as well so that it can't be bypassed by simply skipping GetInfo and sending a BuyRequest right away.

[TheBlueMatt]

CC @martinsaposnic if you want to take this on feel free.

[tnull]

I have no idea why the spec requires the token only in GetInfo and not in BuyRequest, (...) We should require the token in BuyRequest and bubble that up to the driver via the event as well so that it can't be bypassed by simply skipping GetInfo and sending a BuyRequest right away.

I'm not quite sure I'm following: a client can use a token to get an OpeningFeeParams that is committed to by the service's promise secret. When the service receives a buy requests with OpeningFeeParams, it validates the promise before doing anything. So what's the issue?

but we need to ignore the spec and do something sensible here.

Please don't advocate for randomly breaking compatibility with the spec.

[TheBlueMatt]

I'm not quite sure I'm following: a client can use a token to get an OpeningFeeParams that is committed to by the service's promise secret. When the service receives a buy requests with OpeningFeeParams, it validates the promise before doing anything. So what's the issue?

The OpeningFeeParams is not tied to the client in any way, and is basically all constant information (your feerate information is basically static for most LSPs). Lets say you use the token to encode per-client information like live attestations via Apple or Google's device authenticity checks. All that someone has to do is get one valid OpeningFeeParams (that has a long expiry) and leak the HMAC and now anyone can use your LSP and you can't control it.

Please don't advocate for randomly breaking compatibility with the spec.

Admittedly I'd not seen the HMAC (its somewhat well hidden given the public API just shows the raw params), but its not a full solution here. Given an unacceptable security issue that is not fixable, we should be willing to break the spec to fix it :).

But, luckily, as you note there isn't in this case - we just need to commit to the peer's public key in the HMAC and be done with it.

[tnull]

The OpeningFeeParams is not tied to the client in any way, and is basically all constant information (your feerate information is basically static for most LSPs). Lets say you use the token to encode per-client information like live attestations via Apple or Google's device authenticity checks. All that someone has to do is get one valid OpeningFeeParams (that has a long expiry) and leak the HMAC and now anyone can use your LSP and you can't control it.

Yes, but the same thing is true for the token? If it leaks, an unauthorized party might use your service? Committing to the client public key in the promise however makes sense, I think.

Given an unacceptable security issue that is not fixable, we should be willing to break the spec to fix it :).

That would depend on the severity, as breaking interop is also very annoying, especially given that we already have several incompatible LSPS 'flavors' out there.

we just need to commit to the peer's public key in the HMAC and be done with it.

Yeah, I think that's reasonable, now opened #4040

[TheBlueMatt]

Yes, but the same thing is true for the token? If it leaks, an unauthorized party might use your service? Committing to the client public key in the promise however makes sense, I think.

I believe this depends on how the token is used, no? AFAIU tokens do not need to be fixed (and the implementation, even if not the one in ldk-node is very much designed to allow them to be unique per-client), so if, eg, you include a captcha or device integrity token that is some ephemeral single-use thing this isn't true.