Last Comment Bug 371470 - vfychain needs option to verify for specific date
: vfychain needs option to verify for specific date
Status: RESOLVED FIXED
:
Product: NSS
Classification: Components
Component: Tools (show other bugs)
: 3.11.6
: All All
: P3 enhancement (vote)
: 3.12
Assigned To: Nelson Bolyard (seldom reads bugmail)
:
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2007-02-23 19:19 PST by Nelson Bolyard (seldom reads bugmail)
Modified: 2007-09-26 07:27 PDT (History)
3 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
patch v1 - also corrects the usage message (7.30 KB, patch)
2007-02-23 20:23 PST, Nelson Bolyard (seldom reads bugmail)
neil.williams: review+
julien.pierre: superreview+
Details | Diff | Splinter Review

Description Nelson Bolyard (seldom reads bugmail) 2007-02-23 19:19:05 PST
vfychain only verifies the chain for time = now.
It needs an option to verify for a user-specified time.
I propose to give it a -b date option, like certutil has.
The syntax would be:  -b CCYYMMDDHHMM[timezone]
E.g. -b 200511011200Z
Comment 1 Nelson Bolyard (seldom reads bugmail) 2007-02-23 20:23:50 PST
Created attachment 256250 [details] [diff] [review]
patch v1 - also corrects the usage message

Please review.
Comment 2 Nelson Bolyard (seldom reads bugmail) 2007-03-13 22:49:34 PDT
Committed on bypass branch:
nss/cmd/lib/secutil.c;       new revision: 1.71.2.3.8.1;  previous: 1.71.2.3
nss/cmd/lib/secutil.h;       new revision: 1.18.24.1.8.1; previous: 1.18.24.1
nss/cmd/vfychain/vfychain.c; new revision: 1.9.70.1;      previous: 1.9
Comment 3 Nelson Bolyard (seldom reads bugmail) 2007-04-25 19:06:58 PDT
Comment on attachment 256250 [details] [diff] [review]
patch v1 - also corrects the usage message

requesting second review for branch.
Comment 4 Julien Pierre 2007-04-25 19:27:28 PDT
Nelson,

I believe the changes for the time are fine, but I think this change for the usage is incorrect :

+	case 'u' : usage    = PORT_Atoi(optstate->value);
+	           if (usage < 0 || usage > 62) Usage(progName);
+		   certUsage = ((SECCertificateUsage)1) << usage; break;

I may be nitpicking here, but checking for against usage>62 only prevents you from having an overflow of usage, but doesn't guarantee its range validity.

You should also check that the resulting usage is <= certificateUsageHighest .
Unfortunately, that value is a compile macro, and not dynamically obtained from the shared library, so if new usages are introduced in newer libraries, the program would have to be recompiled to allow specifying the new usage. Maybe a function should be introduced in libnss in the future that dynamically returns the value. But if not, the extra check I suggested should be sufficient.
Comment 5 Julien Pierre 2007-04-25 19:34:26 PDT
We don't generally ship vfychain to customers - it is used for QA only, so it is very unlikely to be run against anything other than libraries from the same source tree. Thus the fact that certificateUsageHighest is static is no big deal. Therefore, just checking that certUsage is is <= certificateUsageHighest should be enough.
Comment 6 Julien Pierre 2007-04-26 18:38:22 PDT
Comment on attachment 256250 [details] [diff] [review]
patch v1 - also corrects the usage message

r+ with this change :

certUsage = ((SECCertificateUsage)1) << usage; 

+if (certUsage>certificateUsageHighest) Usage(progName);

break;
Comment 7 Nelson Bolyard (seldom reads bugmail) 2007-05-02 21:22:41 PDT
Committed on trunk (with Julien's suggested change)

Checking in vfychain.c; new revision: 1.10; previous revision: 1.9
Checking in secutil.c;  new revision: 1.76; previous revision: 1.75
Checking in secutil.h;  new revision: 1.20; previous revision: 1.19
Comment 8 Biswatosh Chakraborty 2007-09-26 07:27:08 PDT
*** Bug 231158 has been marked as a duplicate of this bug. ***

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