470055 - pkix_HttpCertStore_FindSocketConnection reuses closed socket, OCSP fails

Status: RESOLVED FIXED

(NSS :: Libraries, defect)

Description

[Kai Engert (:KaiE:)]

I have a test program, it attempts to verify multiple certificates.
This involves talking to OCSP each time.
(Unfortunately I currently have an inhouse intranet test only)

The first verification attempt succeeds.
Future OCSP verif. fail.

Does not depend on the certs, same result with different order (first works, other fail).

Comment 1

[Kai Engert (:KaiE:)]

The failure is in the attempt to find-and-use a cached connection.

After the first attempt succeeded, I can see (using netstat) that the socket is already in state CLOSED_WAIT.

But the next OCSP attempt still tries to use the socket.
It writes the request, not detecting the failure (I see a successful write).
Then it attempts to read from the socket, but obtains zero bytes, and goes to the "socket closed error".

The OCSP attempt fails.

Desired behavior: The code in pkix_pl_httpdefaultclient.c should detect that the socket to the OCSP server has already been closed, and should open a new socket.

Comment 2

[Kai Engert (:KaiE:)]

pkix_pl_HttpDefaultClient_RequestCreate makes a lookup to a hashtable, finds a socket and returns it, but the socket is already closed.

Comment 3

[Kai Engert (:KaiE:)]

Attachment: Workaround patch v1

This patch disables the cache for http sockets.
It fixes the problem report I've received.

Comment 4

[Kai Engert (:KaiE:)]

FYI originally reported at: https://bugzilla.redhat.com/show_bug.cgi?id=474727

Comment 5

[Kai Engert (:KaiE:)]

Comment on attachment 353525 [details] [diff] [review]
Workaround patch v1

I don't have a better idea at this time.

Comment 6

[Alexei Volkov]

Comment on attachment 353525 [details] [diff] [review]
Workaround patch v1

r-. The patch is ok but incomplete. I prefer we disable the whole socket cache for now.

Comment 8

[Alexei Volkov]

3.12.4(the current trunk) has this problem resolved by excluding initialization of the socket cache. As the result, libpkix wont be able to cache a socket and the socked will be closed.

http://bonsai.mozilla.org/cvsview2.cgi?diff_mode=context&whitespace_mode=show&file=pkix_lifecycle.c&branch=&root=/cvsroot&subdir=mozilla/security/nss/lib/libpkix/pkix/top&command=DIFF_FRAMESET&rev1=1.8&rev2=1.9

Nevertheless, there are cases when it is preferable to keep and reuse the socket. For these cases we will extend cache functionality if we need it in the future. But for now closing the bug.

Comment 9

[Nelson Bolyard (seldom reads bugmail)]

The OCSP code reads in the response from the socket 1K bytes at a time.
If the responder sends back a response that is longer than 1024 bytes,
and the response code is not 200, the OCSP client stops processing the
response without reading it all in.  Consequently, if this connection 
is reused, the next attempt to read in an OCSP response will read in 
part of the previous response.  

The code that causes this early termination of the response processing is 
in file pkix_pl_httpdefaultclient.c in function 
pkix_pl_HttpDefaultClient_HdrCheckComplete at about line 225:

        if (client->responseCode != 200) {
                client->connectStatus = HTTP_ERROR;
                goto cleanup;
        }
I'm not saying that this code is wrong, but merely that, if we are going to 
continue to allow this code to stop without reading in the entire response
then we must make sure the socket is closed and not reused after that.