495097 - sdb_mapSQLError returns signed int, causing sign extension on 64-bit platforms

Status: RESOLVED FIXED

(NSS :: Libraries, defect, P1)

Description

[Wan-Teh Chang]

Attachment: Proposed patch

sdb_mapSQLError maps SQLite errors to PKCS #11 errors,
but it is incorrectly declared to return (signed) int.
When its return value is assigned to CK_RV, which is
unsigned long, on 64-bit platforms, sign extension
occurs.

The sign extension causes CKR_NETSCAPE_CERTDB_FAILED
(0xce534351) to become 0xffffffffcd534351.

A place where accurate error reporting is important is
http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/security/nss/lib/pk11wrap/pk11load.c&rev=1.25&mark=146,153-158,164#122

If I call NSS_InitReadWrite(sql:/home/wtc/.pki/nssdb)
and the directory doesn't exist, I get the confusing
SEC_ERROR_INVALID_ARGS error because the wrong
error code causes secmod_ModuleInit to call the
softoken again with C_Initialize(NULL), and that
fails with SEC_ERROR_INVALID_ARGS.

Comment 1

[Wan-Teh Chang]

Attachment: Proposed patch with regression test

Comment 2

[Robert Relyea]

Comment on attachment 379932 [details] [diff] [review]
Proposed patch with regression test

Though there should already be a test case for this called db test, it may be better to make sure dbtest checks for the correct SEC_ERROR error code.

Also old off fix check in. We are likely to be doing a respin for FIPS, and I'd like to collect this one with the FIPS respin.

Comment 3

[Wan-Teh Chang]

Bob, could you check in my patch for me?  I'm not following
the respin for FIPS, so I'm afraid that I may miss the right
time to check this in.  Thanks.

I can move the regression test to dbtest.c later.

Comment 4

[Robert Relyea]

To problem. You'ved marked it as fips and assigned it to me... so we are set.

bob

Comment 5

[Robert Relyea]

RCS file: /cvsroot/mozilla/security/nss/cmd/tests/baddbdir.c,v
done
Checking in cmd/tests/baddbdir.c;
/cvsroot/mozilla/security/nss/cmd/tests/baddbdir.c,v  <--  baddbdir.c
initial revision: 1.1
done
Checking in cmd/tests/manifest.mn;
/cvsroot/mozilla/security/nss/cmd/tests/manifest.mn,v  <--  manifest.mn
new revision: 1.8; previous revision: 1.7
done
Checking in lib/softoken/sdb.c;
/cvsroot/mozilla/security/nss/lib/softoken/sdb.c,v  <--  sdb.c
new revision: 1.13; previous revision: 1.12
done