If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

ssl_V3_SUITES_IMPLEMENTED is off by 3 when the FORTEZZA cipher suites were removed

RESOLVED FIXED in 3.11.1

Status

NSS
Libraries
P1
trivial
RESOLVED FIXED
12 years ago
12 years ago

People

(Reporter: Wan-Teh Chang, Assigned: Nelson Bolyard (seldom reads bugmail))

Tracking

({regression})

3.11
3.11.1
regression

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

591 bytes, patch
Nelson Bolyard (seldom reads bugmail)
: review+
Details | Diff | Splinter Review
(Reporter)

Description

12 years ago
In NSS 3.11, when we removed FORTEZZA support, three
FORTEZZA cipher suites were removed from the cipherSuites
array in lib/ssl/ssl3con.c, but the ssl_V3_SUITES_IMPLEMENTED
macro (the size of that array) in lib/ssl/sslimpl.h
is not updated.  So ssl_V3_SUITES_IMPLEMENTED is now
off by 3, resulting in three zero entries at the end
of the cipherSuites array.  I believe those three zero
entries are harmless, hence the "trivial" severity of
this bug.  But it is best to fix this on the NSS_3_11_BRANCH
the first chance we have.
(Reporter)

Updated

12 years ago
Keywords: regression
Priority: -- → P1
Target Milestone: --- → 3.11.1
(Reporter)

Comment 1

12 years ago
Created attachment 205785 [details] [diff] [review]
Patch for NSS_3_11_BRANCH

I fixed this bug on the NSS trunk (NSS 3.12) with
a patch (attachment 205783 [details] [diff] [review]) in bug 236245.
Attachment #205785 - Flags: review?(nelson)
(Assignee)

Comment 2

12 years ago
Comment on attachment 205785 [details] [diff] [review]
Patch for NSS_3_11_BRANCH

Wan-Teh, there's a discrepancy between this patch and the one
you made for the trunk, for the case when NSS_ENABLE_ECC is defined.
In this patch, you're decreasing the value by 3, from 40 to 37,
but in the patch for the trunk, you increased it by 3, from 40 to 43.
Does the trunk have 6 more ECC ciphersuites than 3.11 did?
or ??

> #ifdef NSS_ENABLE_ECC
>-#define ssl_V3_SUITES_IMPLEMENTED 40
>+#define ssl_V3_SUITES_IMPLEMENTED 37
> #else
>-#define ssl_V3_SUITES_IMPLEMENTED 26
>+#define ssl_V3_SUITES_IMPLEMENTED 23
> #endif /* NSS_ENABLE_ECC */
(Reporter)

Comment 3

12 years ago
Nelson: yes, Douglas added 6 new ECC cipher suites
to the trunk in that patch, resulting in a net change
of +3 (+6 -3).  In hindsight, I should not have included
the fix for this bug in that patch because it is confusing
if you just look at the CVS commit comment.
(Assignee)

Comment 4

12 years ago
Comment on attachment 205785 [details] [diff] [review]
Patch for NSS_3_11_BRANCH

Thanks for the explanation.
Attachment #205785 - Flags: review?(nelson) → review+
(Reporter)

Comment 5

12 years ago
I checked in the patch on the NSS_3_11_BRANCH for NSS 3.11.1.

Checking in sslimpl.h;
/cvsroot/mozilla/security/nss/lib/ssl/sslimpl.h,v  <--  sslimpl.h
new revision: 1.42.2.1; previous revision: 1.42
done
Status: NEW → RESOLVED
Last Resolved: 12 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.