spring-83 icon indicating copy to clipboard operation
spring-83 copied to clipboard

body content in the DELETE /:key_id endpoint

Open kindrowboat opened this issue 2 years ago • 20 comments

Hey Robin, just read through the new draft.

In reference to the DELETE /:id endpoint. I would caution against relying on a body with a time tag for two reasons.

  1. The HTTP spec discourages adding a body to a delete request. RFC 9110 HTTP Semantics 9.3.5 DELETE. You could make the case that we fall into the carve out of

    A client SHOULD NOT generate content in a DELETE request unless it is made directly to an origin server that has previously indicated, in or out of band, that such a request has a purpose and will be adequately supported.

    However this would make Spring '83 an odd-duck in HTTP backed protocols, which leads to the next point...

  2. Some HTTP clients will drop body content on DELETE requests, especially proxies in an attempt to broadly mitigate request smuggling attacks.

Counter proposal: no DELETE endpoint

I personally think the DELETE endpoint adds unnecessary complexity to the specification. A board can already be deleted by:

  1. wait for the TTL to expire, or
  2. make a POST /:key_id with only the time body in it

While (2) may not truely delete the board, it does clear it out, and the cleared out board will be propagated to the realm just like any other board. The board's very existence will eventually drop after the TTL expires.

kindrowboat avatar Jun 30 '22 03:06 kindrowboat

I realized that making another issue might be adding more noise that signal. To tie it together, this is related to #6 and #9. Feel free to close this issue if you think the discussion should be happening there.

kindrowboat avatar Jun 30 '22 03:06 kindrowboat

Not at all, this is super helpful! I appreciate the perspective (and the cross-linkage).

robinsloan avatar Jun 30 '22 03:06 robinsloan

Heh heh -- reading this and thinking about it made me realize something slightly different: the DELETE endpoint, as currently specified, can totally be abused; a server could cache a DELETE request and then use it, verbatim, as a PUT request to "re-initialize" a board at any time in the next 22 days. Which would be a weird thing to do, obviously… but the point is, DELETE doesn't absolutely delete, which is the one thing you want a delete command to do 🤓

There are potential fixes, but they make the specification for this part even wonkier, which is saying a lot.

Designing this DELETE stuff, it has felt like it "goes against the grain" of the cryptographic medium somehow, and I'm now inclined to agree and say, yeah, maybe the way you delete a board is just PUT a blank one and let it expire. Given my druthers I'd rather have a pure, powerful DELETE as well, but not at the expense of complexity and wonkiness.

robinsloan avatar Jun 30 '22 14:06 robinsloan

Yikes, good catch. It can't just be the time that's signed, it has to be some message that's explicitly a delete, and also invalid as a board so it can't be reused. Maybe like delete 2022-06-30T15:05:40Z. But yeah, PUTing a blank one also works.

makew0rld avatar Jun 30 '22 15:06 makew0rld

I really think DELETE should be kept in the spec, I don't like the idea of just PUTing blank boards and waiting for them to expire, it's just not the same function. As @makeworld-the-better-one notes and I'm pretty sure I also noted in another thread somewhere (getting hard to keep track!) a signed header that is explicitly a "delete" token followed by datetime will both avoid the problem of having a body with a DELETE request and ensure the signed token is explicit to its function.

rpj avatar Jun 30 '22 15:06 rpj

@rpf By "signed header" do you mean HTTP header? It would have to be "signed header, with the fields in exactly this order, in exactly this format" -- which is of course doable, but pretty "heavy", both in terms of specification and implementation. I think the simplest fix is to require the body to be

<time datetime="<timestamp inside a 5 minute window>">DELETE</time>

acknowledging that DELETE bodies are, as @motevets notes, fairly ghostly entities.

It's preeetty wonky! But the command is useful. I am mulling.

robinsloan avatar Jun 30 '22 16:06 robinsloan

<time datetime="<timestamp inside a 5 minute window>">DELETE</time>

A malicious server can still cache this and reuse it for a PUT, it's still a valid board. Or am I missing something?

makew0rld avatar Jun 30 '22 16:06 makew0rld

@rpf By "signed header" do you mean HTTP header? It would have to be "signed header, with the fields in exactly this order, in exactly this format"

@motevets yup exactly my thinking (probably actually needs to be two headers: one for the data, another for the signature). it's slightly more machinery I agree but it shouldn't be much of a heavy lift for any of the servers, we already have all the necessary pieces in place it's just a matter of extracting the data from a different spot and verifying that.

You are right that it could easily be accomplished by sticking a DELETE token in the body. However I do worry about the condition you brought up recently (that I admit I had forgotten): DELETE requests by-spec shouldn't have a body, which means a lot of proxies will either choke on the request or just strip the body. Either would be bad news bears for us. A signed header avoids that entirely.

rpj avatar Jun 30 '22 16:06 rpj

Ah right, you can't have a body. All this complexity and catches make me lean towards just PUTing a blank board instead of having DELETE. Then you get everything "for free": signatures, gossiping, TTL, etc.

makew0rld avatar Jun 30 '22 16:06 makew0rld

I don't agree with the assessment that DELETE requests can't/shouldn't have bodies. My reading indicates it's okay that they do; it's just that the meaning of those bodies is totally undefined.

@makeworld-the-better-one You're right about re-use, but servers could reject PUTs with this particular content -- essentially, it becomes a "magic word". The wonkiness increases, though.

Anyway, if we are bothering with that kind of special handling, we could just as easily say to servers: "If you receive a blank board with absolutely nothing except the <time> element, interpret that as a request to wipe the board from the database." Using the "correct" HTTP verbs is fine when it's useful, but I am absolutely not a purist.

I'm currently leaning towards no DELETE as well, but I will continue to mull.

