Add accessor functions for SSL_ImplementedCiphers

RESOLVED FIXED in 3.12.6

Status

NSS
Libraries
--
enhancement
RESOLVED FIXED
9 years ago
3 years ago

People

(Reporter: Wan-Teh Chang, Assigned: Wan-Teh Chang)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Assignee)

Description

9 years ago
Right now, the array SSL_ImplementedCiphers is declared in the
public header "ssl.h" as an array rather than a pointer:

  /* constant table enumerating all implemented SSL 2 and 3 cipher suites. */
  SSL_IMPORT const PRUint16 SSL_ImplementedCiphers[];

For some reason, this causes the Linux linker ld to record the
size of the array at link time, and that size is compared with
the actual array size at run time.  If the two sizes differ, ld
emits a scary runtime warning message when an app that uses
SSL_ImplementedCiphers starts up:

./chrome: Symbol `SSL_ImplementedCiphers' has different size in shared 
object, consider re-linking

This is reported in the following Chromium bug report:
http://code.google.com/p/chromium/issues/detail?id=12826

I'm not sure if we can declare SSL_ImplementedCiphers as a
pointer in "ssl.h" without breaking binary compatibility.
It would be safe to add a function that returns SSL_ImplementedCiphers:

const PRUint16 *
SSL_GetImplementedCiphers()
{
    return SSL_ImplementedCiphers;
}

or a function that returns the i-th implemented cipher:

PRUint16
SSL_GetImplementedCipher(PRUint16 i)
{
    if (i >= SSL_NumImplementedCiphers)
        return 0;
    return SSL_ImplementedCiphers[i];
}
> I'm not sure if we can declare SSL_ImplementedCiphers as a
> pointer in "ssl.h" without breaking binary compatibility.

No, I believe we cannot, at least not on Unix/Linux.  

Surely there must be some way to disable that pedantic nonsense.
(Assignee)

Comment 2

9 years ago
I did some web searches but didn't find anything
that describes how to suppress that warning message.

This is how Dan Kegel plans to work around this problem:
http://codereview.chromium.org/118367
I have no objection to adding either of the functions suggested in comment 0.

I do object to driving developers away from using system behavior that has 
worked flawlessly, without being "scary", for decades.  But that's what 
Linux has evidently chosen to do.  Sigh.
Summary: Add functions to replace SSL_ImplementedCiphers → Add accessor functions for SSL_ImplementedCiphers
(Assignee)

Comment 4

8 years ago
Created attachment 426089 [details] [diff] [review]
Add the SSL_GetImplementedCiphers and SSL_GetNumImplementedCiphers functions

There are four programs under nss/cmd that use SSL_ImplementedCiphers
and SSL_NumImplementedCiphers: selfserv, strsclnt, tstclnt, vfyutil

I modified two of them (strsclnt and tstclnt) to use the new functions.
The other two still use the global variables.
Attachment #426089 - Flags: review?(nelson)
(Assignee)

Updated

8 years ago
Assignee: nobody → wtc
Status: NEW → ASSIGNED
Target Milestone: --- → 3.12.6
Attachment #426089 - Flags: review?(nelson) → review+
Comment on attachment 426089 [details] [diff] [review]
Add the SSL_GetImplementedCiphers and SSL_GetNumImplementedCiphers functions

r=nelson
(Assignee)

Comment 6

8 years ago
I checked in the patch on the NSS trunk (NSS 3.12.6).

Checking in cmd/strsclnt/strsclnt.c;
/cvsroot/mozilla/security/nss/cmd/strsclnt/strsclnt.c,v  <--  strsclnt.c
new revision: 1.66; previous revision: 1.65
done
Checking in cmd/tstclnt/tstclnt.c;
/cvsroot/mozilla/security/nss/cmd/tstclnt/tstclnt.c,v  <--  tstclnt.c
new revision: 1.62; previous revision: 1.61
done
Checking in lib/ssl/ssl.def;
/cvsroot/mozilla/security/nss/lib/ssl/ssl.def,v  <--  ssl.def
new revision: 1.23; previous revision: 1.22
done
Checking in lib/ssl/ssl.h;
/cvsroot/mozilla/security/nss/lib/ssl/ssl.h,v  <--  ssl.h
new revision: 1.36; previous revision: 1.35
done
Checking in lib/ssl/sslenum.c;
/cvsroot/mozilla/security/nss/lib/ssl/sslenum.c,v  <--  sslenum.c
new revision: 1.17; previous revision: 1.16
done
Status: ASSIGNED → RESOLVED
Last Resolved: 8 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.