The default bug view has changed. See this FAQ.

shlibsign returns 0 although it fails

RESOLVED FIXED in 3.15.1

Status

NSS
Tools
RESOLVED FIXED
7 years ago
4 years ago

People

(Reporter: Ales Marecek, Assigned: kaie)

Tracking

(Blocks: 1 bug)

3.12.4
3.15.1
All
Linux

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: FIPS)

Attachments

(1 attachment)

(Reporter)

Description

7 years ago
User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.2.3) Gecko/20100412 Red Hat/3.6.3-4.el6 Firefox/3.6.3
Build Identifier: 

Description of problem:
shlibsign returns zero value although it fails.

Reproducible: Always

Steps to Reproduce:
1. /usr/lib/nss/unsupported-tools/shlibsign -v -i nonexistent_lib.so; echo "
---> returned: $?"
Actual Results:  
moduleSpec configdir='' certPrefix='' keyPrefix='' secmod='' flags=noCertDB,
noModDB
Generate a DSA key pair ... 
nonexistent_lib.so: -5950: File not found
 ---> returned: 0

Expected Results:  
moduleSpec configdir='' certPrefix='' keyPrefix='' secmod='' flags=noCertDB,
noModDB
Generate a DSA key pair ... 
nonexistent_lib.so: -5950: File not found
 ---> returned: 1

It is not important what to return but it should be non-zero value.
Blocks: 544289
Whiteboard: FIPS
Version: unspecified → 3.12.4
Summary: shlibsign returns 0 although if fails → shlibsign returns 0 although it fails
Having re-read the code, it's apparent to me that the author(s) didn't 
intend for it to return non-zero except in the case where the attempt to 
shutdown the PKCS#11 module used for signing failed.  So, this is a case 
of mismatched expectations.  

Now, I cannot explain or defend the decision to return non-zero solely in
that case.  I too would expect it to return non-zero on any failure.  
But it's always done what it is now doing, and I am concerned that changing
it now could break scripts and/or builds.  

Bob, what do you think?

Comment 2

7 years ago
I think the change is fine. shlibsign is outside the FIPS boundary, so it's OK to change without a revalidation.

I think I would want to know what scripts and builds fail if we make this change. It sounds like a bug in the script or build. I believe the owners of those are either the NSS team, or close enough to the NSS team that they can be fixed if there is a problem.

bob

Comment 3

7 years ago
Clearly no longer unconfirmed.
Status: UNCONFIRMED → NEW
Ever confirmed: true
(Assignee)

Comment 4

4 years ago
Created attachment 742571 [details] [diff] [review]
patch v1

The statement in comment 1 is incorrect - the function returns the non-zero error return code in many scenarios:
- if incorrect arguments are given
- whenever a call to a pkcs#11 API fails

However, for any non-pkcs#11 layer related failures, the function will return zero, and in the success scenario, it will return zero, too.

I believe it makes absolute sense to return non-zero in any failure scenario.

I cannot imagine that any tests break if we make that change.
If it does, those would rather have been undetected failures!

I'm attaching a trivial patch that implements the request.
I've ran the test suite, and it still passes.
Assignee: nobody → kaie
Attachment #742571 - Flags: review?(rrelyea)

Comment 5

4 years ago
Comment on attachment 742571 [details] [diff] [review]
patch v1

r+ rrelyea
Attachment #742571 - Flags: review?(rrelyea) → review+
(Assignee)

Comment 6

4 years ago
https://hg.mozilla.org/projects/nss/rev/d23a241f9ab5
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → 3.15.1
You need to log in before you can comment on or make changes to this bug.