Issue 163 in jain-sip: Continues Overflow in SSLStateMachine when unwrapping

[jain...@googlecode.com]

Status: Accepted
Owner: jean.deruelle
Labels: Type-Defect Priority-Medium

New issue 163 by jean.deruelle: Continues Overflow in SSLStateMachine when
unwrapping
https://code.google.com/p/jain-sip/issues/detail?id=163

NOTE: we have not been able to reproduce and as such we do not know if our
suggested fix actually would work. For now, we really just want your input
and let you guys be aware that there is an issue with how the engine is
handling buffer overflows. So far, it has taken out one node in production
but since this is on TLS, we do not have any pcaps.

Symptom: 100% CPU and TLS Port unresponsive. Log flooded with "Buffer
overflow , must prepare the buffer again. Check for continious overflow
here?"

A note on the log. In the version of the jain sip library we were running,
this was logged on INFO but now seems to have changed to DEBUG. We
obviously do not run with DEBUG log on in production.

The line where this comes from is in SSLStateMachine.unwrap(ByteBuffer src,
ByteBuffer dst) and it really seems that the log line is a TODO since it
actually says "Check for continious overflow here?". We suspect that this
is what actually hit us. I.e., there were continuous overflows and that
took the SSLStateMachine out of play and the machine too since it ended up
in an infinite loop (just a theory based on the log message flooding and
100% CPU). A suggestion is to e.g. do:

1. Add a max overflow limit:

private static final int OVERFLOW_LIMIT = 10;

2. Have a counter for each instance of the state machine:

private int overflowCount = 0;

3. And then simply count:

if(result.getStatus().equals(Status.BUFFER_OVERFLOW)) {
if(logger.isLoggingEnabled(LogWriter.TRACE_DEBUG)) {
logger.logDebug("Buffer overflow , must prepare the buffer again. Check for
continious overflow here?");
}
if (++this.overflowCount > OVERFLOW_LIMIT) {
throw new SSLException("SSL unwrap: too much overflow");
}
dst = channel.prepareAppDataBuffer();
continue;
} else {
this.overflowCount = 0;
}

or something...

Btw, just a suggestion as well:

result.getStatus().equals(Status.BUFFER_OVERFLOW)

could lead to a NPE if result.getStatus() would return null (bug or
whatever)

Status.BUFFER_OVERFLOW.equals(result.getStats()) would never blow up.

--
You received this message because this project is configured to send all
issue notifications to this address.
You may adjust your notification preferences at:
https://code.google.com/hosting/settings

[jain...@googlecode.com]

Comment #1 on issue 163 by jean.deruelle: Continues Overflow in

SSLStateMachine when unwrapping
https://code.google.com/p/jain-sip/issues/detail?id=163

I think the issue is that the overflow is not handled at all and keep
looping on a buffer of the same size. So your fix will make sure that the
CPU doesn't get to 100% as it will exit after a number of tries but will
not solve the issue of accepting the big packet I think which will lead to
lost calls or call setup.

I think a better fix would be to increase the buffer size and retry such as
the code snippet below

if(result.getStatus().equals(Status.BUFFER_OVERFLOW)) {
if(logger.isLoggingEnabled(LogWriter.TRACE_DEBUG)) {
logger.logDebug("Buffer overflow , must prepare the buffer again."

+ " outNetBuffer remaining: " + dst.remaining()
+ " outNetBuffer postion: " + dst.position()
+ " Packet buffer size: " + sslEngine.getSession().getPacketBufferSize()
+ " new buffer size: " + sslEngine.getSession().getPacketBufferSize() +
dst.position());
}
ByteBuffer newBuf =
channel.prepareAppDataBuffer(sslEngine.getSession().getPacketBufferSize() +
dst.position());
dst.flip();
newBuf.put(dst);
dst = newBuf;
if(logger.isLoggingEnabled(LogWriter.TRACE_DEBUG)) {
logger.logDebug(" new outNetBuffer remaining: " + dst.remaining()
+ " new outNetBuffer postion: " + dst.position());
}
continue;

[jain...@googlecode.com]

Comment #2 on issue 163 by jean.deruelle: Continues Overflow in

SSLStateMachine when unwrapping
https://code.google.com/p/jain-sip/issues/detail?id=163

https://telestax.atlassian.net/browse/JSIP-27 &
https://java.net/jira/browse/JSIP-502

[jain...@googlecode.com]

Comment #3 on issue 163 by jean.der...@telestax.com: Continues Overflow in

SSLStateMachine when unwrapping
https://code.google.com/p/jain-sip/issues/detail?id=163

This issue was updated by revision 5229e474fec7.


JSIP-502

Fix + Fix for JSIP-483
(cherry picked from commit 181e3ea87ca39cc9a840276e3beade7bc8055c84)

[jain...@googlecode.com]

Updates:
Status: Fixed

Comment #4 on issue 163 by jean.deruelle: Continues Overflow in

SSLStateMachine when unwrapping
https://code.google.com/p/jain-sip/issues/detail?id=163

(No comment was entered for this change.)