367023 - Too much memory allocated for each SSL connection

Status: NEW

(NSS :: Libraries, defect, P2)

Description

[Michal Novotny [:michal]]

User-Agent:       Mozilla/5.0 (compatible; Konqueror/3.5; Linux) KHTML/3.5.5 (like Gecko)
Build Identifier: 3.11

For every SSL server socket is allocated around 135Kb. 100Kb of this memory are 5x18432 (sslBuffer) and 1x11272 (sslSocket). Back traces of each allocation are:

18432 bytes
PR_Malloc
PORT_Alloc
sslBuffer_Grow [sslsecur.c:423]
ssl3_AppendHandshake [ssl3con.c:2863]
ssl3_AppendHandshakeNumber [ssl3con.c:2912]
ssl3_AppendHandshakeHeader [ssl3con.c:2948]
ssl3_SendServerHello [ssl3con.c:6041]
ssl3_SendServerHelloSequence [ssl3con.c:5330]
ssl3_HandleV2ClientHello [ssl3con.c:5979]
ssl2_HandleClientHelloMessage [sslcon.c:3485]
ssl_Do1stHandshake [sslsecur.c:149]
ssl_SecureRecv [sslsecur.c:1032]
ssl_Recv [sslsock.c:1352]
PR_Recv [priometh.c:221]
apFileDescWrapper::Peek(void*, unsigned int, unsigned int*)
[apFileDescWrapper.cpp:251]

18432 bytes
PR_Malloc
PORT_Alloc
sslBuffer_Grow [sslsecur.c:423]
ssl_CreateSecurityInfo [sslsecur.c:777]
ssl_NewSocket [sslsock.c:2122]
ssl_DupSocket [sslsock.c:243]
SSL_ImportFD [sslsock.c:1116]
apSSLServer::AcceptProtocol(apIProtocolServer*, apISocketWrapper*)
[apSSLServer.cpp:356]

18432 bytes
PR_Malloc
PORT_Alloc
sslBuffer_Grow [sslsecur.c:423]
ssl_SaveWriteData [sslsecur.c:451]
4ssl3_SendRecord [ssl3con.c:1985]
ssl3_SendChangeCipherSpecs [ssl3con.c:2440]
ssl3_HandleFinished [ssl3con.c:7474]
ssl3_HandleHandshakeMessage [ssl3con.c:7723]
ssl3_HandleHandshake [ssl3con.c:7791]
ssl3_HandleRecord [ssl3con.c:8054]
ssl3_GatherCompleteHandshake [ssl3gthr.c:206]
ssl_GatherRecord1stHandshake [sslcon.c:1258]
ssl_Do1stHandshake [sslsecur.c:149]
ssl_SecureRecv [sslsecur.c:1032]
ssl_Recv [sslsock.c:1352]
PR_Recv [priometh.c:221]
apFileDescWrapper::Peek(void*, unsigned int, unsigned
int*)[apFileDescWrapper.cpp:251]

18432 bytes
PR_Malloc
PORT_Alloc
sslBuffer_Grow [sslsecur.c:423]
ssl_InitGather [sslgathr.c:437]
ssl_NewSocket [sslsock.c:2125]
ssl_DupSocket [sslsock.c:243]
SSL_ImportFD [sslsock.c:1116]
apSSLServer::AcceptProtocol(apIProtocolServer*, apISocketWrapper*)
[apSSLServer.cpp:356]

18432 bytes
PR_Malloc
PORT_Alloc
sslBuffer_Grow [sslsecur.c:423]
ssl3_GatherData [ssl3gthr.c:149]
ssl3_GatherCompleteHandshake [ssl3gthr.c:195]
ssl_GatherRecord1stHandshake [sslcon.c:1258]
ssl_Do1stHandshake [sslsecur.c:149]
ssl_SecureRecv [sslsecur.c:1032]
ssl_Recv [sslsock.c:1352]
PR_Recv [priometh.c:221]
apFileDescWrapper::Peek(void*, unsigned int, unsigned int*)
[apFileDescWrapper.cpp:251]

11272 bytes
PR_Calloc [prmem.c:474]
PORT_ZAlloc [secport.c:140]
ssl_NewSocket [sslsock.c:2074]
ssl_DupSocket [sslsock.c:243]
apSSLServer::AcceptProtocol(apIProtocolServer*, apISocketWrapper*)
[apSSLServer.cpp:356]

