vfychain needs option to verify for specific date

RESOLVED FIXED in 3.12

Status

NSS
Tools
P3
enhancement
RESOLVED FIXED
10 years ago
10 years ago

People

(Reporter: Nelson Bolyard (seldom reads bugmail), Assigned: Nelson Bolyard (seldom reads bugmail))

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

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
(Assignee)

Comment 1

10 years ago
Created attachment 256250 [details] [diff] [review]
patch v1 - also corrects the usage message

Please review.
Attachment #256250 - Flags: review?(neil.williams)

Updated

10 years ago
Attachment #256250 - Flags: review?(neil.williams) → review+
(Assignee)

Comment 2

10 years ago
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
(Assignee)

Comment 3

10 years ago
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)

Comment 4

10 years ago
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

10 years ago
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

10 years ago
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+
(Assignee)

Updated

10 years ago
Priority: -- → P3
(Assignee)

Comment 7

10 years ago
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
Last Resolved: 10 years ago
Resolution: --- → FIXED
Target Milestone: 3.11.7 → 3.12

Updated

10 years ago
Duplicate of this bug: 231158
You need to log in before you can comment on or make changes to this bug.