search

emschwartz / ilp3

An implementation of Interledger V3
5 stars 3 forks source link

psk implementation incompatible with psk spec #1

Closed michielbdejong closed 6 years ago

michielbdejong commented 6 years ago

This is obviously on purpose, but I'm wondering what the reason behind it is; https://github.com/emschwartz/ilp3/blob/master/lib/psk.js#L10-L11 uses different strings than https://github.com/interledger/rfcs/pull/336 - why not stick to the strings that are used in PSK/1.0?

emschwartz commented 6 years ago

That's not the only thing that makes the two implementations incompatible. The main thing is that PSK/1.0 is generated from the hash of the ILP (type 1) packet, whereas this implementation only hashes the data. If the two are incompatible anyway, I think the magic strings should be different, since the whole point of those is to make it impossible to generate the same keys when using the same secret in two different contexts.

michielbdejong commented 6 years ago

Right, ok. It would be trivial to define PSK/1.0 for both type 1 and type 9. But I guess by taking just the data you remove the dependency on OER, which is nice. The only price you pay is the destination address can now be tampered with (although only within the margin of ILP addresses that still point to the same receiver). And what happened to the whole public header / private header string with the newlines, is that gone as well?

emschwartz commented 6 years ago

The destination address can be tampered with but if it is the payment shouldn't get to the right place or if it does and the receiver uses some data from there to regenerate the encryption or condition key they won't be able to fulfill the payment.

This version doesn't have public headers. For the moment it assumes you're using AES-256-GCM for encryption and doesn't have a way of expressing the fact that you're using a different algorithm. I think we might want to deal with upgrading encryption algorithms by making that a parameter that's passed in on both sides to the PSK module, rather than something that's sent in the clear and theoretically negotiated on the fly.

As far as private headers go, it currently has some structure data that's being used for chunked payments and end-to-end quoting but that's in binary and not extensible. @sentientwaffle made a good point that we might want to separate the chunked payments logic from PSK, in which case PSK would only pass through the decrypted data and the protocol built on top of that would define whatever format it wants.

michielbdejong commented 6 years ago

I think you misunderstood my question. From what I understand, you implemented something that will compete with psk, and my question is, why didn't you just use psk;

In psk, for normal payments, the fulfillment is an hmac of destination address, destination amount, and payload, right?

For best effort payments then, I would expect just the destination amount (not the destination address) to disappear from that list, so the fulfillment would be an hmac of destination address and payload, right?

But in your algorithm, it seems the fulfillment is an hmac of just the payload, and you decided to leave out the destination address. My question is, did you do this because you want to get rid of the dependency on OER?

emschwartz commented 6 years ago

Ah I see. The main reason to hash the packet in the first place is because it includes (or included) the destination amount and we didn't want that to be tampered with. If that isn't there, there's no point of hashing anything except the data and if you do that it also has the nice property of not requiring any specific encoding format at all (because the data is opaque anyway).