You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@mina.apache.org by Emmanuel Lécharny <el...@symas.com> on 2013/05/03 15:19:26 UTC
[MINA 3] SSL and fragmentation
Hi guys,
the way we handle incomming SSL data on the server will not work if the
data are fragmented.
Let's say we are receiving some Handshake data. This is typically a few
hundreds of bytes, that usually comes in one block. In this case, we are
all good. But if those data aren't read in one block, then we are dead.
I have simulated such a fragmentation by sending a ClientHello block of
data in three parts : as soon as the SslEngine if given the second block
of data, it throws an exception :
2013-05-03 15:07:15,546 ERROR [SelectorWorker Server-I/O-1]
tcp.NioTcpSession (NioTcpSession.java:330) - Exception while reading :
javax.net.ssl.SSLException: Unsupported record version Unknown-0.0
at
sun.security.ssl.EngineInputRecord.bytesInCompletePacket(EngineInputRecord.java:116)
at sun.security.ssl.SSLEngineImpl.readNetRecord(SSLEngineImpl.java:845)
at sun.security.ssl.SSLEngineImpl.unwrap(SSLEngineImpl.java:758)
at javax.net.ssl.SSLEngine.unwrap(SSLEngine.java:624)
at org.apache.mina.session.SslHelper.unwrap(SslHelper.java:190)
This is plain normal : when we read the first block of data, we unwrap
it, and the SslEngine resturn with a BUFFER_UNDERFLOW status (expected).
Now, we return back to the main loop, and we *clear* the reaBuffer. We
then lose the first bytes we have read, parts that the SslEngine is
expecting to have the next time we call it.
There is one way to avoid such a problem : we just have to compact the
buffer instead of clearing it.
Now, regarding the SSL code, it's really messy atm. The fact that we
separate the handshake from the normal processing of data is wrong. When
we do an unwrap, it will initiate a handshake if needed. The fact is we
have to review the full code and rewrite it...
I'm currently doing some experiments here, but I would be please to
share ideas, suggestion, proposals from any of you...
Re: [MINA 3] SSL and fragmentation
Posted by Julien Vermillard <jv...@gmail.com>.
On extending java.nio.ByteBuffer : it's not doable because the constructor
is private :(
On Fri, May 3, 2013 at 3:40 PM, Jeff MAURY <je...@jeffmaury.com> wrote:
> +1
>
> I think we shall rewrite the code and let be driven by the underlying
> SSLengine status.
> Now, if after processing there are still remaining data in the input
> buffer, we must keep it until the next buffer is received.
> In order to preserve performances, as we need to assemble the input buffer
> with the rest of the previous input buffer, I had the idea of having a
> CompositeReadOnlyByteBuffer that extends ByteBuffer and that will be
> composed of the two buffers. This will prevent to perform costly copies
> while allowing the SSLEngine to see the whole data.
> Any thoughts ?
>
> Jeff
>
>
> On Fri, May 3, 2013 at 3:19 PM, Emmanuel Lécharny <elecharny@symas.com
> >wrote:
>
> > Hi guys,
> >
> > the way we handle incomming SSL data on the server will not work if the
> > data are fragmented.
> >
> > Let's say we are receiving some Handshake data. This is typically a few
> > hundreds of bytes, that usually comes in one block. In this case, we are
> > all good. But if those data aren't read in one block, then we are dead.
> >
> > I have simulated such a fragmentation by sending a ClientHello block of
> > data in three parts : as soon as the SslEngine if given the second block
> > of data, it throws an exception :
> > 2013-05-03 15:07:15,546 ERROR [SelectorWorker Server-I/O-1]
> > tcp.NioTcpSession (NioTcpSession.java:330) - Exception while reading :
> > javax.net.ssl.SSLException: Unsupported record version Unknown-0.0
> > at
> >
> >
> sun.security.ssl.EngineInputRecord.bytesInCompletePacket(EngineInputRecord.java:116)
> > at
> sun.security.ssl.SSLEngineImpl.readNetRecord(SSLEngineImpl.java:845)
> > at sun.security.ssl.SSLEngineImpl.unwrap(SSLEngineImpl.java:758)
> > at javax.net.ssl.SSLEngine.unwrap(SSLEngine.java:624)
> > at org.apache.mina.session.SslHelper.unwrap(SslHelper.java:190)
> >
> >
> > This is plain normal : when we read the first block of data, we unwrap
> > it, and the SslEngine resturn with a BUFFER_UNDERFLOW status (expected).
> > Now, we return back to the main loop, and we *clear* the reaBuffer. We
> > then lose the first bytes we have read, parts that the SslEngine is
> > expecting to have the next time we call it.
> >
> > There is one way to avoid such a problem : we just have to compact the
> > buffer instead of clearing it.
> >
> > Now, regarding the SSL code, it's really messy atm. The fact that we
> > separate the handshake from the normal processing of data is wrong. When
> > we do an unwrap, it will initiate a handshake if needed. The fact is we
> > have to review the full code and rewrite it...
> >
> > I'm currently doing some experiments here, but I would be please to
> > share ideas, suggestion, proposals from any of you...
> >
>
>
>
> --
> Jeff MAURY
>
>
> "Legacy code" often differs from its suggested alternative by actually
> working and scaling.
> - Bjarne Stroustrup
>
> http://www.jeffmaury.com
> http://riadiscuss.jeffmaury.com
> http://www.twitter.com/jeffmaury
>
Re: [MINA 3] SSL and fragmentation
Posted by Emmanuel Lécharny <el...@gmail.com>.
Le 5/3/13 4:45 PM, Jeff MAURY a écrit :
> On Fri, May 3, 2013 at 3:47 PM, Emmanuel Lécharny <el...@gmail.com>wrote:
>
>> Le 5/3/13 3:40 PM, Jeff MAURY a écrit :
>>> +1
>>>
>>> I think we shall rewrite the code and let be driven by the underlying
>>> SSLengine status.
>> Absolutely. I have created a branch to play a bit with SSL.
>>> Now, if after processing there are still remaining data in the input
>>> buffer, we must keep it until the next buffer is received.
>> Thus the compact().
>>
>> The pb is for the normal calls (ie, not SSL- as the buffer can be
>> propagated up to the user. In this case, if we compact the buffer, we
>> may get some weaird side effect if the user has done some weird things
>> wih the buffer.
>>
> Why do we need to take care of non SSL configuration in this SSL discussion
> ?
Sorry, I was unclear. I meant when we have already processed the
Handshake. The unwrapped buffer will be probagated to the user, and can
potentially be modified (even if it's a mistake).
Not sure this is a big issue atm...
>
>
>>> In order to preserve performances, as we need to assemble the input
>> buffer
>>> with the rest of the previous input buffer, I had the idea of having a
>>> CompositeReadOnlyByteBuffer that extends ByteBuffer and that will be
>>> composed of the two buffers. This will prevent to perform costly copies
>>> while allowing the SSLEngine to see the whole data.
>>> Any thoughts ?
>> That's an option.
>>
>> However, internally, the SslEngine does copy the ByteBuffer if it's not
>> a Direct buffer. So I'm not sure we can spare any CPU time by
>> concatenating the received ByteBuffer into a composite structure.
>>
> I think so as I suspect the SSLEngine to be message oriented so when an
> input buffer is given, then the SLL engine will read the first bytes to
> decode the length and if not enough data is present, it will throw a
> BufferUnderflow so only the first bytes of the input buffer will be read
> and taht's why we need to concatenate.
This is most certainly what the SSLEngine does. A SSL message typically
starts with a heade :
b[0] : ContentType {0x14:ChangeCipherSpec, 0x15:Alert, 0x16:Handshake,
0x17:ApplicationData}
b[1-2] : Version {3.0 : SSL 3.0, 3.1 : TLS 1.0, 3.2 : TLS 1.1, 3.3 : TLS
1.2}
b[3-4] : length <= 2^14 (16Kb)
So if it gets at least 5 bytes, the SslEngine can know how many bytes to
expect.
>
>> I mean, if we pass a DirectBuffer to the SslEngine, it will be faster.
>> If this DirectBuffer does not contain enough data to produce an
>> unwrapped buffer, the we will have to pass a new DirectBuffer containing
>> the previous data (copied from the previous DirectBuffer) plus the newly
>> read data. In any case, we have some copy...
>>
> I don't think so.
If we reuse the readBuffer (which is allocated once per session),
without clearing it between reads, and if it's a DirectBuffer, then I
think there is no copy done.
--
Regards,
Cordialement,
Emmanuel Lécharny
www.iktek.com
Re: [MINA 3] SSL and fragmentation
Posted by Jeff MAURY <je...@jeffmaury.com>.
On Fri, May 3, 2013 at 3:47 PM, Emmanuel Lécharny <el...@gmail.com>wrote:
> Le 5/3/13 3:40 PM, Jeff MAURY a écrit :
> > +1
> >
> > I think we shall rewrite the code and let be driven by the underlying
> > SSLengine status.
> Absolutely. I have created a branch to play a bit with SSL.
> > Now, if after processing there are still remaining data in the input
> > buffer, we must keep it until the next buffer is received.
> Thus the compact().
>
> The pb is for the normal calls (ie, not SSL- as the buffer can be
> propagated up to the user. In this case, if we compact the buffer, we
> may get some weaird side effect if the user has done some weird things
> wih the buffer.
>
Why do we need to take care of non SSL configuration in this SSL discussion
?
> > In order to preserve performances, as we need to assemble the input
> buffer
> > with the rest of the previous input buffer, I had the idea of having a
> > CompositeReadOnlyByteBuffer that extends ByteBuffer and that will be
> > composed of the two buffers. This will prevent to perform costly copies
> > while allowing the SSLEngine to see the whole data.
> > Any thoughts ?
> That's an option.
>
> However, internally, the SslEngine does copy the ByteBuffer if it's not
> a Direct buffer. So I'm not sure we can spare any CPU time by
> concatenating the received ByteBuffer into a composite structure.
>
I think so as I suspect the SSLEngine to be message oriented so when an
input buffer is given, then the SLL engine will read the first bytes to
decode the length and if not enough data is present, it will throw a
BufferUnderflow so only the first bytes of the input buffer will be read
and taht's why we need to concatenate.
>
> I mean, if we pass a DirectBuffer to the SslEngine, it will be faster.
> If this DirectBuffer does not contain enough data to produce an
> unwrapped buffer, the we will have to pass a new DirectBuffer containing
> the previous data (copied from the previous DirectBuffer) plus the newly
> read data. In any case, we have some copy...
>
I don't think so.
Jeff
>
>
> --
> Regards,
> Cordialement,
> Emmanuel Lécharny
> www.iktek.com
>
>
--
Jeff MAURY
"Legacy code" often differs from its suggested alternative by actually
working and scaling.
- Bjarne Stroustrup
http://www.jeffmaury.com
http://riadiscuss.jeffmaury.com
http://www.twitter.com/jeffmaury
Re: [MINA 3] SSL and fragmentation
Posted by Emmanuel Lécharny <el...@gmail.com>.
Le 5/3/13 3:40 PM, Jeff MAURY a écrit :
> +1
>
> I think we shall rewrite the code and let be driven by the underlying
> SSLengine status.
Absolutely. I have created a branch to play a bit with SSL.
> Now, if after processing there are still remaining data in the input
> buffer, we must keep it until the next buffer is received.
Thus the compact().
The pb is for the normal calls (ie, not SSL- as the buffer can be
propagated up to the user. In this case, if we compact the buffer, we
may get some weaird side effect if the user has done some weird things
wih the buffer.
> In order to preserve performances, as we need to assemble the input buffer
> with the rest of the previous input buffer, I had the idea of having a
> CompositeReadOnlyByteBuffer that extends ByteBuffer and that will be
> composed of the two buffers. This will prevent to perform costly copies
> while allowing the SSLEngine to see the whole data.
> Any thoughts ?
That's an option.
However, internally, the SslEngine does copy the ByteBuffer if it's not
a Direct buffer. So I'm not sure we can spare any CPU time by
concatenating the received ByteBuffer into a composite structure.
I mean, if we pass a DirectBuffer to the SslEngine, it will be faster.
If this DirectBuffer does not contain enough data to produce an
unwrapped buffer, the we will have to pass a new DirectBuffer containing
the previous data (copied from the previous DirectBuffer) plus the newly
read data. In any case, we have some copy...
--
Regards,
Cordialement,
Emmanuel Lécharny
www.iktek.com
Re: [MINA 3] SSL and fragmentation
Posted by Jeff MAURY <je...@jeffmaury.com>.
+1
I think we shall rewrite the code and let be driven by the underlying
SSLengine status.
Now, if after processing there are still remaining data in the input
buffer, we must keep it until the next buffer is received.
In order to preserve performances, as we need to assemble the input buffer
with the rest of the previous input buffer, I had the idea of having a
CompositeReadOnlyByteBuffer that extends ByteBuffer and that will be
composed of the two buffers. This will prevent to perform costly copies
while allowing the SSLEngine to see the whole data.
Any thoughts ?
Jeff
On Fri, May 3, 2013 at 3:19 PM, Emmanuel Lécharny <el...@symas.com>wrote:
> Hi guys,
>
> the way we handle incomming SSL data on the server will not work if the
> data are fragmented.
>
> Let's say we are receiving some Handshake data. This is typically a few
> hundreds of bytes, that usually comes in one block. In this case, we are
> all good. But if those data aren't read in one block, then we are dead.
>
> I have simulated such a fragmentation by sending a ClientHello block of
> data in three parts : as soon as the SslEngine if given the second block
> of data, it throws an exception :
> 2013-05-03 15:07:15,546 ERROR [SelectorWorker Server-I/O-1]
> tcp.NioTcpSession (NioTcpSession.java:330) - Exception while reading :
> javax.net.ssl.SSLException: Unsupported record version Unknown-0.0
> at
>
> sun.security.ssl.EngineInputRecord.bytesInCompletePacket(EngineInputRecord.java:116)
> at sun.security.ssl.SSLEngineImpl.readNetRecord(SSLEngineImpl.java:845)
> at sun.security.ssl.SSLEngineImpl.unwrap(SSLEngineImpl.java:758)
> at javax.net.ssl.SSLEngine.unwrap(SSLEngine.java:624)
> at org.apache.mina.session.SslHelper.unwrap(SslHelper.java:190)
>
>
> This is plain normal : when we read the first block of data, we unwrap
> it, and the SslEngine resturn with a BUFFER_UNDERFLOW status (expected).
> Now, we return back to the main loop, and we *clear* the reaBuffer. We
> then lose the first bytes we have read, parts that the SslEngine is
> expecting to have the next time we call it.
>
> There is one way to avoid such a problem : we just have to compact the
> buffer instead of clearing it.
>
> Now, regarding the SSL code, it's really messy atm. The fact that we
> separate the handshake from the normal processing of data is wrong. When
> we do an unwrap, it will initiate a handshake if needed. The fact is we
> have to review the full code and rewrite it...
>
> I'm currently doing some experiments here, but I would be please to
> share ideas, suggestion, proposals from any of you...
>
--
Jeff MAURY
"Legacy code" often differs from its suggested alternative by actually
working and scaling.
- Bjarne Stroustrup
http://www.jeffmaury.com
http://riadiscuss.jeffmaury.com
http://www.twitter.com/jeffmaury