394271 - two public SSL functions require PRFD* to point to SSL layer

Status: RESOLVED FIXED

(NSS :: Libraries, defect, P2)

Description

[Nelson Bolyard (seldom reads bugmail)]

libSSL's methods that get called through NSPRs PRIOMethods table require 
that the PRFileDescriptor (PRFD) argument must be the SSL layer's PRFD.  
Nearly all other SSL functions that have PRFD arguments permit, but do not
require, that the PRFD be the SSL layer's PRFD.  They will work even if 
called with the PRFD from another layer in the stack, PROVIDED that there
is only one SSL layer in the stack (which is the usual case).

However, there are two functions, new in NSS 3.11, that require that the 
PRFD argument be precisely the SSL layer PRFD.  They are 
SSL_ReHandshakeWithTimeout and SSL_ForceHandshakeWithTimeout.  
These are the only two callers to ssl_SetTimeout, also new in NSS 3.11.

They require that the PRFD be the SSL layer's, rather than another layer
in the stack, because ssl_SetTimeout calls ssl_GetPrivate to get the 
address of the sslSocket structure.  Most other non-PRIOMethod functions
call ssl_FindSocket instead of ssl_GetPrivate for this purpose.  

ssl_GetPrivate requires that its argument be an SSL layer PRFD, while
ssl_FindSocket searches the stack for the SSL layer PRFD if the PRFD 
address passed to it is not an SSL layer PRFD.  

I propose to 
a) move ssl_SetTimeout to sslsecur.c, to be in the same file with its callers, 
b) make it a static function, and
c) make it call ssl_FindSocket instead of ssl_GetPrivate.

Comment 1

[Nelson Bolyard (seldom reads bugmail)]

Attachment: patch v1

This patch does what I proposed in comment 0.

Comment 2

[Wan-Teh Chang]

Comment on attachment 278903 [details] [diff] [review]
patch v1

r=wtc.  We also need to remove the function declaration
in sslimpl.h.

Comment 3

[Nelson Bolyard (seldom reads bugmail)]

Attachment: patch v2, removes func from header file

I'm asking Wan-Teh for SR again.  Wan-Teh, if you are unable to set the 
SR flag, please don't cancel the SR request, and instead let's press this
issue to get it fixed.

Comment 4

[Wan-Teh Chang]

Comment on attachment 278983 [details] [diff] [review]
patch v2, removes func from header file

I can't set superreview+ on this patch.  The dropdown box for
superreview only has empty (cancel request) and '?'.

This patch is correct.

Comment 5

[Nelson Bolyard (seldom reads bugmail)]

On trunk:
Checking in sslimpl.h;   new revision: 1.60; previous revision: 1.59
Checking in sslsecur.c;  new revision: 1.39; previous revision: 1.38
Checking in sslsock.c;   new revision: 1.54; previous revision: 1.53

Comment 6

[Nelson Bolyard (seldom reads bugmail)]

On branch:
Checking in sslimpl.h;  new revision: 1.42.2.13; previous revision: 1.42.2.12
Checking in sslsecur.c; new revision: 1.34.2.5; previous revision: 1.34.2.4
Checking in sslsock.c;  new revision: 1.44.2.9; previous revision: 1.44.2.8