Last Comment Bug 323570 - Make dbck Debug mode work with Softoken
: Make dbck Debug mode work with Softoken
Status: RESOLVED FIXED
:
Product: NSS
Classification: Components
Component: Tools (show other bugs)
: 3.0
: All All
: P3 normal (vote)
: 3.12
Assigned To: Nelson Bolyard (seldom reads bugmail)
: Jason Reid
Mentors:
Depends on:
Blocks: 47940
  Show dependency treegraph
 
Reported: 2006-01-15 19:00 PST by Nelson Bolyard (seldom reads bugmail)
Modified: 2006-04-24 19:23 PDT (History)
2 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
patch part 1 - for softoken (7.33 KB, patch)
2006-01-16 17:13 PST, Nelson Bolyard (seldom reads bugmail)
rrelyea: review+
Details | Diff | Review
patch part 2 - dbck changes (81.25 KB, patch)
2006-01-16 17:26 PST, Nelson Bolyard (seldom reads bugmail)
rrelyea: review+
Details | Diff | Review
Alternative patch part 2 (83.19 KB, patch)
2006-01-18 15:37 PST, Nelson Bolyard (seldom reads bugmail)
no flags Details | Diff | Review

Description Nelson Bolyard (seldom reads bugmail) 2006-01-15 19:00:27 PST
The dbck program has two modes: "Debug" and "recover".
The Debug (or Dump) mode merelys dumps the DB contents, and displays an
analysis of problems found.  The recover mode attempts to reconstruct a
valid cert DB from the recoverable parts of the old DB.  

Bug 47940 requests that Recover mode be made to work with softoken.
This bug requests that Debug mode be made to work with softoken.
Comment 1 Nelson Bolyard (seldom reads bugmail) 2006-01-16 17:13:23 PST
Created attachment 208687 [details] [diff] [review]
patch part 1 - for softoken

This patch is a prerequisite to the forthcoming dbck patch. 
It provides the changes needed in softoken for dbck.
This patch adds a new function to libsoftoken, and corrects the certDBEntry
union to be a union of ALL the cert DBEntry structs.
The new function can decode any of the current entry types.  
It's designed to be used by callbacks called by nsslowcert_TraverseDBEntries().
Bob, please review.
Comment 2 Nelson Bolyard (seldom reads bugmail) 2006-01-16 17:26:16 PST
Created attachment 208690 [details] [diff] [review]
patch part 2 - dbck changes

dbck originally had two functional modes: debug and recovery. The recovery 
code has been ifdeffed out for a long time, and will need much rework.  
This patch moves most of the recovery code into a new separate file.
It makes debug mode work properly.  I have tested it on several cert DBs.
This patch requires the other patch, to lib/softoken, as a prerequisite.
Bob, please review, although this is obviously not high priority.
Comment 3 Nelson Bolyard (seldom reads bugmail) 2006-01-16 21:06:33 PST
Here is sample output from the command 
dbck -d <mydir> -D -m -a
using my cert DB from my mozilla profile at home:

