[saag] Re: Requesting feedback on Push-Based Delivery For Mu ltiple Security Event Token (SET) Using HTTP

Hi Yaron, thank you for your review. Responses are inline.

# Review: draft-deshpande-secevent-http-multi-set-push-00


Acting individually, but as a former SecEvent co-chair, below are my
> comments. In general I think this is useful work which makes total sense as
> an extension to the existing SecEvent protocols.


## Comments


### Title

Token -> Tokens


Thanks, this has been fixed.

### General

The document requires quite a bit of copyediting. Let ChatGPT have a go at
> it!


We will make sure we do another pass at copy editing before the next
version.

### Buffer Size

The Intro mentions that "possible implementation constraints such as buffer
> size limitations between the transmitter and recipient is out of scope." I
> think this needs to be explained. Buffer size constraints/negotiation is
> very common, why is it not needed in this case? Even if RFC 8935 did not do
> that, it's clearly more relevant for the batch use case than for the
> single-SET case.


We added "...but is expected to be defined by profiles of this
specification." to that sentence.

A hardcoded limit of 20 SETs is mentioned below, IMO that's a good
> justification but then please mention it earlier.


Upon further consideration, we thought it would be more appropriate to
replace the specific recommendation of 20 with this: "The maximum number of
SETs in a push MAY be set by the Transmitter for itself and SHOULD be
communicated offline to the Receivers."

### SET Delivery Semantics


This section is very confusing. It seems to discuss receipt and
> acknowledgement of individual SETs, one by one; on the other hand, the SETs
> are delivered as part of a JSON document which makes individual receipt
> hard or impossible, depending on the JSON implementation.


There are some streaming JSON implementations out there, but I'm wondering
> if instead of one big JSON document, it wouldn't be better to deliver many
> separate JSON documents, one for each SET, and then use HTTP chunking or
> even HTTP SSE underneath it for efficient and more reliable delivery.


Again, once the hardcoded-20 limit comes into consideration, these
> questions largely go away.


The intent was not to imply this is a streaming protocol that requires
streaming JSON. Like you mentioned, this wasn't really clear until reading
the recommended limit of 20. We've revised the introduction to include this
sentence which should hopefully clear this up sooner:

"This specification defines a way to send batches of SETs in a single POST
request for more efficient transport."


### Transmitting SETs

"The following is a non-normative example of a response." This should be
> "request."


Thanks, fixed.

### Sets Field

The `sets` member is marked as Optional but then the second example sends
> it as an empty object. If that's the preferred way, better make it
> Mandatory.


You're right, this has now been updated to a required field and noted that
it "MAY be an empty object"

### Media Type

You are using a simple `application/json`, and I'm not sure that's ideal.
> The SET spec does not go for a simple `application/jwt`. Maybe
> `applications/secevent-batch+json`?


The authors don't believe there is any advantage of adding a more specific
media type to the HTTP request.

### Empty "ack"

Is there a semantic difference between a non-existent `ack` and one that
> contains an empty array?


You're right, these are equivalent. We will revise this to be a required
field that may be an empty array.

### Response: High Level


I think there's too much optionality. Specifically, `err` should be
> Mandatory.


Agreed, `err` should be mandatory.

It's also not ideal to include two objects with the same type of key, where
> the Recipient needs to ensure there is no intersection and that all
> received SETs are included. It may be cleaner to turn the structure
> inside-out into one object indexed by `jti` where there's either an `ack`
> (possibly signaled by a `null`) or a `setErr` for each SET.


This structure is aligned with RFC8936, so I don't think it would be a good
idea to significantly change it at this point.

Since `description` is optional (as it should be IMO), there's no need to
> make Content-Language a MUST.


Thanks, revised this to add "If the response contains a `description`, then
the response MUST include a Content-Language header..."

### Failure Response


Please consider using RFC 7807, "problem details". And again, the `err`
> field should be mandatory. Also, `many_sets` -> `too_many_sets`.


We wanted to align with RFC8936, so using Problem Details instead would be
too much of a change.

We will make the `err` required, and change to `too_many_sets`

### 7.1 section title


Replace "response" by "request." And by the way this is symmetric:
> Transmitters also need to protect themselves against responses that are too
> large.


Fixed title.

There isn't anything unique about this draft in terms of dealing with too
large responses, so I don't think it's worth mentioning. The intended
response size will be proportional to the request size. Of course a
malicious receiver can send a large response body or hold a connection open
indefinitely, but that is always true with any API.

### IANA Considerations


Arguably "Failure Response” errors should be its own registry, if you want
> interoperability between Transmitters and Receivers.


Given that the errors in this section are not about individual SETs, but
about the request, there isn't a clear interoperability story for using an
error registry here anyway. It's likely that any failure will require
manual intervention to resolve. When the error refers to an individual SET,
the error codes defined in RFC8935 are used instead.

---
Aaron Parecki

_______________________________________________
saag mailing list -- [email protected]
To unsubscribe send an email to [email protected]