Hi Paul, Thank you for the review. I've put up a PR to address your points: https://github.com/tlswg/tls-subcerts/compare/nick/wouters-review-may22?expand=1
Comments inline. On Mon, May 9, 2022 at 9:03 PM Paul Wouters <paul.wout...@aiven.io> wrote: > > As this document already saw an AD review by Ben Kaduk, I will not further > hold up this document. Ben's write up can be found at: > https://mailarchive.ietf.org/arch/msg/tls/qa908s0dMNxbUOZhnUel0qEVBSg/ > > Please treat the below as you would treat any other comments. > > Test vectors available but still not included - did these get verified ? > Can these be included? > The current vectors are out of date. We're working on getting these updates in the PR (https://github.com/tlswg/tls-subcerts/pull/77). > > expected_cert_verify_algorithm - why is this not called > dc_cert_verify_algorithm or dc_verify_algorithm ? > As in, why is there an "expected" prefix? We talk about > signature_algorithms and not expected_signature_algorithms ? > I'm fine with dc_cert_verify_algorithm and have updated it. > > algorithm: The signature algorithm used to verify > DelegatedCredential.signature. > > This reads weird. If the signature algorithm is "used", it was to create > the signature. > The verification is in the future though. Perhaps say "The signature > algorithm used to > create the DelegatedCredential.signature" > Ok, changed. > > > 1. A string that consists of octet 32 (0x20) repeated 64 times. > > - Why is there a 64 spaces prefix ? > - Should it say a non-null terminated string? Or perhaps "octet stream" > instead of "string" ? > It's a technique for domain separation that was taken from TLS 1.3 (4.4.3. Certificate Verify) to prevent chosen-prefix attacks ( https://github.com/tlswg/tls13-spec/commit/c3902e65). The initial bytes cover an entire hash block before the domain separator. This text was taken directly from TLS 1.3, so the ambiguity remains there too, but I'm happy to change this text to be more explicit. > > > 2. The context string "TLS, server delegated credentials" for server > authentication and "TLS, client delegated credentials" for client > authentication. > > - Same here, non-null terminated string? > > 3. A single 0 byte, which serves as the separator. > > a few lines up it used hex notation for 0x20 and named it octet. Should > this be called > a single octet of value 0x00 ? > This text is taken directly from TLS 1.3 also, but it can be made less ambiguous. > > A client which supports this specification SHALL send a > "delegated_credential" extension in its ClientHello. > > This oddly means that a client that supports this cannot really choose to > not use it. Normally, this is more written as > "A client that is willing to use DC includes a "delegated_credential" > extension in its ClientHello" > Agreed. Re-phrased. > > If the client receives a delegated credential without having > indicated support in its ClientHello > > According to the above, this "SHALL" not happen because to recognise the > extension it needs to support it and if it supports it, it SHALL send it :) > With the new phrasing above this now makes sense. The client can support DCs but not be willing to use it in a given connection, and in this case if the server sends it the connection should fail. However, this is the default behavior in TLS when a client receives an unsolicited extension, so it may still be a bit redundant. > > The server MUST ignore the extension unless > (D)TLS 1.3 or a later version is negotiated. > > That is oddly phrased as a MUST with an exception (which is really a > SHOULD) > Perhaps: "When a (D)TLS version negotiated is less than 1.3, the server > MUST ignore this extension" > Yes, this is better. > > The client MUST ignore the extension unless > (D)TLS 1.3 or a later version is negotiated. > > Same here. > Done > > The server MUST send the delegated credential > > Should that be "delegated_credential", eg underscore instead of space? Or > use DC ? > > We'll use DC. > > the server SHOULD > ignore delegated credentials sent as extensions to any other > certificate. > > Why is the case a "SHOULD ignore" where all the other cases of unexpected > DC uses "MUST <some fatal error>" ? > This draft doesn't speak to the use of DCs in the certificate chain or modify the behavior of TLS with respect to certificate chain validation. Adding a requirement to change behavior based on the certificate chain may require implementers to modify PKIX libraries, which this draft currently does not do. > > > I'm a little confused by Section 7.4. Interactions with Session > Resumption. > The session resumption uses a seperate continue mechanism that omits > needing to re-authenticate the peer. It has its own > session ticket lifetimes to keep its state. Why is that mechanism on its > own not enough? If the server still has the session state > why would a valid DC be a requirement to use an existing ticket? Or > phrased differently, one could say that a session resumption > ticket lifetime should never fall outside the DC validity period (if that > is what you want to enforce, but I'm not sure that is what > you want or should do). > The session resumption mechanism is under-defined in TLS 1.3 and server implementers often do not follow the recommendations that set ticket lifetimes based on the expiration of the certificate or dynamically at all. RFC 8447 Section 4.6.1. > It is RECOMMENDED that implementations place limits on the total lifetime of such keying material; these limits should take into account the lifetime of the peer’s certificate, the likelihood of intervening revocation, and the time since the peer’s online CertificateVerify signature. To mitigate this, RFC 8447 sets a maximum lifetime for session tickets (both in the ticket and in the client cache) to 7 days. This allows resumptions for up to 7 days after the connection has expired. To prevent this, some client implementations cache the certificate lifetime and only resume when the certificate has not expired but given the typical lifetime of a certificate, this is a relatively small window. DC are short-lived by default, so resuming 7 days after expiration could effectively double or more the lifetime of connections from a DC. Section 7.4 section is a reminder that client-side enforcement of resumption timelines based on certificate expiration alone is not sufficient in the case where a DC is used. > NITS: > > Please fix nits as per genart review: > https://datatracker.ietf.org/doc/review-ietf-tls-subcerts-12-genart-lc-davies-2022-04-08/ > Addressed by the chairs: https://github.com/tlswg/tls-subcerts/pull/103/files > > > Paul >
_______________________________________________ TLS mailing list TLS@ietf.org https://www.ietf.org/mailman/listinfo/tls
