Closed Bug 363987 Opened 18 years ago Closed 18 years ago

crlutil does not change thisUpdate date when creating a modified CRL

Categories

(NSS :: Tools, defect, P1)

3.11
defect

Tracking

(Not tracked)

RESOLVED FIXED
3.11.5

People

(Reporter: alvolkov.bgs, Assigned: alvolkov.bgs)

References

Details

Attachments

(1 file, 1 obsolete file)

The date should be reset to PR_Now. User should use "update <date>" crl generation script command to set a specific thisUpdate date.

crl generation tests needed to be modified to explicitly use "update" command to Zulu and local time differences.
Attached patch reset time of modified crl (obsolete) — Splinter Review
Attachment #248767 - Flags: review?(nelson)
Alexei, this patch (attachment 248767 [details] [diff] [review]) patches two files.  The name of one of
the two files begins with ../ which really confuses bugzilla's attachment.cgi 
patch diffing tool.  That makes it difficult for me to review this patch.
So please submit a new version of your patch where the file names all start
in mozilla/security and don't have any ../ in them.  Thanks.

I need to understand some more about crlutil in order to properly review this.
I hope to ask you some questions about it, face to face.

I'd also like you to explain the remark about Zulu and local time differences
and its impact on this issue. 
Summary: crlutil does not change thisUpdate date when modifies a crl → crlutil does not change thisUpdate date when creating a modified CRL
Comment on attachment 248767 [details] [diff] [review]
reset time of modified crl

r= only because of this:
>Index: ../cmd/crlutil/crlutil.c
which prevents me from doing more review.
Out of curiosity, how did you create that?
Did you do "cvs diff -u ../cmd/crlutil/crlutil.c" ?
Attachment #248767 - Flags: review?(nelson) → review-
(In reply to comment #2) where I wrote:
> I need to understand some more about crlutil in order to properly review this.

I got most of my answers in the crlutil documentation page at 
   http://www.mozilla.org/projects/security/pki/nss/tools/crlutil.html
Good documentation!  
> >Index: ../cmd/crlutil/crlutil.c
> which prevents me from doing more review.
> Out of curiosity, how did you create that?
> Did you do "cvs diff -u ../cmd/crlutil/crlutil.c" ?

Yes, this is exactly what I used. I always use relative names
when do diffs.
Apparently cvs can not resolve to full name relatively to cvs ws
if ".." are used in a file/dir path.
Attached patch corrected diffSplinter Review
Attachment #248767 - Attachment is obsolete: true
Attachment #249022 - Flags: review?(nelson)
Comment on attachment 249022 [details] [diff] [review]
corrected diff

r=nelson
Attachment #249022 - Flags: review?(nelson) → review+
Priority: -- → P1
Target Milestone: --- → 3.11.5
Version: unspecified → 3.11
Comment on attachment 249022 [details] [diff] [review]
corrected diff

Asking Neil for second review for the branch.
Attachment #249022 - Flags: review?(neil.williams)
Comment on attachment 249022 [details] [diff] [review]
corrected diff

Looks good. You might want to add a note about your face-to-face with Nelson so we know what the Zulu comment referred to.
Attachment #249022 - Flags: review?(neil.williams) → review+
/cvsroot/mozilla/security/nss/cmd/crlutil/crlutil.c,v  <--  crlutil.c
new revision: 1.29; previous revision: 1.28
/cvsroot/mozilla/security/nss/tests/cert/cert.sh,v  <--  cert.sh
new revision: 1.37; previous revision: 1.36
Status: NEW → RESOLVED
Closed: 18 years ago
Resolution: --- → FIXED
The fix was only commmitted on the trunk, not the branch, so this bug
is not yet resolved/fixed for the target release.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Committed on the 3.11 branch:

/cvsroot/mozilla/security/nss/cmd/crlutil/crlutil.c,v  <--  crlutil.c
new revision: 1.26.24.2; previous revision: 1.26.24.1
/cvsroot/mozilla/security/nss/tests/cert/cert.sh,v  <--  cert.sh
new revision: 1.28.2.7; previous revision: 1.28.2.6
Status: REOPENED → RESOLVED
Closed: 18 years ago18 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: