436949 - restore function pkix_trace_dump_cert and fix its leaks and logging

Status: NEW

(NSS :: Libraries, defect, P2)

Description

[Nelson Bolyard (seldom reads bugmail)]

A side effect of the final checkin for bug 397832 was that it ifdeffed 
out function pkix_trace_dump_cert and its callers in the source file 
libpkix/pkix/top/pkix_build.c .  It also removed a line of code from this
function that invoked the PKIX_ENTER macro.   This bug asks that that 
function be restored (no longer ifdeffed out) and that the problems with 
that function be fixed.

Function pkix_trace_dump_cert was written to aid in debugging.  It's intended to be a void function, having no effect on the code that calls it, and should just do some tracing.  The entry to and exit from this function should not 
be traced.  That is, the only tracing that should happen as a result of 
calling this function is the tracing that this function does conditionally.
When the variable pkixLog is NULL, the call to this function should not do
ANY logging at all.  It should be invisible (in the log files).

This function calls some PKIX functions that follow the libPKIX calling
conventions, returning PKIX_Error structures.  For some of those calls, this 
code leaks any returned PKIX_Errors, rather than assigning them to any local 
variables from which they can subsequently be freed. But this code uses the 
PKIX_TOSTRING and PKIX_DECREF macros which use those local variables, and so 
it invokes the PKIX_ENTER macro to setup all the variables used by those macros.  But the function does not call PKIX_RETURN at the end, so any PKIX_ERRORs returned by PKIX_DECREF are leaked. 

I suspect that the solution to this bug involves using the PKIX_ENTER and PKIX_RETURN macros INSIDE of a conditional block code code, e.g. something
like this:

function()
{
  if (pkixLog) {
    PKIX_ENTER()
    ...
cleanup:
    PKIX_RETURN();
  }
}

So that when PKIX logging is not enabled, this function is a complete no-op.

Alexei, I invite you to share ideas on how best to do this.

Comment 1

[Nelson Bolyard (seldom reads bugmail)]

Alexei, I am assigning to you.  
You don't need to fix it, but please offer your advice on how best to fix it.

Comment 2

[BugBot [:suhaib / :marco/ :calixte]]

The bug assignee is inactive on Bugzilla, and this bug has priority 'P2'.
:beurdouche, could you have a look please?

For more information, please visit auto_nag documentation.

Comment 3

[Marco Castelluccio [:marco]]

We have modified the bot to only consider P1 as high priority, so I'm cancelling the needinfo here.