Closed
Bug 248751
Opened 21 years ago
Closed 21 years ago
signtool should create XPInstall compatible signed archives
Categories
(NSS :: Tools, enhancement, P3)
Tracking
(Not tracked)
RESOLVED
FIXED
3.10
People
(Reporter: tagnarth, Assigned: nelson)
Details
Attachments
(2 files, 3 obsolete files)
13.86 KB,
patch
|
nelson
:
review+
|
Details | Diff | Splinter Review |
6.77 KB,
patch
|
wtc
:
review+
|
Details | Diff | Splinter Review |
User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7) Gecko/20040614 Firefox/0.8
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.7) Gecko/20040614 Firefox/0.8
Currently XPInstall only recognizes .xpi's as signed if the pkcs7 file is first
in the archive. Currently signtool places it dead last in the archive. This
causes a perfectly signed archive to appear as unsigned when an archive is being
installed through XPInstall.
Adding a new option to signtool that would arrange an archive in an XPInstall
compatible order would be a great benefit to extension developers. I have
created a patch that would add a new option -X. This option when used with -Z
only will create an archive that meets the expectations of XPInstall. If the -X
option is not set archives are created exactly the same as before.
Currently the patch is located at http://www.j-maxx.net/signtool_xpi.patch . I
will add it as an attachment in a followup post. It was created against
NSS_3_9_RTM with the command "cvs diff NSS_3_9_RTM signtool"
I have successfully compiled NSS with this patch on Windows XP and Linux. Both
compiles returned the same resulting archives.
Reproducible: Always
Steps to Reproduce:
1. Create a signed archive with signtool
signtool -d "certificate directory" -k "cert nickname" -p "password" -X -Z
"file.xpi" directory-tree
2. Load the file.xpi into Mozilla, FireFox, Thunderbird or NS 7.1 by dragging it
over the application. Thus triggering XPInstall. This can also be accomplished
via a web page. Example of compatible and in compatible signed archives here
http://www.j-maxx.net/signtool.php
Reporter | ||
Comment 1•21 years ago
|
||
This patch file introduces a new option into signtool . The -X option used in
conjunction with -Z will create an archive compatible with what XPInstall
expects for a signed archive. It does error checking to make sure when -X is
passed that -Z is also passed. If -X is not passed the resulting archive and
steps to create the archive are the same as the unpatched signtool.
The method used I know is not the best.(manifest.mf and zigbert.sf are created
twice). I have had 3 ideas on ways to implement the option and this was the
easiest and fastest. The other two would involve modifying multiple functions
instead of just one. I've tested this method and found when the -X option is
selected there is a 9% increase in completion time against a regular archive
creation. I expect that the other methods will also create a similar increase.
I'm willing to write out the other methods if so wished. They may take a bit to
flesh out.
Reporter | ||
Comment 2•21 years ago
|
||
Comment on attachment 151772 [details] [diff] [review]
Patch File to include new -X option for XPI archive creation
Index: signtool/README
===================================================================
RCS file: /cvsroot/mozilla/security/nss/cmd/signtool/README,v
retrieving revision 1.1
diff -r1.1 README
2c2
< 1.3 Release Notes
---
> 1.4 Beta Release Notes
9a10,18
> === New Features in 1.4 Beta
> =======================
> One new option (-X) has been added to create a Mozilla aware signed XPI archive.
> The option must be accompanied by the -Z option. This new option
> creates a JAR file with the META-INF/zigbert.rsa/dsa file as the first file in
> the archive instead of the default third to last. This will enable the archive
> to be seen as signed by products incorporating XPInstall. i.e. .xpi extensions
> for FireFox or Mozilla.
>
Index: signtool/sign.c
===================================================================
RCS file: /cvsroot/mozilla/security/nss/cmd/signtool/sign.c,v
retrieving revision 1.8
diff -r1.8 sign.c
76,85c76
< if(zip_file) {
< zipfile = JzipOpen(zip_file, NULL /*no comment*/);
< }
<
< manifesto (tree, install_script, recurse);
<
< if (keyName)
< {
< status = create_pk7 (tree, keyName, &keyType);
< if (status < 0)
---
> if(xpi_arc)
87,89c78,149
< PR_fprintf(errorFD, "the tree \"%s\" was NOT SUCCESSFULLY SIGNED\n",
tree);
< errorCount++;
< exit (ERRX);
---
> manifesto (tree, install_script, recurse);
> if(zip_file) {
> zipfile = JzipOpen(zip_file, NULL /*no comment*/);
> }
> if (keyName)
> {
> status = create_pk7 (tree, keyName, &keyType);
> if (status < 0)
> {
> PR_fprintf(errorFD, "the tree \"%s\" was NOT SUCCESSFULLY SIGNED\n", tree);
> errorCount++;
> exit (ERRX);
> }
> }
>
> /* If creating XPI compatible archive add rsa/dsa first
> then create manifest lists*/
>
> /* rsa/dsa to zip */
> sprintf (tempfn, "META-INF/%s.%s", base, (keyType==dsaKey ? "dsa" : "rsa"));
> sprintf (fullfn, "%s/%s", tree, tempfn);
> JzipAdd(fullfn, tempfn, zipfile, compression_level);
>
> manifesto (tree, install_script, recurse);
>
> /* mf to zip */
> strcpy (tempfn, "META-INF/manifest.mf");
> sprintf (fullfn, "%s/%s", tree, tempfn);
> JzipAdd(fullfn, tempfn, zipfile, compression_level);
>
> /* sf to zip */
> sprintf (tempfn, "META-INF/%s.sf", base);
> sprintf (fullfn, "%s/%s", tree, tempfn);
> JzipAdd(fullfn, tempfn, zipfile, compression_level);
>
> if(verbosity >= 0) {
> PR_fprintf(outputFD, "%s \n", XPI_TEXT);
> }
>
> } else { /* Create regularly signed archive */
>
> if(zip_file) {
> zipfile = JzipOpen(zip_file, NULL /*no comment*/);
> }
>
> manifesto (tree, install_script, recurse);
>
> if (keyName)
> {
> status = create_pk7 (tree, keyName, &keyType);
> if (status < 0)
> {
> PR_fprintf(errorFD, "the tree \"%s\" was NOT SUCCESSFULLY SIGNED\n", tree);
> errorCount++;
> exit (ERRX);
> }
> }
>
> /* mf to zip */
> strcpy (tempfn, "META-INF/manifest.mf");
> sprintf (fullfn, "%s/%s", tree, tempfn);
> JzipAdd(fullfn, tempfn, zipfile, compression_level);
>
> /* sf to zip */
> sprintf (tempfn, "META-INF/%s.sf", base);
> sprintf (fullfn, "%s/%s", tree, tempfn);
> JzipAdd(fullfn, tempfn, zipfile, compression_level);
>
> /* rsa/dsa to zip */
> sprintf (tempfn, "META-INF/%s.%s", base, (keyType==dsaKey ? "dsa" : "rsa"));
> sprintf (fullfn, "%s/%s", tree, tempfn);
> JzipAdd(fullfn, tempfn, zipfile, compression_level);
91,105d150
< }
<
< /* mf to zip */
<
< strcpy (tempfn, "META-INF/manifest.mf");
< sprintf (fullfn, "%s/%s", tree, tempfn);
< JzipAdd(fullfn, tempfn, zipfile, compression_level);
<
< /* sf to zip */
<
< sprintf (tempfn, "META-INF/%s.sf", base);
< sprintf (fullfn, "%s/%s", tree, tempfn);
< JzipAdd(fullfn, tempfn, zipfile, compression_level);
<
< /* rsa/dsa to zip */
107,109d151
< sprintf (tempfn, "META-INF/%s.%s", base, (keyType==dsaKey ? "dsa" :
"rsa"));
< sprintf (fullfn, "%s/%s", tree, tempfn);
< JzipAdd(fullfn, tempfn, zipfile, compression_level);
Index: signtool/signtool.c
===================================================================
RCS file: /cvsroot/mozilla/security/nss/cmd/signtool/signtool.c,v
retrieving revision 1.9
diff -r1.9 signtool.c
82a83,84
> int xpi_arc = 0;
>
136c138,139
< TOKEN_OPT
---
> TOKEN_OPT,
> XPI_ARC_OPT
258a262,263
> } else if(!PL_strcasecmp(buf, "xpi")) {
> type = XPI_ARC_OPT;
392a398,400
> case 'X':
> type = XPI_ARC_OPT;
> break;
793a802,804
> case XPI_ARC_OPT:
> xpi_arc = 1;
> break;
891a903,913
> /* -X needs -Z */
>
> if (xpi_arc && !zipfile)
> {
> PR_fprintf (errorFD, "%s: option XPI (-X) requires option jarfile (-Z)\n",
> PROGRAM_NAME);
> errorCount++;
> retval = -1;
> goto cleanup;
> }
>
Index: signtool/signtool.h
===================================================================
RCS file: /cvsroot/mozilla/security/nss/cmd/signtool/signtool.h,v
retrieving revision 1.6
diff -r1.6 signtool.h
78c78
< #define VERSION "1.3"
---
> #define VERSION "1.4 Beta - XPI Testing"
80c80
<
---
> #define XPI_TEXT "Creating XPI Compatible Archive"
127a128
> extern int xpi_arc;
Index: signtool/util.c
===================================================================
RCS file: /cvsroot/mozilla/security/nss/cmd/signtool/util.c,v
retrieving revision 1.16
diff -r1.16 util.c
196c196
< PR_fprintf(outputFD, " -b\"basename\"\t\tbasename of .sf, .rsa files for
signing\n");
---
> PR_fprintf(outputFD, " -b \"basename\"\t\tbasename of .sf, .rsa files for signing\n");
198,203c198,203
< PR_fprintf(outputFD, " -d\"certificate directory\"\tcontains cert*.db
and key*.db\n");
< PR_fprintf(outputFD, " -e\".ext\"\t\t\tsign only files with this
extension\n");
< PR_fprintf(outputFD, " -f\"filename\"\t\t\tread commands from file\n");
< PR_fprintf(outputFD, " -G\"nickname\"\t\tcreate object-signing cert with
this nickname\n");
< PR_fprintf(outputFD, " -i\"installer script\"\tassign installer
javascript\n");
< PR_fprintf(outputFD, " -j\"javascript directory\"\tsign javascript files
in this subtree\n");
---
> PR_fprintf(outputFD, " -d \"certificate directory\"\tcontains cert*.db and key*.db\n");
> PR_fprintf(outputFD, " -e \".ext\"\t\t\tsign only files with this extension\n");
> PR_fprintf(outputFD, " -f \"filename\"\t\t\tread commands from file\n");
> PR_fprintf(outputFD, " -G \"nickname\"\t\tcreate object-signing cert with this nickname\n");
> PR_fprintf(outputFD, " -i \"installer script\"\tassign installer javascript\n");
> PR_fprintf(outputFD, " -j \"javascript directory\"\tsign javascript files in this subtree\n");
206c206
< PR_fprintf(outputFD, " -k\"cert nickname\"\t\tsign with this
certificate\n");
---
> PR_fprintf(outputFD, " -k \"cert nickname\"\t\tsign with this certificate\n");
209c209
< PR_fprintf(outputFD, " -m\"metafile\"\t\tinclude custom
meta-information\n");
---
> PR_fprintf(outputFD, " -m \"metafile\"\t\tinclude custom meta-information\n");
214c214
< PR_fprintf(outputFD, " -p\"password\"\t\tfor password on command line
(insecure)\n");
---
> PR_fprintf(outputFD, " -p \"password\"\t\tfor password on command line (insecure)\n");
219c219,221
< PR_fprintf(outputFD, " -x\"name\"\t\t\tdirectory or filename to
exclude\n");
---
> PR_fprintf(outputFD, " -x \"name\"\t\t\tdirectory or filename to exclude\n");
> PR_fprintf(outputFD, " -X\t\t\t\tCreate XPI Compatible Archive\n"
> "\t\t\t\t(used in conjunction with the -Z)\n");
221c223
< PR_fprintf(outputFD, " -Z\"jarfile\"\t\t\tcreate JAR file with the given
name.\n"
---
> PR_fprintf(outputFD, " -Z \"jarfile\"\t\t\tcreate JAR file with the given name.\n"
238a241,242
> PR_fprintf(outputFD, "%s -d \"certificate directory\" -k \"cert nickname\" -p \"password\" -X -Z \"file.xpi\" directory-tree\n", PROGRAM_NAME);
> PR_fprintf(outputFD, " Common syntax to create a XPInstall compatible signed archive\n\n");
Assignee | ||
Comment 3•21 years ago
|
||
Jeff, thanks for your work on this.
I confirm this is an enhancement request. :)
I take it that your comment 2 above was intended to be a revision to
the patch you attached. Is that right?
If so, the way to do that is to *attach* the revised patch to the bug,
and then mark the previous attachment as "obsolete".
Putting a patch inline in the bug commentary makes it difficult to use,
and should only be used when commenting on a small part of a patch.
Also, please use "diff -u" (or "cvs diff -u") when creating patches.
In your posting in the newsgroup, you mentioned that you are now
calling manfesto twice, which results in putting two copies of some
files into the directories, and you wondered how to overcome that.
Creating two copies of the various files is a problem that would be
serious enough that it would prevent the patch from being accepted.
The answer may be to split the "manifesto" function apart into two
functions that can be called at separate times. I have not studied
this in enough depth to make a specific recomendation.
Status: UNCONFIRMED → NEW
Ever confirmed: true
OS: Windows XP → All
Priority: -- → P3
Version: unspecified → 3.9
Reporter | ||
Updated•21 years ago
|
Attachment #151772 -
Attachment is obsolete: true
Reporter | ||
Comment 4•21 years ago
|
||
Updated patch, fixed indentations.
Reporter | ||
Updated•21 years ago
|
Attachment #151788 -
Attachment is obsolete: true
Reporter | ||
Comment 5•21 years ago
|
||
(In reply to comment #3)
> Jeff, thanks for your work on this.
>
> I confirm this is an enhancement request. :)
>
> I take it that your comment 2 above was intended to be a revision to
> the patch you attached. Is that right?
I got confused with the interface, I was trying to update it. I added the new
one as an attachment.
>
> Also, please use "diff -u" (or "cvs diff -u") when creating patches.
Will Do.
> In your posting in the newsgroup, you mentioned that you are now
> calling manfesto twice, which results in putting two copies of some
> files into the directories, and you wondered how to overcome that.
>
> Creating two copies of the various files is a problem that would be
> serious enough that it would prevent the patch from being accepted.
>
> The answer may be to split the "manifesto" function apart into two
> functions that can be called at separate times. I have not studied
> this in enough depth to make a specific recomendation.
Ok, I did alot more looking through the way manifesto() and manifesto_fn() run.
Most of those two functions are meant for creating the manifest.mf and
zigbert.sf files. There is some other code in there that deals with the -J
options. Which cannot be used with -Z. I found that it involved very little to
actually make a new functions that will only add the files to the archive. All
the directory and file permissions were checked previously when manifesto() ran
so that part is already covered.
The new patch added has this new method implemented. I have tested it with all
my test archives and it is behaving as expected. Archives are created exactly
the same as before with the -X not present. If -X is present it runs manifesto
before the zip file is set so the files are not added to the zip file. After
all the META-INF/ files are created and zigbert.rsa is added to the zip file,
the new function will run through all the files in the tree and add them to the
zip file.
Let me know what you think.
Assignee | ||
Comment 6•21 years ago
|
||
In news://news.mozilla.org:119/40DD0D6C.9020604@meer.net Doug Turner wrote:
> I did something like this to create the xpi (from a newsgroup posing I made):
>
> signtool -d ./certs -kdougt test
> cd test
> zip test.xpi META-INF/zigbert.rsa
> zip -r -D test.xpi * -x META-INF/zigbert.rsa
> mv test.xpi ../
> cd ..
So, there is a work-around for this bug, hence P3
Assignee | ||
Comment 7•21 years ago
|
||
Comment on attachment 151789 [details] [diff] [review]
New -X option for XPI archive creation -- Not using manifesto() twice.
Review comments on this third patch:
1) in general, this patch is much better than earlier ones.
Don't worry about whitespace/indentation. I intend to run the
signtool sources through cb (c beautifier) prior to the NSS 3.10
release.
2) We don't want to call the new code 1.3 or 1.4 Beta or 1.4.
6 years ago, signtool was released as a stand-alone tool,
and so it had its own release notes and version number.
For the last 4 years, signtool has not been released separately from NSS.
It is just another one of the NSS tools, and NSS has its own release
numbers and release notes. This new code will be part of NSS 3.10,
and so, if anything, this new version should be version 3.10.
3) This patch duplicates a lot of code in sign.c so that it can reorder
a couple of things. It would be better if this patch didn't duplicate
all that code.
It appears to me that the differences between the xpi_arc code path
and the previous code path are:
a) whether manifesto is called immediately before or immediatel after
the conditional call to Jzipopen.
b) an additional conditional verbose output (could be in both paths),
c) whether the output of the .[dr]sa file happens just before the code
marked with "mf to zip", or whether it happens just after the code
marked with "sf to zip", and
d) whether there is an additional foreach loop just before "mf to zip".
Those differences could be accomplished with must less duplication. For
example (in pseudo code here):
a) if (xpi_arc)
manifesto
JzipOpen
if (!xpi_arc)
manifesto
c) if (xpi_arc) {
jzipadd .[dr]sa
foreach
}
mf to zip
sf to zip
if (!xpi_arc)
jzipadd .[dr]sa
4) it would be good to put a block comment by each of those two places,
explaining the significance of the ordering of the JzipOpen and manifesto
calls, and explaining why the extra foreach call is needed in the xpi_arc
path.
Attachment #151789 -
Flags: review-
Reporter | ||
Comment 8•21 years ago
|
||
(In reply to comment #7)
> (From update of attachment 151789 [details] [diff] [review])
> Review comments on this third patch:
>
> 1) in general, this patch is much better than earlier ones.
> Don't worry about whitespace/indentation. I intend to run the
> signtool sources through cb (c beautifier) prior to the NSS 3.10
> release.
Didn't even now a beautifier existed for C.
> 2) We don't want to call the new code 1.3 or 1.4 Beta or 1.4.
> 6 years ago, signtool was released as a stand-alone tool,
> and so it had its own release notes and version number.
> For the last 4 years, signtool has not been released separately from NSS.
> It is just another one of the NSS tools, and NSS has its own release
> numbers and release notes. This new code will be part of NSS 3.10,
> and so, if anything, this new version should be version 3.10.
Thats fine. I named it that so I could differentiate which one I was testing.
> 3) This patch duplicates a lot of code in sign.c so that it can reorder
> a couple of things. It would be better if this patch didn't duplicate
> all that code.
>
> It appears to me that the differences between the xpi_arc code path
> and the previous code path are:
> a) whether manifesto is called immediately before or immediatel after
> the conditional call to Jzipopen.
> b) an additional conditional verbose output (could be in both paths),
> c) whether the output of the .[dr]sa file happens just before the code
> marked with "mf to zip", or whether it happens just after the code
> marked with "sf to zip", and
> d) whether there is an additional foreach loop just before "mf to zip".
>
> Those differences could be accomplished with must less duplication. For
> example (in pseudo code here):
> a) if (xpi_arc)
> manifesto
> JzipOpen
> if (!xpi_arc)
> manifesto
>
> c) if (xpi_arc) {
> jzipadd .[dr]sa
> foreach
> }
> mf to zip
> sf to zip
> if (!xpi_arc)
> jzipadd .[dr]sa
That is better. Like I said before I'm not much a C programmer. Thanks for the
criticism. I am adding a new patch of my current code with this method.
> 4) it would be good to put a block comment by each of those two places,
> explaining the significance of the ordering of the JzipOpen and manifesto
> calls, and explaining why the extra foreach call is needed in the xpi_arc
> path.
>
I had gone through and commented it all after I made the last patch but lost it
when DevC++ decided to crash on me and I gave up and submitted the uncommented
patch. I didn't have any more time to be at the computer. The new patch has more
comments in it.
No longer blocks: 249330
Reporter | ||
Comment 9•21 years ago
|
||
Attachment #151789 -
Attachment is obsolete: true
Assignee | ||
Comment 12•21 years ago
|
||
Comment on attachment 152090 [details] [diff] [review]
-X option patch -- Using Nelson's proposed method
The XPI path in this patch depends on JzipAdd failing each and every time
it is called UNTIL JzipOpen is called (which is after manifesto has already
trafersed the tree once). Given that the patch intends for JzipAdd to fail,
it would be good if JzipAdd failed quickly, without doing a lot of work
first.
So, I suggest an additional patch, to JzipAdd in zip.c, to cause it to
detect that zipfp is NULL, and return immediately in that case,
before it tries to do anything more. I am giving this patch r+,
conditional on the completion of the patch to JzipAdd.
Jeff, you can just add that patch as an additional attachment to this bug,
without needing to obsolete the present patch. I am also going to ask
for a second review of this patch.
Wan-Teh, I am planning to run the sources through cb after this, so I
have forgone the usual comments about coding style (since these sources
already have at least 4 distinct styles in them). I would prefer to see
the version strings come from some NSS header file rather than being hard
coded in signtool.h (where they are now), but I haven't looked to see
how to do that.
Attachment #152090 -
Flags: superreview?(wchang0222)
Attachment #152090 -
Flags: review+
Comment 13•21 years ago
|
||
Comment on attachment 152090 [details] [diff] [review]
-X option patch -- Using Nelson's proposed method
I don't understand why there is an extra foreach call
(to invoke manifesto_xpi_fn) in the xpi_arc case.
Does that add the files to the zip archive twice?
(Note: I am not familiar with signtool or zip archives.)
You can get the "3.10" string by including "nss.h"
and using the NSS_VERSION macro. Use C's string
literal concatenation. For example,
#define DEFAULT_COMMON_NAME "Signtool " NSS_VERSION " Testing Certificate"
#define CREATOR "Signtool (signtool " NSS_VERSION ")"
>+ * m a n i f e s t o _xpi_ f n
Nit: needs spaces between the three letters in "xpi".
Assignee | ||
Comment 14•21 years ago
|
||
Comment 13 tells me that the block comment that Jeff added just before the
only call to "manifesto" was insufficient. That comment needs to be
expanded to explain that, if the zipfile itself is not opened prior to
calling manifesto, manifesto will traverse the tree and generate all the
relevant signature files, but will not actually put any of them into the
zip file (since it's not open). Consequently, after that job is done, it
is necessary to take another pass over the tree to output the files in
the desired order.
Jeff, please expand the comment to say that.
Also, please use Wan-Teh's suggestion for using the string from nss.h
rather than hardcoding it, as the patch presently does.
You might as well submit yet another single patch that includes these
changes as well as the change to JzipAdd that I proposed in comment 12.
Comment 15•21 years ago
|
||
Comment on attachment 152090 [details] [diff] [review]
-X option patch -- Using Nelson's proposed method
Apparently Nelson understands how the patch works,
so I'll ask him to explain this patch to me.
Here is the status of my review of this patch: I
verified that it won't introduce regressions because
if xpi_arc is 0, the new code is equivalent to the
old code. So in this sense the patch meets my
minimum requirement. I also verified that the code
matches the comments, and the comments make sense to
me. But I don't fully understand the changes in
sign.c yet.
I have a question: should we add a diagnostic
message to the beginning of manifesto_xpi_fn because
manifesto_fn has it?
if(verbosity >= 0) {
PR_fprintf(outputFD, "--> %s\n", relpath);
}
Assignee | ||
Comment 16•21 years ago
|
||
Checked in Jeff's patch. I plan to make additional changes soon.
Checking in README; new revision: 1.2; previous revision: 1.1
Checking in sign.c; new revision: 1.10; previous revision: 1.9
Checking in signtool.c; new revision: 1.11; previous revision: 1.10
Checking in signtool.h; new revision: 1.8; previous revision: 1.7
Checking in util.c; new revision: 1.18; previous revision: 1.17
Assignee | ||
Comment 17•21 years ago
|
||
This patch changes signtool to get the version number string from nss.h,
and causes JzipAdd to shortcut when the zip file is not open.
It also extends the QA test script to test out signtool in this new mode.
Assignee | ||
Comment 18•21 years ago
|
||
Comment on attachment 153234 [details] [diff] [review]
Apply Wan-Teh and Nelson's review feedback
Wan-Teh, please review that this patch implements your suggestions correctly,
and that it tests the new -X option.
Attachment #153234 -
Flags: review?(wchang0222)
Comment 19•21 years ago
|
||
Comment on attachment 153234 [details] [diff] [review]
Apply Wan-Teh and Nelson's review feedback
r=wtc.
Attachment #153234 -
Flags: review?(wchang0222) → review+
Assignee | ||
Comment 20•21 years ago
|
||
Checking in cmd/signtool/sign.c;
new revision: 1.11; previous revision: 1.10
Checking in cmd/signtool/signtool.h;
new revision: 1.9; previous revision: 1.8
Checking in cmd/signtool/util.c;
new revision: 1.19; previous revision: 1.18
Checking in cmd/signtool/zip.c;
new revision: 1.4; previous revision: 1.3
Checking in tests/tools/tools.sh;
new revision: 1.12; previous revision: 1.11
Status: NEW → RESOLVED
Closed: 21 years ago
Resolution: --- → FIXED
Assignee | ||
Comment 21•21 years ago
|
||
Many thanks to Jeff Klawiter for contributing this fix!
Comment 22•21 years ago
|
||
Comment on attachment 153234 [details] [diff] [review]
Apply Wan-Teh and Nelson's review feedback
>Index: cmd/signtool/zip.c
>@@ -172,10 +173,12 @@
> if( !fullname || !filename || !zipfile) {
> return -1;
> }
>
> zipfp = zipfile->fp;
>+ if (!zipfp)
>+ return -1;
>
>
> if( (readfp = PR_Open(fullname, PR_RDONLY, 0777)) == NULL) {
> char *nsprErr;
> if(PR_GetErrorTextLength()) {
Nelson, while I think this change is fine,
I don't understand why we weren't crashing
in PR_Seek(zipfp) or PR_Write(zipfp) before
if zipfp was NULL.
Reporter | ||
Comment 23•21 years ago
|
||
(In reply to comment #21)
> Many thanks to Jeff Klawiter for contributing this fix!
You're welcome. I'm sorry I wasn't able to finish up the patch with the rest of
the comments made. Things have been really busy in the real world lately and
I've barely even had the time to check my email for the last couple weeks.
Either way I'm happy this new option is being added to signtool and look forward
to the next official release of NSS.
Assignee | ||
Updated•21 years ago
|
Attachment #152090 -
Flags: superreview?(wchang0222)
Comment 24•20 years ago
|
||
Can anyone please e-mail me a Windows version of the new signtool?
Note: I don't have a compiler at hand (6000 miles from home) and desperately
need to sign one of my XPI's, thank you.
/HJ
Comment 25•20 years ago
|
||
See
http://groups-beta.google.com/group/netscape.public.mozilla.crypto/msg/2f15d0abe608bcd6.
If you have further questions, please follow up in the newsgroup.
Assignee | ||
Comment 26•20 years ago
|
||
Google's web-based interface to newsgroups is a great way to search news
archives, but AFAIK, messages posted there do not propagate to mozilla's
own news server, and so may not be seen by mozilla developers. So if you
want to post messages into the mozilla crypto newsgroup, I suggest that
you use mozilla's news server directly. This URL should get you there.
news://news.mozilla.org:119/netscape.public.mozilla.crypto
You need to log in
before you can comment on or make changes to this bug.
Description
•