642697 - Incomplete/confusing error handling in nsNSSBadCertHandler

Status: RESOLVED DUPLICATE of bug 674147

(Core :: Security: PSM, defect)

Description

[Brian Smith (:briansmith, :bsmith, use NEEDINFO?)]

See bug 642395 comment 36 and the following comments in that bug. 

The code is:

  if (!collected_errors)
  {
    NS_NOTREACHED("why did NSS call our bad cert handler if \
                   all looks good? Let's cancel the connection");
    return SECFailure;
  }

The assertion is reached when the fix for bug 642395 is used and (probably) when a revoked certificate has been encountered, so the NS_NOTREACHED assertion is invalid.

Additionally, all other return statements are of the form:

   nsHandleSSLError(infoObject, i_node->error);
   PR_SetError(...);
   return cancel_and_failure(infoObject);

If nsHandleSSLError and/or cancel_and_failure are not needed, then the code should be documented with a note as to why they are not needed in this specific case.

Comment 1

[Brian Smith (:briansmith, :bsmith, use NEEDINFO?)]

Fixed in the patch for bug 674147.