[Id-event] AD review of draft-ietf-secevent-http-poll-06

[Benjamin Kaduk <[e-mail]>]

Hi all,

The changes here are also fairly straightforward, so we should be able to
run the IETF LC for push and poll concurrently.


As is the case for push, we should probably cite RFC 8446 somewhere in
addition to 5246.  There's not as clear of a rhetorical need to reference
RFC 2818 here, but it's not wrong to do so.

Section 1

   A mechanism for exchanging configuration metadata such as endpoint
   URLs and cryptographic key parameters between the transmitter and
   recipient is out of scope for this specification.  How SETs are

[my comment about JWT signing key vs. TLS certificate/etc. from push
applies here, to]

Section 1.1

   Throughout this document, all figures MAY contain spaces and extra
   line wrapping for readability and due to space limitations.

I suggest using the lowercase "may", as there's not much point in giving
normative guidance to the authors of this document, from within this
document.

Section 2

   When a SET is available for a SET Recipient, the SET Transmitter
   attempts to deliver the SET by queueing the SET in a buffer so that a
   SET Recipient can poll for SETs using HTTP/1.1 POST.

nit: I expect some other reviewers to want to wordsmith this sentence,
so as an effort to forestall it, I suggest s/attempts to
delivery/prepares to deliver/ (or similar).  The idea being that if you
always know you're going to enqueue, that isn't really an "attempt at
delivery".

   [I-D.ietf-secevent-http-push].  The SET Recipient MUST acknowledge
   receipt to the SET Transmitter.  The SET Recipient SHALL NOT use the

I expect we'll get some IESG comments about this sentence (e.g., "is
there a time bound on when the acknowledgment has to happen?", "is the
transmitter expected to bound the length of its queue and drop new
events if the recipient fails to acknowledge in a timely manner?"),
though I don't have any specific changes I personally propose.  That is,
if you want to leave it as-is and see what happens, I'm okay with that.
(Even a forward-reference to Section 2.4 that has "(e.g., seconds or
minutes)" might help.)

Section 2.1

   more SETs.  Requests MAY be made at a periodic interval (short
   polling) or requests MAY wait, pending availability of new SETs using
   long polling, per Section 2 of [RFC6202].

Someone who follows the references would probably be able to figure out
that in general, short-polling is fairly likely to result in some
zero-SET responses, and long-polling is generally likely to
result in some SETs being returned.  So it's not clear to me whether we
want to think about putting a bit more detail inline here, especially
since there are no hard guarantees in either case.

   o  after validating previously received SETs, the SET Recipient
      initiates another poll request using HTTP POST that includes
      acknowledgement of previous SETs and waits for the next batch of
      SETs.

nit: "waits for the next batch" could be read as implying long-polling,
to the exclusion of short-polling.  Maybe just using "requests" (as per
the previous bullet) again is best.

Section 2.2

   acknowledgement parameters in the form of JSON objects.  The request
   payloads are delivered in a JSON document, as described in
   Section 2.4 and Section 2.5.

nit: 2.5 covers *response* payloads.  I'm not sure whether we want to
cover both requests and responses here or just requests, but we should
be consistent.

      ack
         A JSON array of strings whose values are the "jti" values of
         successfully received SETs that are being acknowledged.  If
         there are no outstanding SETs to acknowledge, this member is
         omitted.  When acknowledging a SET, the SET Transmitter is
         released from any obligation to retain the SET.

nit: as currently written, this is saying that the Transmitter is
acknowledging a SET.  So we probably want something like "When the
request is acknowledging a SET" or "When a SET has been acknowledged".

      setErrs
         A JSON object with one or more members whose keys are the "jti"
         values of invalid SETs received.  The values of these objects
         are themselves JSON objects that describe the errors detected
         using the "err" and "description" values specified in
         Section 2.6.  If there are no outstanding SETs with errors to
         return, this member is omitted.

nit: I suggest s/return/report/, as it avoids the misparse "(SETs with
errors) to return" of the intended "SETs with (errors to return)".

Section 2.3

   In response to a poll request, the SET Transmitter checks for
   available SETs and responds with a JSON document containing the
   following JSON object members:

