Closed dthaler closed 4 years ago
Responses to first half of feedback:
MHO the middle layer is not needed in this figure. We have two layers, namely the TEEP protocol layer and the HTTP transport.
Disagree. There are three protocols with different specs (TEEP protocol spec, this spec, and an HTTP spec). Added a sentence to clarify:
This document specifies the middle layer (TEEP-over-HTTP), whereas the top layer (TEEP) is specified in [I-D.ietf-teep-protocol].
Next, in response to
I am wondering whether Section 3 shouldn’t go to the architecture draft
Great question, I'd be ok with that.
The content type has to change. Currently it talks about JSON.
Draft 06 never mentions JSON anywhere, maybe you looked at an older version? Draft 05 already changed all JSON to CBOR (see https://tools.ietf.org/rfcdiff?url2=draft-ietf-teep-otrp-over-http-05.txt)
Reference https://tools.ietf.org/html/rfc6125 Instead of RFC 2818.
Addressed in PR #14
To me, the following three headers shown in the draft are an overkill:
X-Content-Type-Options: nosniff Content-Security-Policy: default-src 'none' Referrer-Policy: no-referrer
The draft is following the security recommendations of https://tools.ietf.org/html/draft-ietf-httpbis-bcp56bis which includes these headers.
• Does it use the HTTP URI scheme? (Yes, in our case) • What HTTP messages are being used? (POST only in our case) • What URIs are we sending the requests to? (I don’t think we talk about this but we should.)
The draft does discuss the URIs already. For example, it says:
Only the POST method is specified for TAM resources exposed over HTTP. A URI of such a resource is referred to as a "TAM URI". A TAM URI can be any HTTP(S) URI. The URI to use is configured in a TEEP Agent via an out-of-band mechanism, as discussed in the next section.
Issues #22, #23, and #24 now track portions of Hannes's review and this issue is only for the (mainly editorial) comments not covered in any of those.
Fixed in draft -07
Hannes writes: I read through https://tools.ietf.org/html/draft-ietf-teep-otrp-over-http-06 and have a few high level comments.
I wonder whether there is room for simplification in these two figures below.
IMHO the middle layer is not needed in this figure. We have two layers, namely the TEEP protocol layer and the HTTP transport.
The same is true for the next figure in the document. I would remove the TEEP/HTTP implementation part and the figure is already substantially simpler to understand. Furthermore, I am wondering whether Section 3 shouldn’t go to the architecture draft because this is not really about the HTTP transport. Replace HTTP with CoAP, MQTT, etc. and the design aspect would still be the same. Furthermore, we have decided in the architecture that we want to provide application layer security independent of the transport and hence the question about where various pieces should go is less about security anymore. We could have made the design differently but we followed the OTrP approach.
Minor aspects:
The content type has to change. Currently it talks about JSON.
Reference https://tools.ietf.org/html/rfc6125 Instead of RFC 2818.
In terms of HTTP transport aspects I believe the important aspects are
• Does it use the HTTP ports (80/443)? (Yes, in our case) • Does it use the HTTP URI scheme? (Yes, in our case) • What HTTP messages are being used? (POST only in our case) • What URIs are we sending the requests to? (I don’t think we talk about this but we should.) • What header fields does the client and the server use? () • Are we using Cookies? (I would say that we don’t. Currently not discussed.) • The use of status codes • Server Push • HTTP authentication ** • Redirect handling ***
*) The text says that the client uses the Accept header but I don’t see any normative language there. The headers the server uses are listed but my understanding of those is that they are only relevant for co-existence with web browsing. Clearly, the communication we are describing isn’t useful for web browsing scenarios.
IMHO the following statement should be enough: “ The "Cache-control" header SHOULD be set to disable caching of any TEEP protocol messages by HTTP intermediaries. Otherwise, there is the risk of stale TEEP messages. “ To me, the following three headers shown in the draft are an overkill:
I would also add the following sentence: “ If the TAM does not receive the appropriate Content-Type and Accept header fields, the TAM SHOULD fail the request, returning a 406 (not acceptable) response. TAM responses MUST include a Content-Length header. “
**) The example gives me the impression that the protocol end indication is the 204 No Content. While I never saw it that way we should describe it more clearly if that’s indeed the case. (FWIW the note that the TEEP Agent can start with a QueryResponse if it has the TAM public key is IMHO incorrect.)
***) I don’t see any use for it.
****) I don’t see a need to use HTTP-based client authentication
*****) The text is currently very fuzzy. It says “Redirects MAY be automatically followed.” Based on what decision should a developer decide whether it wants to follow the redirect?