Database statistics:
N0: Found  220 Certificate entries.
N1: Found  203 Subject entries (unique DN's).
N2: Found  220 Cert keys within Subject entries.
N3: Found  125 Nickname entries.
N4: Found   87 S/MIME entries.
N5: Found    5 CRL entries.

E0: Found    0 Certificate entries that had no subject entry.
E1: Found    0 Subject entries with no corresponding Certificate entries.
E2: Found    8 Subject entries that had no nickname or S/MIME entries.
E3: Found    0 Redundant nicknames (subjects with the same nickname).
E4: Found    0 Subject entries that had no nickname entry.
E5: Found    0 Redundant email addresses (subjects with the same email address).

E6: Found    0 Subject entries that had no S/MIME entry.
E7: Found    0 Nickname entries that had no subject entry.
E8: Found    1 S/MIME entries that had no subject entry.
E9: Found   16 Subject entries with BOTH nickname and S/MIME entries.
--------------
    Found   25 errors in database.

Certificates:
N0 == N2 + E0 + E1
220 == 220 + 0 + 0

Subjects:
N1 == N3 + N4 + E2 + E3 + E4 + E5 + E6 - E7 - E8 - E9
203 == 125 + 87 +  8 +  0 +  0 +  0 +  0 -  0 -  1 - 16
Comment 4 Robert Relyea 2006-01-17 10:28:20 PST
Comment on attachment 208687 [details] [diff] [review]
patch part 1 - for softoken

r+ with the following comments:

The inclusion of lowkeyti.h does not appear to be releated to the rest of this patch.
Comment 5 Nelson Bolyard (seldom reads bugmail) 2006-01-17 15:56:41 PST
Thanks for the review Bob.

The inclusion of lowkeyti.h is related to this patch.  dbck.c now includes pcert.h.  There's a function declaration in pcert.h that returns a pointer 
to type NSSLOWKEYPublicKeyStr.  If you include pcert.h without including 
lowkeyti.h, you get a syntax error in pcert.h at that declaration. 

As you know, I believe that each of our header files should be able to 
"stand alone", that is, it should #include any dependencies it has, so
that a .c file can #include it, and not have to go find all its dependencies.
So I added that #include to pcert.h rather than to dbck.c.
Comment 6 Robert Relyea 2006-01-17 16:11:48 PST
Ok, so it fixes a bug in pcert.h. It's fine to go in then.
Comment 7 Robert Relyea 2006-01-17 17:09:44 PST
Comment on attachment 208690 [details] [diff] [review]
patch part 2 - dbck changes

r+ with the following comments:

very minor nit: Printf presents -R as an option even if DORECOVER is not defined (just before printing the usage message.

General: Most of this code is likely 'going away' in the shared db. It will stay around as a way to access legacy databases. This work is still useful, however, since 1) it will help diagnose problems with current and old databases, 2) it's not clear when the shared database work will go in (I'm targeting 3.12, but it may not go in until 3.13). 3) This code will still be around for access to old databases.

I just wanted to set the expectations.

bob
Comment 8 Nelson Bolyard (seldom reads bugmail) 2006-01-18 15:37:16 PST
Created attachment 208913 [details] [diff] [review]
Alternative patch part 2

This slightly updated patch part 2 also parses the PKCS11 profile data,
which the previous patch dumped in hex.
Comment 9 Nelson Bolyard (seldom reads bugmail) 2006-01-18 18:10:53 PST
Part 1 checked in on trunk.
Checking in pcert.h;   new revision: 1.12; previous revision: 1.11
Checking in pcertdb.c; new revision: 1.54; previous revision: 1.53
Checking in pcertt.h;  new revision: 1.14; previous revision: 1.13
Comment 10 Nelson Bolyard (seldom reads bugmail) 2006-01-21 22:56:12 PST
Checked in on trunk.

Checking in Makefile;    new revision: 1.4; previous revision: 1.3
Checking in dbck.c;      new revision: 1.8; previous revision: 1.7
Checking in dbrecover.c; initial revision: 1.1
Checking in manifest.mn; new revision: 1.5; previous revision: 1.4
Comment 11 Nelson Bolyard (seldom reads bugmail) 2006-04-24 00:01:06 PDT
Bob, Wan-Teh, I want to carry this fix back to the 3.11 branch.
Any objections?
Comment 12 Robert Relyea 2006-04-24 13:15:43 PDT
No objection. the patch is very safe.

bob
Comment 13 Nelson Bolyard (seldom reads bugmail) 2006-04-24 19:23:19 PDT
Backported from trunk
Checking in pcert.h;   new revision: 1.11.2.2; previous revision: 1.11.2.1
Checking in pcertdb.c; new revision: 1.53.2.7; previous revision: 1.53.2.6
Checking in pcertt.h;  new revision: 1.13.30.1; previous revision: 1.13

Note You need to log in before you can comment on or make changes to this bug.