As written, this seems to require that both 'sets' and 'moreAvailable'
are always present, so that we do not need to set a default behavior for
the absence of 'moreAvailable'.  If that's correct, we might want to
specify some error handling for when the response is malformed in that
manner.

   sets
      A JSON object that contains zero or more nested JSON objects.
      Each nested JSON object's key corresponds to the "jti" of a SET to
      be delivered, and its value is a JSON string containing the value
      of the encoded corresponding SET.  If there are no outstanding
      SETs to be transmitted, the JSON object SHALL be empty.

I think the -03 may have been overzealous with its s/attribute/object/
change -- we don't have nested objects within 'sets'.  That is, there's
the toplevel JSON object for the response, which has a map key "sets",
and the value corresponding to the "sets" key is a JSON object.  But the
contents of that last object is just string/string keys/values, with the
keys being "jti" values and the values being the corresponding encoded
SETs.  So I think this should be "contains zero or more key/value pairs.
Each key string is the "jti" value of a SET to be delivered, and the
value is the corresponding encoded SET" (or similar).

Section 2.4

   After a period of time configured between the SET Transmitter and
   Recipient, a SET Transmitter MAY redeliver SETs it has previously
   delivered.  The SET Recipient SHOULD accept repeat SETs and
   acknowledge the SETs regardless of whether the Recipient believes it
   has already acknowledged the SETs previously.  A SET Transmitter MAY
   limit the number of times it attempts to deliver a SET.

I'm assuming that "redeliver" here involves using the exact same encoded
SET, including 'jti'.  On that assumption, I expect to get some IESG
comments about this paragraph, regarding what layer "reliability" should
be at (since, e.g., we get an HTTP 200 in response to our explicit ack
message) and perhaps about the pacing/rate of such retransmits.  But
it's okay to wait and see what happens; I can't predict sufficiently
accurately what comments we might receive to be confident about a
preemptive "fix".

   If the SET Recipient has received SETs from the SET Transmitter, the
   SET Recipient SHOULD parse and validate received SETs to meet its own
   requirements and SHOULD acknowledge receipt in a timely fashion
   (e.g., seconds or minutes) so that the SET Transmitter can mark the
   SETs as received.  SET Recipients SHOULD acknowledge receipt before

Section 2 has "MUST acknowledge receipt" but this is just "SHOULD
acknowledge receipt in a timely fashion".  While technically these are
not conflicting requirements, it's awfully close, and it would be good
to harmonize them.

Section 2.4.4

   {
     "ack": ["3d0c3cf797584bd193bd0fb1bd4e7d30"],
     "setErrs": {
       "4d3559ec67504aaba65d40b0363faad8": {
         "err": "jwtAud",
         "description": "The audience value was invalid."
       }
     },
     "returnImmediately": true
   }

             Figure 5: Example Poll Acknowledgement with Error

I don't see "jwtAud" as a to-be-registered value in the "Security Event
Token Delivery Error Codes" registry established by the push document,
nor do we register it from this document.

Section 2.5

   In response to a valid poll request, the service provider MAY respond
   immediately if SETs are available to be delivered.  If no SETs are
   available at the time of the request, the SET Transmitter SHALL delay
   responding until a SET is available or the timeout interval has
   elapsed unless the poll request parameter "returnImmediately" is
   "true".

We may get someone asking for us to specify here what the behavior is
when "returnImmediately" is "true".  I know it's described previously in
the document, so may not be strictly required, but it may be helpful to
have nonetheless.  This would also give an opportunity to reword to "if
the poll request parameter 'returnImmediately' is false, the SET
transmitter SHALL delay responding until <X or Y>", as the current
formulation is perhaps ambiguous as to whether the statement groups as
"until (X or Y) unless Z" or "until (X) or (Y unless Z)".

   The following is a non-normative example response to the request
   shown in Section 2.4, which indicates that no new SETs or
   unacknowledged SETs are available:

I think we should pick a subsection (2.4.1?) of Section 2.4, since
there's not an example request at the toplevel.

Section 2.5.1

   The response body for responses to invalid poll requests is left
   undefined.

Can you refresh my memory of the WG discussions where we concluded this
should not be defined?  This is likely to be a magnet for comments and
I'd like to be able to respond properly to them.

