Closed Bug 371470 Opened 17 years ago Closed 17 years ago

vfychain needs option to verify for specific date

Categories

(NSS :: Tools, enhancement, P3)

3.11.6
enhancement

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: nelson, Assigned: nelson)

Details

Attachments

(1 file)

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
Please review.
Attachment #256250 - Flags: review?(neil.williams)
Attachment #256250 - Flags: review?(neil.williams) → review+
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
Status: NEW → ASSIGNED
Comment on attachment 256250 [details] [diff] [review]
patch v1 - also corrects the usage message

requesting second review for branch.
Attachment #256250 - Flags: superreview?(julien.pierre.boogz)
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.
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 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;
Attachment #256250 - Flags: superreview?(julien.pierre.boogz) → superreview+
Priority: -- → P3
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
Status: ASSIGNED → RESOLVED
Closed: 17 years ago
Resolution: --- → FIXED
Target Milestone: 3.11.7 → 3.12
You need to log in before you can comment on or make changes to this bug.