Hi Sean,
On Fri, Oct 27, 2023 at 04:08:36PM +0100, Sean Whitton wrote:
> [attempt to resend to correct list address]
>
> Hello,
>
> I am trying to backport the fix for CVE-2020-25648 to nss version 3.42.1
> in Debian 10, "buster". Unfortunately, the four tests added by the fix
> do not pass.
>
> Would someone be able to take a look at the test output, please?
> Perhaps the failed test does not indicate that the fix is ineffective.
> As all the other tests pass, I'm not too concerned about introducing any
> regressions.
>
It seems your backported patch might be faulty. The upstream patch [0]
contains the following change:
ss->ssl3.hs.ws != idle_handshake &&
cText->buf->len == 1 &&
cText->buf->buf[0] == change_cipher_spec_choice) {
- /* Ignore the CCS. */
- return SECSuccess;
+ if (ss->ssl3.hs.allowCcs) {
+ /* Ignore the first CCS. */
+ ss->ssl3.hs.allowCcs = PR_FALSE;
+ return SECSuccess;
+ }
+
+ /* Compatibility mode is not negotiated. */
+ alert = unexpected_message;
+ PORT_SetError(SSL_ERROR_RX_MALFORMED_CHANGE_CIPHER);
}
if ((IS_DTLS(ss) && !dtls13_AeadLimitReached(spec)) ||
However, your patch has this instead:
cText->buf->buf[0] == change_cipher_spec_choice) {
/* Ignore the CCS. */
return SECSuccess;
+ if (ss->ssl3.hs.allowCcs) {
+ /* Ignore the first CCS. */
+ ss->ssl3.hs.allowCcs = PR_FALSE;
+ return SECSuccess;
+ }
+
+ /* Compatibility mode is not negotiated. */
+ alert = unexpected_message;
+ PORT_SetError(SSL_ERROR_RX_MALFORMED_CHANGE_CIPHER);
}
if (IS_DTLS(ss) ||
It appears that the code which was added is never reached because the
unconditional 'return SECSuccess;' was not pushed down into the new 'if'
statement.
I tried a few times to run the tests, but I couldn't manage it. So, my
theory is untested, but based on the fact that your failing test output
includes
Got error code (null) expecting SSL_ERROR_RX_MALFORMED_CHANGE_CIPHER
Since it seems like the function is returning SECSuccess and that that
added code is where the SSL_ERROR_RX_MALFORMED_CHANGE_CIPHER value is
set, all of that makes me think that my theory is a likely explanation
for your unexpected test failure.
Regards,
-Roberto
[0]
https://hg.mozilla.org/projects/nss/rev/57bbefa793232586d27cee83e74411171e128361
--
Roberto C. Sánchez