Section 2.6

   If a SET is invalid, error codes from the IANA "Security Event Token
   Delivery Error Codes" registry established by
   [I-D.ietf-secevent-http-push] are used in error responses.  As

I'm not sure if there's a good clean way to remind the reader that these
are SET-poll-level responses, not HTTP responses.  The rest of the
section does help, so maybe we should just leave it as-is for now.

   When included as part of a batch of SETs, the above JSON is included
   as part of the "setErrs" member, as defined in Section 2.3 and
   Section 2.4.4.

I think s/2.3/2.2/

Section 3

It might be worth some discussion that for poll, HTTP authentication
authenticates the recipient, whereas for push it authenticates the
transmitter; workflows that need to authenticate the other party would
have to rely on TLS, it seems.

Since poll has the TLS server as the SET Transmitter, we could
potentially pull in RFC 6125 and talk about validating DNS-IDs to
authenticate the Transmitter.  Given that the name to be authenticated
would be part of the information conveyed out-of-band, though, it's not
entirely clear how much value there would be in doing so.

   As per Section 4.1 of [RFC7235], a SET delivery endpoint SHALL
   indicate supported HTTP authentication schemes via the "WWW-
   Authenticate" header.

I'm not entirely sure we need this paragraph (but if we keep it, we
should probably scope it to "when using HTTP authentication").

   Authorization for the ability to pick-up or deliver SETs can be
   determined by using the identity of the SET issuer, or via other
   employed authentication methods.  Among other benefits,

Presumably this would involve some comparison/relationship between
sender and issuer?

As for push, I'm not sure that "pick-up" will be clear to readers.

Section 4

One could perhaps argue that "Authenticating Persisted SETs" from push
could apply here as well, though the scenarios are somewhat different.

Section 4.1

   (see [RFC7515] and Section 5 of [RFC8417]).  This enables the SET
   Recipient to validate that the SET issuer is authorized to deliver
   the SET.

Looking at the diff from push, there's a clear bug in push that I
comment on there, but also it seems we want to s/issuer/Transmitter/
here?

Section 4.3

   SETs may contain sensitive information that is considered Personally
   Identifiable Information (PII).  In such cases, SET Transmitters and

The diff from push shows up as s/e.g., subject claims/PII/.  Either
version is defensible, but we should be consistent.

Section 4.4

   When using access tokens, such as those issued by OAuth 2.0
   [RFC6749], implementers MUST take into account threats and
   countermeasures documented in Section 8 of [RFC7521].

I think the average reader will need a bit more handholding about what
access token usage is relevant here -- my first instinct was about
confusability between SET and a JWT-formatted access token, but it seems
that the rest of the discussion is more in line with using access tokens
for (poll) request authorization.  N.B. that we only talk about HTTP
authentication and TLS authentication earlier in the document, so this
would be the first part that introduces OAuth 2.0 as a possibility.

Section 4.4.1

   [...]
   authentication methods.  Designating the specific methods of
   authentication and authorization are out of scope for the delivery of
   SETs, however this information is provided as a resource to
   implementers.

Indeed, for this whole section it's hard to see a direct connection to
SET transmission; is this general sort of advice not already present
somewhere else that we could incorporate it by reference?

Section 5

[N.B. that these comments are pasted from -push]

I would have expected to see some form of reminder to encrypt data that
needs confidentiality protection, in a privacy considerations section.

   If a SET needs to be retained for audit purposes, a JWS signature MAY
   be used to provide verification of its authenticity.

We should probably give the reader a bit more to conect this signature
to the privacy considerations in question.

Section 5

We have some significant divergence from -push here; I think there's
scope for harmonization (in both directions).

   The propagation of subject identifiers can be perceived as personally
   identifiable information.  Where possible, SET Transmitters and

nit: propagation is an action, but PII has to be a concrete thing.
Maybe we just want to say that subject identifiers can be perceived as
PII (which is what -push does)?

Section 7.2

Earlier we say "MUST take into account threats and countermeasures
documented in Section 8 of [RFC7521]", which is probably enough to get
people demanding that 7521 be listed as Normative.

-Ben