222726 - SSL_NO_CACHE option should obviate server cache

Status: RESOLVED FIXED

(NSS :: Libraries, defect, P2)

Description

[Nelson Bolyard (seldom reads bugmail)]

When the SSL_NO_CACHE option has been set on an SSL server socket, there 
should be NO calls to any SSL server cache functions made by the SSL library 
in the context of that server socket, or any of its progency sockets.  
It should not be necessary to configure the SSL server cache when the sockets
have been configured to not use the cache.  

But in practice, SSL server calls are made, even when noCache is set.  
This should be fixed for certain tiny SSL servers that do not wish to use
the cache.

Comment 1

[Nelson Bolyard (seldom reads bugmail)]

I will look at this this weekend, and try to get this in for 3.8.3.  

Comment 2

[Nelson Bolyard (seldom reads bugmail)]

Attachment: patch v1 for trunk

This patch adds tests of the noCache flag in several places where they were
missing.  It also adds a new -N option to selfserv to test this configuration.

When all the server sockets use the SSL_NO_CACHE option, the server session 
cache is not used at all and there is no need to initialize or destroy it.

This patch is for the trunk.  I will add a patch for the 3.8 branch shortly.

Comment 3

[Wan-Teh Chang]

Comment on attachment 133608 [details] [diff] [review]
patch v1 for trunk

r=wtc.

Comment 4

[Wan-Teh Chang]

Comment on attachment 133608 [details] [diff] [review]
patch v1 for trunk

Although I've reviewed this patch, I'd like to suggest two
changes.

1. lib/ssl/ssl3con.c

>-    } else {
>+    } else if (!ss->noCache) {
> 	/* server. */

We should update the comment to say "server with session ID cache"
to match the new code.

2. cmd/selfserv/selfserv.c

>+    if (!NoReuse || maxProcs > 1 || envString) {
>+	SSL_ShutdownServerSessionIDCache();
>+    }

How about adding a Boolean local variable "hasSidCache",
set it to true after SSL_InheritMPServerSIDCache,
SSL_ConfigMPServerSIDCache, or SSL_ConfigServerSessionIDCache
is called, and call SSL_ShutdownServerSessionIDCache if
it is true?

Comment 5

[Nelson Bolyard (seldom reads bugmail)]

Attachment: patch for NSS 3.8 branch

This is essentially the same patch, but for the NSS 3.8 branch.

Comment 6

[Nelson Bolyard (seldom reads bugmail)]

OK, I just noticed your comment 4.  Those are good ideas.  

Comment 7

[Wan-Teh Chang]

Comment on attachment 133611 [details] [diff] [review]
patch for NSS 3.8 branch

r=wtc.

Comment 8

[Nelson Bolyard (seldom reads bugmail)]

Attachment: patch as checked in on NSS 3.8 branch

Patch for trunk is similar.

Comment 9

[Nelson Bolyard (seldom reads bugmail)]

Marking fixed.

Comment 10

[Wan-Teh Chang]

Comment on attachment 133619 [details] [diff] [review]
patch as checked in on NSS 3.8 branch

I just wanted to point out that this patch also
contains an earlier change to ssl3con.c, made in
rev. 1.53.2.1 (NSS 3.8.1) for bug 202348.

Comment 11

[Nelson Bolyard (seldom reads bugmail)]

I am reopening this bug because it isn't compmletely fixed.  
There are still cases where you get a null cache crash if you don't
initialize the cache.  This can be demonstrated with selfserv.

Comment 12

[Nelson Bolyard (seldom reads bugmail)]

Bug 237724 records the remaining problem that occurs when the cache is
not initialized.  So, I am returning this bug to the resolved/fixed 
status.  

Comment 13

[Nelson Bolyard (seldom reads bugmail)]

This bug and Bug 237724 are very similar, but not quite identical.
They may have a common cause.

This bug states that when all SSL sockets have SSL_NO_CACHE set,
then it should not be necessary to initialize the SSL server session 
cache.  However, selfserv -N is enough to demonstrate that this is not so.

Bug 237724 states that a server crashes when the SSL server session cache
is not initialized, and SSL_NO_CACHE is NOT SET on the ssl sockets.  
That case should fail, and is due to an application programming error.
The only issue there is that perhaps it should not crash.

I am changing the target milestone of this bug from 3.8.3 to 3.10,
since it was not really completely fixed in NSS 3.8.3.  :-(

Comment 14

[Nelson Bolyard (seldom reads bugmail)]

I am marking this bug fixed again.  It was marked fixed once before.
Since fixing bug 237724, I cannot reproduce this crash, and so I 
believe it is (again) fixed in NSS 3.10.