Jim, Thank you for this very useful review. Please find our comments inline:
Jim Schaad <[email protected]> writes: > Section 3 - Am I just missing where you talk about introspection or is it > missing from the text? The role of token introspection for authorization is discussed in draft-ietf-ace-oauth-authz which is referenced here as the governing mechanism. The DTLS profile is completely agnostic in this regard as you may or may not use token introspection on access tokens retrieved via the DTLS profile. > Section 3.2 - looks like you missed a "cnf" in the first paragraph - should > be req_cnf Fixed. > Section 3.3 - Figure 4 - Where is the 'alg' parameter defined at that level? See next comment. > Section 3.3 - I am always bothered by the fact that PSK should really be PSS > at this point. The secret value is no longer a key and thus does not > necessarily have a length. There is also a problem of trying to decide what > the length of this value would be based on the algorithm. If the client > offers TLS_PSK_WITH_AES_128_CCM_8 and TLS_PSK_WITH_AES_256_CCM_8 (I may > have gotten these wrong but the intent should be understandable) then what > length is the PSK supposed to be? I think what you are saying is that for the shared secret (k) in the COSE_Key structure in Fig. 4, the AS needs to tell C what to do with that shared secret? This was the intention of the alg parameter (which has a not-so-useful value in this example). > Section 3.3 - Figure 5 - Is this defining a new set of entries for cnf or is > there a missing layer someplace? This is indeed a new set of entries. An internal discussion between the draft authors led to the opinion that we cannot use a COSE_Key structure in the cnf structure as this would have different semantics as intended here. We came to the conclusion that for key derivation, listing the required fields directly in the cnf structure is the cleanest solution. > Section 3.3 - Should some type of key identifier also be defined so that the > entire token is not always sent on every DTLS setup? Also so that the key > can be referenced on a second request to the AS. How about adding the following to the paragraph on the cnf in the access information at the end of the second paragraph in Section 3.3: OLD: "AS adds a cnf parameter to the access information carrying a COSE_Key object that informs the client about the symmetric key that is to be used between C and the resource server." NEW: "AS adds a cnf parameter to the access information carrying a COSE_Key object that informs the client about the symmetric key that is to be used between C and the resource server. To enable dynamic updates of the access token, the AS SHOULD add a kid field with a random identifier for the symmetric key that is conveyed by the cnf object." > Section 3.4 - Is it worth while to make it explicit when the DTLS session is > required to be shutdown and that a simple authorization error is not one of > them? Looks like it should be - these errors SHOULD NOT cause the DTLS > connection to be closed. How about this: "Unauthorized requests that have been received over a DTLS session should be treated as non-fatal by the RS, i.e., the DTLS session should be kept alive until the associated access token has expired." > Section 4 - The requirement that the "kid" be in a previously issued token > would have problems with this if you were to use the "kid" of an RPK which > can be constant rather than being changed on each newly created token. To solve this we could require that the COSE_Key structure for RS's RPK in the AS-to-Client response must also have a kid. Section 4 does not prevent the kid to be constant. > Section 4 - The term "must associate" is slightly ambiguous - does this mean > add to (augment) or replace the previous token. How about: OLD: "A resource server MUST associate the updated authorization information with any existing DTLS session that is identified by this key identifier. NEW: "A resource server MUST replace the authorization information of any existing DTLS session that is identified by this key identifier with the updated authorization information." (Effectively allowing only one access token per session at a time.) > Section 4 - Should a secondary access token have a later expiration date > that the one that contains the actual key value? I suppose yes but this should be solved in general by the framework. > Section 5 - this seems to be a very strange place to define the new > parameter. True. How about moving this parameter to Section 4? > Section 8 - you need to put your kid in the IANA considerations as well Fixed. > Section 9.1 - Having tilicoa-tls-dos-handshake as a normative reference may > cause problems unless this is on a publication track. You are right. After speaking to the main author, we came to the conclusion that we should remove this reference entirely as draft-tilocoa-tls-dos-handshake will not be pursued any more. > Section 9.3 - Can we get these URLs converted over to normal references? This section somehow is generated from the XML by the submission tool (in the gh-pages version [1] you have clickable links in-place which is very convenient for editing). Once the framework document's structure is stable, we will convert them back to normal references. [1] https://ace-wg.github.io/ace-dtls-profile/ Grüße Olaf _______________________________________________ Ace mailing list [email protected] https://www.ietf.org/mailman/listinfo/ace
