(D)TLS RFI and issues

[kr15tyk]

RFI202308 https://ccusersforum.onlyoffice.co/Products/Files/DocEditor.aspx?fileid=8581666&action=view

Issue #327 https://github.com/ND-iTC/Documents/issues/327

Issue #316 https://github.com/ND-iTC/Documents/issues/316

Issue #317 https://github.com/ND-iTC/Documents/issues/317

Issue #318 https://github.com/ND-iTC/Documents/issues/318

[kr15tyk]

@plughy2 @kenji-lightship @mclearn Please provide your review for your requested changes. Thank you!

[mclearn]

Have confirmed the changes made for RFI202308 appear to be as per the NIT resolution.

Have confirmed that #316 seems to be incorporated (with the one change that the original wording requested "shall select" and the new text is written as "should select" due to, IIRC from another conversation, "shall" statements should not appear in the App Notes).

Have confirmed that #317 seems to be incorporated. A note: Item #316/#317 implies that the signature_algorithms extension is mandatory for the PP's use. However, the PP SFR element has left it as a selection. At least one needs to be selected since there is no "none" option. Of the two options "signature_algorithms" or "signature_algorithms_cert", the former is mandatory and the latter is optional. If the latter is not provided, then the same algorithms from the former apply to the latter. (See RFC 8446 section 4.2.3.) Technically, it would have been better to ensure that only "signature_algorithms_cert" was the only possible selection [making signature_algorithms mandatory]. But the effort to edit outweights the nitpick.

For #318, I pushed a commit up to address what I think does a good job to unify the requirements. See https://github.com/ND-iTC/Documents/pull/330/commits/51e2385294e9b02af13c454ece8bdd8acc52882a. @plughy2 can you examine and see if this seems okay? Note that the inclusion of "signature_algorithms_cert" is necessary since there is complicated interplay between the "signature_algorithms_cert" and "signature_algorithms" when it comes to how signatures are checked. But RFC 8446 section 4.3.2 says that a compliant solution will send at least "signature_algorithms" and possibly "signature_algorithms_cert" -- in which case the latter takes precedence. The purpose of the test -- which is reasonably close to previous NDcPP iterations -- is that we show that a cert with the correct signature can be accepted. The "signature_algorithms" -- if presented alone -- does double-duty: it is used for the algorithm in the CertiifcateVerify message, and also for the permitted certificate signatures. Finally, I briefly considered the term "chain" in (new) test 3, since it kind of implies the chain validation occurring within FIA_X509_EXT.1/Rev or /ITT. But I didn't want to complicate matters within this test case by trying to define the chain length.

I should mention that I have tested OpenSSL 3.x in the past and have made similar comments on the TLS FP 2.0 to NIAP that negative tests for "signature_algorithms" and "signature_algorithms_cert" are likely to fail because of the complex and historical interplay between those two extensions and to which parts of the handshake or certificate they apply to. For this reason, negative testing should not be included.

[mclearn]

Given that I am the only approved reviewer on file, someone other than me/Lightship should approve the changes I just pushed.

[plughy2]

I reviewed Greg's commit and found it looks great to me. I approve.

[kr15tyk]

Thank you @plughy2 @mclearn for taking us to the finish line on this.

[mclearn]

I'm having second thoughts about #317 test 3. Specifically, in TLS 1.2, because of the complex interplay b/w the signatureAndHash and the CertificateType, I think the test case might be slightly wrong for TLS 1.2.

The CertificateRequest message contains two things: a certificate_types list and a supported_signature_algorithms list. The signature algorithms are supposed to be used for cert signature checking, but in OpenSSL they are not (there's a known issue that tracks it that I can't find at the moment). I think the gist of the OpenSSL issue is that the RFC is ambiguous under which circumstances each list is supposed to be used for which purpose. I think OpenSSL uses the supported_signature_algorithms for the CertificateVerify message and the certificate_types for the actual cert that is allowed to go across. But for the certificate signature, that implies knowing the public key type for the parent cert, which -- and I'm grasping at faint memories here -- OpenSSL claimed is not known at the time. Therefore, the best OpenSSL is able to do is examine the certificate_types to ensure the over-the-wire cert matches the request. This is why negative testing (especially with OpenSSL 1.x), failed, IIRC.

The test case I wrote may not actually be accurate. I think it might need a slight rewrite to be "correct" -- even though the net result will be the same.

Test 3: The evaluator shall establish a DTLS connection and perform client authentication with a certificate chain using each of the SignatureSchemes (DTLS 1.3) or ClientCertificateType (DTLS 1.2) specified by the requirement. For DTLS 1.3, the evaluator shall ensure the signature used in the certificate is consistent with the value being tested. For DTLS 1.2, the evaluator shall ensure the certificate public key algorithm is consistent with the value being tested.

[mclearn]

I believe this is the link to the OpenSSL issue: https://github.com/openssl/openssl/issues/11438. Net-net: since the sum total of testing in the SD is +ve testing, it doesn't really matter. I think the current test wording is probably more RFC-compliant, but as soon as -ve testing is added, OpenSSL will have issues. So maybe just leave as-is and know that this may need to be revisited by RFI/TRRT in the future.