You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@mina.apache.org by Jeff MAURY <je...@jeffmaury.com> on 2014/10/07 23:37:04 UTC
More SSL thoughts
Hello,
as I'm working on the SSL part this time and more specifically on the
handshake/rehandshake processing, I have a couple of questions and some
infos to share:
- I've added 3 more methods in IoHandler to reflect handshake related
event: handshakeStarted, handshakeCompleted and secureClosed. I've added
them as well to IoFilter but I don't quite understand the philosophy as
some method have a chain controller to call the next filter and some not
- In order to support rehandshaking et being efficient, we must keep the
same SSLEngine. So my idea to start a new handshake was to reuse what we
have today through the initSecure method: if the SSLContext is null, then
the rehandkshake is started if we already have an initialized SSLHandler
attached to the session. If SSLContext is null and no SSLHandler is
attached to the session, then an exception (IllegalState ?) will be
through. If an SSLContext is attached and an SSLHandler is attached to the
session, then a new SSLEngine is build. WDYT ?
Jeff
--
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
[MINA3] Re: More SSL thoughts
Posted by Emmanuel Lécharny <el...@gmail.com>.
Replying again, but this time with MINA 3 in mind (please add a tage in
front of teh subject, i order to avoid confusion : I was in MINA 2 mode
those last 3 weeks...)
Le 07/10/14 23:37, Jeff MAURY a écrit :
> Hello,
>
> as I'm working on the SSL part this time and more specifically on the
> handshake/rehandshake processing, I have a couple of questions and some
> infos to share:
>
> - I've added 3 more methods in IoHandler to reflect handshake related
> event: handshakeStarted, handshakeCompleted and secureClosed. I've added
> them as well to IoFilter but I don't quite understand the philosophy as
> some method have a chain controller to call the next filter and some not
This has to be reviewed and fixed.
I just had a look at the IoFilter hierarchy, and there are many wrong
things :
- The LoggingFilter is not extending the AbstractIoFilter class, which
is wrong
- It does not propagate all the events (messageSent, for instance),
which is totally broken
- I think each of the implementation method should call the super
method, not the controller itself. ie, doing things like :
controller.callReadNextFilter(coapMsg);
is not te right way. We should do :
super.messageReceived(session, message, controller);
> - In order to support rehandshaking et being efficient, we must keep the
> same SSLEngine. So my idea to start a new handshake was to reuse what we
> have today through the initSecure method: if the SSLContext is null, then
> the rehandkshake is started if we already have an initialized SSLHandler
> attached to the session. If SSLContext is null and no SSLHandler is
> attached to the session, then an exception (IllegalState ?) will be
> through. If an SSLContext is attached and an SSLHandler is attached to the
> session, then a new SSLEngine is build. WDYT ?
That sounds like teh right way to handle re-handshake, as a standard SSL
start.
Re: More SSL thoughts
Posted by Emmanuel Lécharny <el...@gmail.com>.
Le 11/10/14 08:38, Jeff MAURY a écrit :
> On Sat, Oct 11, 2014 at 8:24 AM, Emmanuel Lécharny <el...@gmail.com>
> wrote:
>
>
> I was mentioning the SSLContext that is the argument of the initSecure
> method. Please note that in 3.0, there is no more SSLFilter as SSL
>> handling
>>> has been moved to core.
>>> So my idea is that if no SSLContext is given to initSecure and SSLHandler
>>> is attached to the session, we keep the same underlying SSLEngine and we
>>> start a new handshake
>> I see. You propose to handle the handshake re-negociation through a call
>> to a newly added method called initSecure( SslContext ) in MINA 2, is
>> that correct ?
>>
> No, I was talking about MINA3 and about the semantic of this existing
> method.
F**** me... I thought you were mentionning MINA 2.0 all along this thread :/
Re: More SSL thoughts
Posted by Jeff MAURY <je...@jeffmaury.com>.
On Sat, Oct 11, 2014 at 8:24 AM, Emmanuel Lécharny <el...@gmail.com>
wrote:
> Le 08/10/14 11:45, Jeff MAURY a écrit :
> > On Wed, Oct 8, 2014 at 10:33 AM, Emmanuel Lécharny <el...@gmail.com>
> > wrote:
> >
> >> Le 07/10/14 23:37, Jeff MAURY a écrit :
> >>> Hello,
> >>>
> >>> as I'm working on the SSL part this time and more specifically on the
> >>> handshake/rehandshake processing, I have a couple of questions and some
> >>> infos to share:
> >>>
> >>> - I've added 3 more methods in IoHandler to reflect handshake
> related
> >>> event: handshakeStarted, handshakeCompleted and secureClosed. I've
> >> added
> >>> them as well to IoFilter but I don't quite understand the philosophy
> >> as
> >>> some method have a chain controller to call the next filter and some
> >> not
> >> The idea behind the chain of filters is that any event is propagated up
> >> to the final filter (ie, the Iohandler) by each filter. If one filter
> >> decide not to propagate the event, then obviously the IoHandler will not
> >> receive it. This is thus to the Filter implementer to be sure it does
> >> propagate the event to teh next filter. If it does not, this is either a
> >> mistake, or a decision that has to be heavily and carefully thought.
> >>
> >> The pb is to delegate this responsability to the filter. It would be
> >> easier if the controller was to propagate the event further without
> >> expecting teh filter to do so. That woudl require some careful rework of
> >> the controller, as in some case (like errors, exceptions, etc) we don't
> >> want to propagate the event.
> >>
> >> In some other cases, we simply want the filter to handle the event
> >> propagation (typically this is the case for the MessageReceived when we
> >> are using a decoder filter : there is no mean to propagate the event
> >> automatically if a full message has not been decocded).
> >>
> >> This is definitively something we want to think about.
> >>
> > What I didn't understand is that not all of IOFilter method signatures
> have
> > a ChainController so I did not understand how they can decide to swallow
> > the event or not.
>
> Not sure what you mean by "not all of IOFilter method signatures have a
> ChainController" in Mina 2.0 context.
>
> The IoFilter class has 3 sets of methods :
> - methods that propagate an event :
> o exceptionCaugth
> o filterClose
> o filterWrite
> o inputClose
> o messageReceived
> o messageSent
> o sessionClosed
> o sessionCreated
> o sessionIdle
> o sessionOpened
>
I don't agree for MINA 3. See
https://github.com/apache/mina/blob/trunk/core/src/main/java/org/apache/mina/api/IoFilter.java
>
> Those methods have a NextFilter parameter, which is the filter that
> will receive the event, should the current filter decide to propagate
> it. This can be seen in, for instance, the BlackListFilter :
>
> public void messageSent(NextFilter nextFilter, IoSession session,
> WriteRequest writeRequest) throws Exception {
> if (!isBlocked(session)) {
> // forward if not blocked
> nextFilter.messageSent(session, writeRequest);
> } else {
> blockSession(session);
> }
> }
>
>
> - methods that manage the chain manipulation :
> o onPreAdd
> o onPostAdd
> o onPreRemove
> o onPostRemove
>
> Those methods just react on the addition or removal of the current
> filter from the session chain.
>
> - general methods :
> o init
> o destroy
>
> Those methods are called when the filter is created or destroyed.
>
> Note that all the filter extends the IoFilterAdapter which already have
> a default implementation for all those methods (hence it should be an
> abstract method, IMO).
>
> Is that what you wanted to get some clarification on ?
>
> >
> >>> - In order to support rehandshaking et being efficient, we must keep
> >> the
> >>> same SSLEngine.
> >> Ok, makes sense.
> >>> So my idea to start a new handshake was to reuse what we
> >>> have today through the initSecure method: if the SSLContext is null,
> >> I don't see how we can have a null SSLContext, as we create it before
> >> creating the SSLFilter, and there is a check for nullity in this
> >> constructor :
> >>
> >> public SslFilter(SSLContext sslContext, boolean autoStart) {
> >> if (sslContext == null) {
> >> throw new IllegalArgumentException("sslContext");
> >> }
> >>
> >> Assuming we always have a not null SSLContext, how does it translates in
> >> your proposed algorithm ?
>>
> > I was mentioning the SSLContext that is the argument of the initSecure
> > method. Please note that in 3.0, there is no more SSLFilter as SSL
> handling
> > has been moved to core.
> > So my idea is that if no SSLContext is given to initSecure and SSLHandler
> > is attached to the session, we keep the same underlying SSLEngine and we
> > start a new handshake
>
> I see. You propose to handle the handshake re-negociation through a call
> to a newly added method called initSecure( SslContext ) in MINA 2, is
> that correct ?
>
No, I was talking about MINA3 and about the semantic of this existing
method.
Jeff
--
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: More SSL thoughts
Posted by Emmanuel Lécharny <el...@gmail.com>.
Le 08/10/14 11:45, Jeff MAURY a écrit :
> On Wed, Oct 8, 2014 at 10:33 AM, Emmanuel Lécharny <el...@gmail.com>
> wrote:
>
>> Le 07/10/14 23:37, Jeff MAURY a écrit :
>>> Hello,
>>>
>>> as I'm working on the SSL part this time and more specifically on the
>>> handshake/rehandshake processing, I have a couple of questions and some
>>> infos to share:
>>>
>>> - I've added 3 more methods in IoHandler to reflect handshake related
>>> event: handshakeStarted, handshakeCompleted and secureClosed. I've
>> added
>>> them as well to IoFilter but I don't quite understand the philosophy
>> as
>>> some method have a chain controller to call the next filter and some
>> not
>> The idea behind the chain of filters is that any event is propagated up
>> to the final filter (ie, the Iohandler) by each filter. If one filter
>> decide not to propagate the event, then obviously the IoHandler will not
>> receive it. This is thus to the Filter implementer to be sure it does
>> propagate the event to teh next filter. If it does not, this is either a
>> mistake, or a decision that has to be heavily and carefully thought.
>>
>> The pb is to delegate this responsability to the filter. It would be
>> easier if the controller was to propagate the event further without
>> expecting teh filter to do so. That woudl require some careful rework of
>> the controller, as in some case (like errors, exceptions, etc) we don't
>> want to propagate the event.
>>
>> In some other cases, we simply want the filter to handle the event
>> propagation (typically this is the case for the MessageReceived when we
>> are using a decoder filter : there is no mean to propagate the event
>> automatically if a full message has not been decocded).
>>
>> This is definitively something we want to think about.
>>
> What I didn't understand is that not all of IOFilter method signatures have
> a ChainController so I did not understand how they can decide to swallow
> the event or not.
Not sure what you mean by "not all of IOFilter method signatures have a
ChainController" in Mina 2.0 context.
The IoFilter class has 3 sets of methods :
- methods that propagate an event :
o exceptionCaugth
o filterClose
o filterWrite
o inputClose
o messageReceived
o messageSent
o sessionClosed
o sessionCreated
o sessionIdle
o sessionOpened
Those methods have a NextFilter parameter, which is the filter that
will receive the event, should the current filter decide to propagate
it. This can be seen in, for instance, the BlackListFilter :
public void messageSent(NextFilter nextFilter, IoSession session,
WriteRequest writeRequest) throws Exception {
if (!isBlocked(session)) {
// forward if not blocked
nextFilter.messageSent(session, writeRequest);
} else {
blockSession(session);
}
}
- methods that manage the chain manipulation :
o onPreAdd
o onPostAdd
o onPreRemove
o onPostRemove
Those methods just react on the addition or removal of the current
filter from the session chain.
- general methods :
o init
o destroy
Those methods are called when the filter is created or destroyed.
Note that all the filter extends the IoFilterAdapter which already have
a default implementation for all those methods (hence it should be an
abstract method, IMO).
Is that what you wanted to get some clarification on ?
>
>>> - In order to support rehandshaking et being efficient, we must keep
>> the
>>> same SSLEngine.
>> Ok, makes sense.
>>> So my idea to start a new handshake was to reuse what we
>>> have today through the initSecure method: if the SSLContext is null,
>> I don't see how we can have a null SSLContext, as we create it before
>> creating the SSLFilter, and there is a check for nullity in this
>> constructor :
>>
>> public SslFilter(SSLContext sslContext, boolean autoStart) {
>> if (sslContext == null) {
>> throw new IllegalArgumentException("sslContext");
>> }
>>
>> Assuming we always have a not null SSLContext, how does it translates in
>> your proposed algorithm ?
>>
> I was mentioning the SSLContext that is the argument of the initSecure
> method. Please note that in 3.0, there is no more SSLFilter as SSL handling
> has been moved to core.
> So my idea is that if no SSLContext is given to initSecure and SSLHandler
> is attached to the session, we keep the same underlying SSLEngine and we
> start a new handshake
I see. You propose to handle the handshake re-negociation through a call
to a newly added method called initSecure( SslContext ) in MINA 2, is
that correct ?
Re: More SSL thoughts
Posted by Jeff MAURY <je...@jeffmaury.com>.
On Wed, Oct 8, 2014 at 10:33 AM, Emmanuel Lécharny <el...@gmail.com>
wrote:
> Le 07/10/14 23:37, Jeff MAURY a écrit :
> > Hello,
> >
> > as I'm working on the SSL part this time and more specifically on the
> > handshake/rehandshake processing, I have a couple of questions and some
> > infos to share:
> >
> > - I've added 3 more methods in IoHandler to reflect handshake related
> > event: handshakeStarted, handshakeCompleted and secureClosed. I've
> added
> > them as well to IoFilter but I don't quite understand the philosophy
> as
> > some method have a chain controller to call the next filter and some
> not
> The idea behind the chain of filters is that any event is propagated up
> to the final filter (ie, the Iohandler) by each filter. If one filter
> decide not to propagate the event, then obviously the IoHandler will not
> receive it. This is thus to the Filter implementer to be sure it does
> propagate the event to teh next filter. If it does not, this is either a
> mistake, or a decision that has to be heavily and carefully thought.
>
> The pb is to delegate this responsability to the filter. It would be
> easier if the controller was to propagate the event further without
> expecting teh filter to do so. That woudl require some careful rework of
> the controller, as in some case (like errors, exceptions, etc) we don't
> want to propagate the event.
>
> In some other cases, we simply want the filter to handle the event
> propagation (typically this is the case for the MessageReceived when we
> are using a decoder filter : there is no mean to propagate the event
> automatically if a full message has not been decocded).
>
> This is definitively something we want to think about.
>
What I didn't understand is that not all of IOFilter method signatures have
a ChainController so I did not understand how they can decide to swallow
the event or not.
>
> > - In order to support rehandshaking et being efficient, we must keep
> the
> > same SSLEngine.
> Ok, makes sense.
> > So my idea to start a new handshake was to reuse what we
> > have today through the initSecure method: if the SSLContext is null,
>
> I don't see how we can have a null SSLContext, as we create it before
> creating the SSLFilter, and there is a check for nullity in this
> constructor :
>
> public SslFilter(SSLContext sslContext, boolean autoStart) {
> if (sslContext == null) {
> throw new IllegalArgumentException("sslContext");
> }
>
> Assuming we always have a not null SSLContext, how does it translates in
> your proposed algorithm ?
>
I was mentioning the SSLContext that is the argument of the initSecure
method. Please note that in 3.0, there is no more SSLFilter as SSL handling
has been moved to core.
So my idea is that if no SSLContext is given to initSecure and SSLHandler
is attached to the session, we keep the same underlying SSLEngine and we
start a new handshake
Jeff
--
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: More SSL thoughts
Posted by Emmanuel Lécharny <el...@gmail.com>.
Le 07/10/14 23:37, Jeff MAURY a écrit :
> Hello,
>
> as I'm working on the SSL part this time and more specifically on the
> handshake/rehandshake processing, I have a couple of questions and some
> infos to share:
>
> - I've added 3 more methods in IoHandler to reflect handshake related
> event: handshakeStarted, handshakeCompleted and secureClosed. I've added
> them as well to IoFilter but I don't quite understand the philosophy as
> some method have a chain controller to call the next filter and some not
The idea behind the chain of filters is that any event is propagated up
to the final filter (ie, the Iohandler) by each filter. If one filter
decide not to propagate the event, then obviously the IoHandler will not
receive it. This is thus to the Filter implementer to be sure it does
propagate the event to teh next filter. If it does not, this is either a
mistake, or a decision that has to be heavily and carefully thought.
The pb is to delegate this responsability to the filter. It would be
easier if the controller was to propagate the event further without
expecting teh filter to do so. That woudl require some careful rework of
the controller, as in some case (like errors, exceptions, etc) we don't
want to propagate the event.
In some other cases, we simply want the filter to handle the event
propagation (typically this is the case for the MessageReceived when we
are using a decoder filter : there is no mean to propagate the event
automatically if a full message has not been decocded).
This is definitively something we want to think about.
> - In order to support rehandshaking et being efficient, we must keep the
> same SSLEngine.
Ok, makes sense.
> So my idea to start a new handshake was to reuse what we
> have today through the initSecure method: if the SSLContext is null,
I don't see how we can have a null SSLContext, as we create it before
creating the SSLFilter, and there is a check for nullity in this
constructor :
public SslFilter(SSLContext sslContext, boolean autoStart) {
if (sslContext == null) {
throw new IllegalArgumentException("sslContext");
}
Assuming we always have a not null SSLContext, how does it translates in
your proposed algorithm ?