generate_certs.sh shouldn't clobber existing certs that don't need to change

RESOLVED FIXED in Firefox 33

Status

()

Core
Security: PSM
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: keeler, Assigned: keeler)

Tracking

unspecified
mozilla34
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox32 wontfix, firefox33 fixed, firefox34 fixed)

Details

Attachments

(2 attachments, 1 obsolete attachment)

(Assignee)

Description

4 years ago
See bug 1034124 comment 9 and bug 1009161 comment 18, wherein it is pointed out that additions to generate_certs.sh that involve things like adding tests shouldn't affect other tests like pinning (if everything gets regenerated, then the key hash for the pinning tests need to be updated, which involves re-generating the built-in key pinning data, which is unnecessary).
(Assignee)

Comment 1

4 years ago
Created attachment 8477100 [details] [diff] [review]
patch

Richard, do you have time to review this? Thanks. (This is the script that generates certificates for our tlsserver-involved xpcshell tests, if you weren't already familiar with it.)
Assignee: nobody → dkeeler
Status: NEW → ASSIGNED
Attachment #8477100 - Flags: review?(rlb)
Comment on attachment 8477100 [details] [diff] [review]
patch

Review of attachment 8477100 [details] [diff] [review]:
-----------------------------------------------------------------

Couple of minor things.

::: security/manager/ssl/tests/unit/tlsserver/generate_certs.sh
@@ +35,5 @@
> +if [ "${3}" == "--clobber" ]; then
> +  CLOBBER=1
> +else
> +  CLOBBER=0
> +fi

Might be slightly clearer to reorder as 

> CLOBBER=0
> if [...]; then CLOBBER=1

@@ +72,5 @@
> +  if [ $CLOBBER -eq 1 ]; then
> +    echo "Found pre-existing NSS DBs. Clobbering old OCSP certs."
> +    rm -f "$OUTPUT_DIR/cert9.db" "$OUTPUT_DIR/key4.db" "$OUTPUT_DIR/pkcs11.txt"
> +    $RUN_MOZILLA $CERTUTIL -d $DB_ARGUMENT -N -f $PASSWORD_FILE
> +  fi

Should there be a message here (as below) to let the user know that old stuff is persisting?
Attachment #8477100 - Flags: review?(rlb) → review+
(Assignee)

Comment 3

4 years ago
Created attachment 8477510 [details] [diff] [review]
patch with comments addressed

Great - thanks! Addressed comments, carrying over r+.
https://hg.mozilla.org/integration/mozilla-inbound/rev/0a25b15c2f8f
Attachment #8477100 - Attachment is obsolete: true
Attachment #8477510 - Flags: review+
https://hg.mozilla.org/mozilla-central/rev/0a25b15c2f8f
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla34
(Assignee)

Comment 5

4 years ago
Created attachment 8483766 [details] [diff] [review]
patch for beta (33)

This is needed for bug 1009161.
Attachment #8483766 - Flags: review+
(Assignee)

Updated

4 years ago
status-firefox33: --- → affected
status-firefox34: --- → fixed
Comment on attachment 8483766 [details] [diff] [review]
patch for beta (33)

[Triage Comment]
Attachment #8483766 - Flags: approval-mozilla-beta+
https://hg.mozilla.org/releases/mozilla-beta/rev/3f1e228fac54
status-firefox32: --- → wontfix
status-firefox33: affected → fixed
You need to log in before you can comment on or make changes to this bug.