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

[Mike Jones <[e-mail]>]

draft-ietf-secevent-http-poll-07<https://tools.ietf.org/html/draft-ietf-secevent-http-poll-07> was published to address these review comments.  (-08<https://tools.ietf.org/html/draft-ietf-secevent-http-poll-08> addressed additional editorial nits.)  Descriptions of the changes made for these comments are inline, prefixed by "Mike>".



-----Original Message-----
From: Id-event <[e-mail]> On Behalf Of Benjamin Kaduk
Sent: Tuesday, December 10, 2019 4:37 PM
To: [e-mail]
Cc: [e-mail]
Subject: [Id-event] AD review of draft-ietf-secevent-http-poll-06



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.



Mike> Done



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]



Mike> Like in Push, this has been simplified to simply "cryptographic keys".



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.



Mike> Done



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".



Mike> Done



   [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.)



Mike> Added "and SHOULD do in a timely fashion, as described in <xref target="pollRequest"/>".



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.



Mike> Added the new sentence starting "Note that short polling will result in retrieving zero or more SETs"...



   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.



Mike> Done



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.



Mike> I deleted the poorly worded sentence, as what it was trying to convey is already normatively described better elsewhere.



      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".



Mike> Reworded to "Once a SET has been acknowledged, the SET Transmitter is released from any obligation to retain the SET.".



      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)".



Mike> Done



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.



Mike> It now says that "moreAvailable" can be omitted and defines what that means.  (Note that an example already omitted it.)



   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).



Mike> I completely rewrote this language, eliminating the incorrect "nested", and substantially simplifying it.



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".



Mike> Per your analysis above, I didn't change this text at this time.



   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.



Mike> The new text cited above "and SHOULD do in a timely fashion, as described in <xref target="pollRequest"/>" achieves this harmonization.



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.



Mike> The example now uses a defined error code.



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)".



Mike> I tightened this up with the new language "unless the poll request parameter <spanx style="verb">returnImmediately</spanx> is present with the value <spanx style="verb">true</spanx>".



   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.



Mike> I now cite the subsection.



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.



Mike> I added that the body "SHOULD be ignored."



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.



Mike> I think it's already clear enough, and so didn't make a change.



   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/



Mike> Fixed



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.



Mike> As in Push, this section was formerly poorly worded, and has largely been rewritten.



   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").



Mike> Done



   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?



Mike> Rewritten



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



Mike> Use of the term "pick-up" is gone.



Section 4



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



Mike> No change at this time.



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?



Mike> I clarified that the issuer provides sets.  (It doesn't deliver them, which is the transmitter's job.)



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.



Mike> Both drafts now say "PII".  (Push was changed.)



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.



Mike> Clarified that access tokens are used with HTTP Authentication.



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?



Mike> I deleted this unhelpful and non-actionable sentence.



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.



Mike> I added the sentence "Furthermore, data that needs confidentiality protection MUST be encrypted, either via TLS or using JSON Web Encryption (JWE), or both" to both drafts.



   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.



Mike> This sentence actually wasn't about privacy at all, so I deleted it from both drafts.



Section 5



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



Mike> I reworked this paragraph and used the same text in both drafts.



   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)?



Mike> Done as part of the reworking above



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.



Mike> Done



-Ben



_______________________________________________

Id-event mailing list

[e-mail]<mailto:[e-mail]>

https://www.ietf.org/mailman/listinfo/id-event



Mike> Thanks again, Ben!



                                                       -- Mike