Nelson B. wrote in the newsgroup:
Your results show that there are 5 different buffers being used, 
 and the same minimum is being applied to them all.  I think some 
 of those buffers can use much lower minumums without hurting 
 performance.

Reproducible: Always

Steps to Reproduce:
1. Listen on a socket and accept is as SSL socket
2. Connect to the socket

Actual Results:  
As described in details: 5 buffers of 18Kb are allocated.

Expected Results:  
Less memory should be needed.

Comment 1

[Nelson Bolyard (seldom reads bugmail)]

The 5 buffers shown being allocated are:
ss->sec.ci.sendBuf  (buffered outgoing handshake messages, not SSL records)
ss->sec.writeBuf    (outgoing SSL record buffer [before & after encryption])
ss->pendingBuf      (buffered encrypted partially-sent outgoing SSL records)
ss->gs.buf          (buffer of received decrypted SSL2/SSL3 records)
ss->gs.inbuf        (buffer of received encrypted SSL3 records)

Prior to NSS 3.11, these buffers were allocated to the minimum size necessary
for the record being sent/received, and then were reallocated any time a 
larger buffer was needed.  That strategy resulted in NUMEROUS reallocations
for every SSL connection, which hurt server performance and really impacted 
the host lock that protects the heap.  So, as part of the performance 
improvement work for 3.11, we eliminated all reallocations by initially 
allocating each of those buffers to the largest size that SSL3 record 
buffers would ever need to be to hold a single SSL2/TLS record of maximum
length.  (Note that SSL2's maximum record size is twice that of SSL3.)
That was done by the addition of one line to sslBuffer_Grow:

 sslBuffer_Grow(sslBuffer *b, unsigned int newLen)
 {
+    newLen = PR_MAX(newLen, MAX_FRAGMENT_LENGTH + 2048);

If we reduce the size of the initial allocation, we will reintroduce the
realloc calls, AND the buffers are still very likely to grow (be realloc'ed) 
up to this maximum size during the course of the SSL connection, especially
in the cases where the client POSTs a file to the server, or the server 
downloads a large page to the client.  Only the tiniest transactions would
ever avoid maximizing the sizes of these buffers.

One area to investigate is the possibility of using fewer buffers.  
SSL2 uses a single input buffer and decrypts incoming records in place.  
Maybe SSL3/TLS could do the same.  But SSL3/TLS supports record compression, which (if/when we implement it) would necessitate two buffers, as SSL3 uses 
now.  

Some other possible ideas:
a) free the buffer of outgoing handshake messages when the handshake is done.
b) Initially allocate the pending buffer to a smaller size, and grow it by
powers of 2 to minimize number of reallocations.  e.g. 4k -> 8k -> 16k. 

Comment 2

[Michal Novotny [:michal]]

> Prior to NSS 3.11, these buffers were allocated to the minimum size necessary
> for the record being sent/received, and then were reallocated any time a 
> larger buffer was needed.  That strategy resulted in NUMEROUS reallocations
> for every SSL connection, which hurt server performance and really impacted 
> the host lock that protects the heap.  So, as part of the performance 
> improvement work for 3.11, we eliminated all reallocations by initially 
> allocating each of those buffers to the largest size that SSL3 record 
> buffers would ever need to be to hold a single SSL2/TLS record of maximum
> length.  (Note that SSL2's maximum record size is twice that of SSL3.)
> That was done by the addition of one line to sslBuffer_Grow:

Is there any possibility to limit maximum size of SSL3 record? I understand that RFC tells that SSL3 record can be max 2^14 bytes, and therefore as receiving side you must be able to handle such record. But if we can limit records that we are sending (lets say to 4096 bytes) the server side will never need more that 4096. I'm not sure if this would be working. If yes then limit outgoing max record size + growing incoming buffer (4k -> 8k ->16k) would be good solution for us.

Comment 4

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Brian, is this something MemShrink should have on its radar?

Comment 5

[Brian Smith (:briansmith, :bsmith, use NEEDINFO?)]

No. This is a very minor thing, and I hope that we will do this buffering a completely different way by the end of the year (assuming we implement TLS 1.2).