robinsloan avatar Jun 30 '22 16:06 robinsloan

Fun discussion. :)

Anything that wipes a board from a server's databate (regardless of its HTTP verb) will be vulnerable to the "replay attack" that @robinsloan noted earlier. To mitigate this, I think the spec would need to specify something like:

After a board is deleted, the server MUST keep track that the board was deleted and its time of deletion and respond to all subsequent PUT /:key_id requests with a time before the delete time with 409 CONFLICT and respond to all GET /:key_id with 404 NOT FOUND until such time that a board whose time is greater than the deleted time is PUT to the server.

Personally as a server maintainer, I'd rather just someone PUT me an empty board and keep that on my server for 22 days rather than keep track of a board's deleted state and time.

kindrowboat avatar Jun 30 '22 16:06 kindrowboat

"If you receive a blank board with absolutely nothing except the

I hadn't even considered this as an option, but I actually think I like this best! Reuses all the PUT machinery as others have noted would be ideal (and it would) but still allows users a way to properly delete a board rather than just pushing an empty one (I realize the distinction is minimal, but I genuinely feel it's an important one as a user)!

rpj avatar Jun 30 '22 16:06 rpj

Also, while it's very important to enumerate possible attack vectors, I'm not sure it's necessary to try to close the "malicious server" vector within the context of this spec addition (partly because it's really hard to do so): those could/should be mitigated with realm block lists instead, no?

rpj avatar Jun 30 '22 16:06 rpj

I'm not sure it's necessary to try to close the "malicious server" vector within the context of this spec addition

@rpj Yeah, I agree with this sentiment -- with the proviso that there's a broad swath of malicious activity that might be difficult to detect and attribute, so, if there are simple interventions that make some of that activity impossible, they are probably worthwhile.

robinsloan avatar Jun 30 '22 16:06 robinsloan

I don't think the replay attack could be mitigated by the blocklist. The blocklist blocks keys (iirc). The replay attack uses the key of the deleted board to repost an old board.

Let's says Celeste and Doug were having an argument, and Celeste posts something in the heat of moment to her board thats she later regrets. She then asks the Spring '83 server to DELETE and forget her board. Doug, who is still upset with Celeste, doesn't doesn't want her to get off that easy. He still has a copy of her board and it's signature (all publicly available.) He posts it back to the server she deleted it from, and the server happily accepts it. The server admin wants to help, but they can't block Doug from reposting a valid board that was signed with Celeste's key, without blocking Celeste entirely. Every time for the next 22 days that Celleste tries to delete the board, Doug reposts it.

Celeste's only real recourse is to PUT a new board (blank or otherwise) with a newer timestamp to her old board.

FWIW: this is Celeste. 20210724_165659

kindrowboat avatar Jun 30 '22 16:06 kindrowboat

I don't think the replay attack could be mitigated by the blocklist. The blocklist blocks keys (iirc). The replay attack uses the key of the deleted board to repost an old board.

Ah yes you are right here, and I should've specified: I meant a host block list in the realm file (which you are right, does not exist yet in the spec). I do realize that we could be playing forever whackamole with something like that, but the same can be said for a malicious key block list: bad actors have the same ability to generate new keys as we do, after all.

Let's says Celeste and Doug were having an argument...

Spot on. Could get around this by closing the replay attack hole, but that presents all sorts of new complexities...

FWIW: this is Celeste.

😍

rpj avatar Jun 30 '22 16:06 rpj

@motevets Oo, thank you for this scenario -- it is so cogent and revealing. I think you've revealed the underlying truth here, something that's deep in the "grain" of the medium: a board must hold its place, with a timestamp, or it can trivially replaced by an old copy of itself.

This is totally clarifying!

robinsloan avatar Jun 30 '22 17:06 robinsloan

Here's an idea. I realized that "make DELETE request" and "return 404 Not Found" were fused in my mind… but perhaps they don't have to be. Here is a pitch:

  1. Publisher makes PUT request for key, with minimal board required by spec: <time datetime="<timestamp string>">. No closing tag. That's 38 bytes.

  2. Server receives request, stores board.

  3. Client makes GET request for key.

  4. Server retrieves board, but before returning it, notes that its byte length is 38... and there is only one kind of board that can be only 38 bytes long, a "tombstone" board... so, it returns: 404 Not Found.

I actually think this is less wonky than several of the things I have proposed earlier. Not completely un-wonky, but perhaps workable.

robinsloan avatar Jun 30 '22 19:06 robinsloan

This idea seems really neat, especially as it would be very simple to implement for the server (as it just is a minimal check while evaluating a GET, and also could be propagated in the same way as other boards). Although, it somewhat bothers me, that </time> is required in ordinary boards, but not this one. Couldn't one just allow "/>" at the end of the time tag in both cases? This would increase consistency, and open up six extra bytes for ordinary boards. (But would mean that an ordinary board cannot be totally empty, because this would not be distinguishable from a deleted board)

scren avatar Jul 01 '22 20:07 scren

Although, it somewhat bothers me, that is required in ordinary boards, but not this one. Couldn't one just allow "/>" at the end of the time tag in both cases?

Heh heh, @scren, it bothers me, too!

I don't think <time /> is a valid tag in HTML5. Not that I am the biggest "rahhh, everything must be done exactly as specified" person in the world… but something about the context of writing a specification made me want to be a bit more attentive to these details. I think, in practice, <time /> would work fine, but requiring something that's "incorrect" makes me feel a bit itchy. I wish it didn't, because it would be a neat solution!

Maybe <time datetime="foo"></time> -- with no text content -- is the right payload for these minimal pseudo-delete requests. I definitely understand the desire for consistency.

robinsloan avatar Jul 02 '22 15:07 robinsloan