Apache Directory Project - [mina] SSLFilter race condition: Take #2

Author

[mina] SSLFilter race condition: Take #2

Trustin Lee

2005-10-12, 5:45 pm

Hi,

We've discussed about implementing StartTLS using SSLFilter before without
any resolution:

http://www.archivum.info/dev%40dire...9/msg00005.html

and here's the related JIRA issue which is marked as a blocker:

http://issues.apache.org/jira/browse/DIRMINA-85

The cause of the problem is that client sends SSL handshake request before
SSLFilter is in action. Possible handler code will look like this:

public void messageReceived(IoSession session, Object message) {
....
if (message instanceof MyStartTLSRequest) {
// insert SSLFilter to start handshaking
session.getFilterChain().addFirst(sslFilter); -- (1)
// write StartTLSResponse
sessios.write(new MyStartTLSResponse(OK)); -- (2)
}
}

(2) won't work as we expected because we've installed SSLFilter at (1).
There should be some way for a developer to send a message bypassing
SSLFilter. Otherwise we need a traffic control mechanism here so that we can
do this in a right order:

public void messageReceived(IoSession session, Object message) {
....
if (message instanceof MyStartTLSRequest) {
// Suspend writing
session.suspendWrite();
// write StartTLSResponse
session.write(new MyStartTLSResponse(OK));
// insert SSLFilter to start handshaking
session.getFilterChain().addFirst(sslFilter);
// now resume writing
session.resumeWrite();
}
}

If we use the first method (bypassing), we could provide a tag interface
called SSLBypassMessage and make MyStartTLSResponse implement it. And then
SSLFilter will detect any messages that should bypass and let it go.

Which way do you prefer to the other?

Trustin
--
what we call human nature is actually human habit
--
http://gleamynode.net/

Trustin Lee

2005-10-13, 5:45 pm

2005/10/12, Trustin Lee <trustin-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>:
>
> Which way do you prefer to the other?

Here's another option:

public void messageReceived(IoSession session, Object message) {
....
if (message instanceof MyStartTLSRequest) {
// insert SSLFilter to start handshaking
session.getFilterChain().addFirst(sslFilter);

// Disable encryption for the specified message type.
sslFilter.excludeMessageType(MyStartTLSResponse.class);

// write StartTLSResponse
sessios.write(new MyStartTLSResponse(OK));
}
}

WDYT? Of course we have to provide includeMessageType(Class), too.

Please this discussion is very important for MINA API design, so it would be
great if someone respond to this thread soon.

Thanks in advance,
Trustin
--
what we call human nature is actually human habit
--
http://gleamynode.net/

Julien Vermillard

2005-10-13, 5:45 pm

Why not :

public void messageReceived(IoSession session, Object message) {
if (message instanceof MyStartTLSRequest) {
// insert SSLFilter to start handshaking
session.getFilterChain().addFirst(sslFilter);

// Disable encryption
sslFilter.setEnabled(false); // SSLfilter is bypassing

// write StartTLSResponse
session.write(new MyStartTLSResponse(OK));

// Enable encryption
sslFilter.setEnabled(true); // SSLfilter is crypting
}
}

Le jeudi 13 octobre 2005 à 21:20 +0900, Trustin Lee a écrit :
> 2005/10/12, Trustin Lee <trustin-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>:
> Which way do you prefer to the other?
>
> Here's another option:
>
> public void messageReceived(IoSession session, Object message) {
> ...
> if (message instanceof MyStartTLSRequest) {
> // insert SSLFilter to start handshaking
> session.getFilterChain().addFirst(sslFilter);
>
> // Disable encryption for the specified message type.
> sslFilter.excludeMessageType(MyStartTLSResponse.class);
>
> // write StartTLSResponse
> sessios.write(new MyStartTLSResponse(OK));
> }
> }
>
> WDYT? Of course we have to provide includeMessageType(Class), too.
>
> Please this discussion is very important for MINA API design, so it
> would be great if someone respond to this thread soon.
>
> Thanks in advance,
> Trustin
>
> --
> what we call human nature is actually human habit
> --
> http://gleamynode.net/

Trustin Lee

2005-10-13, 5:45 pm

2005/10/13, Julien Vermillard <jvermillard-hxtJVsqPMRdGWvitb5QawA@public.gmane.org>:
>
> Why not :
>
> public void messageReceived(IoSession session, Object message) {
> if (message instanceof MyStartTLSRequest) {
> // insert SSLFilter to start handshaking
> session.getFilterChain().addFirst(sslFilter);
>
> // Disable encryption
> sslFilter.setEnabled(false); // SSLfilter is bypassing
>
> // write StartTLSResponse
> session.write(new MyStartTLSResponse(OK));
>
> // Enable encryption
> sslFilter.setEnabled(true); // SSLfilter is crypting
> }
> }

What if a client initiates TLS handshake process before we call
sslFilter.setEnable(true). I know this won't happen easily, but it has
possibility of race condition yet.

I talked with Emmanuel and here's another option:

public void messageReceived(IoSession session, Object message) {
if (message instanceof MyStartTLSRequest) {
// insert SSLFilter to start handshaking
session.getFilterChain().addFirst(sslFilter);

// Disable encryption temporarilly. This attribute will be cleared after
Session.write()
session.setAttribute( SSLFilter.DISABLE_ENCRYPTION, Boolean.TRUE );

// write StartTLSResponse
session.write(new MyStartTLSResponse(OK));
}
}

Trustin
--
what we call human nature is actually human habit
--
http://gleamynode.net/

Julien Vermillard

2005-10-13, 5:45 pm

Le jeudi 13 octobre 2005 à 22:14 +0900, Trustin Lee a écrit :
>
> What if a client initiates TLS handshake process before we call
> sslFilter.setEnable(true). I know this won't happen easily, but it
> has possibility of race condition yet.
>
> I talked with Emmanuel and here's another option:
>
> public void messageReceived(IoSession session, Object message) {
> if (message instanceof MyStartTLSRequest) {
> // insert SSLFilter to start handshaking
> session.getFilterChain().addFirst(sslFilter);
>
> // Disable encryption temporarilly. This attribute will be
> cleared after Session.write()
> session.setAttribute( SSLFilter.DISABLE_ENCRYPTION ,
> Boolean.TRUE );
>
> // write StartTLSResponse
> session.write(new MyStartTLSResponse(OK));
> }
> }
>
> Trustin
I like the last solution, it keep the change localized to SSLFilter

Rodrigo Kumpera

2005-10-13, 5:45 pm

I think this kind of problem will be present in many more situations
than just SSL. The situation, if I understand correctly, it's a state
transition in the way the stream must be processed.

The scenario is the following:

1-client send start-tls
2-server receive the packet
3-server install a ssl filter and send tls-ok back to client

The problems are that 'tls-ok' must be sent unencrypted and if the
client is pipelining* requests the tls handshare

*I stole the term pipelining from http, meaning the client sends a
request even if the server response for the request before it has not
yet been received.

I think mina ether need a filter passthrou schema (eg, 'tls-ok'
ignores the ssl-filter) or a way to have proper ordering of events so
the ssl-filter will onyl apply to packets after 'tls-ok' and
'start-tls'.

About packet ordering, what about a way to say that a filter have a
boundary of when to start working:

public void messageReceived(IoSession session, Object message) {
if (message instanceof MyStartTLSRequest) {
Object reply =3D new MyStartTLSResponse(OK);
// insert SSLFilter to start handshaking, it will work on all
packet after message and reply.
session.getFilterChain().addFirst(sslFilter, message, reply);

// Disable encryption temporarilly. This attribute will be
cleared after Session.write()
session.setAttribute( SSLFilter.DISABLE_ENCRYPTION , Boolean.TRUE )=
;

// write StartTLSResponse
session.write(reply);
}
}

On 10/13/05, Trustin Lee <trustin-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> 2005/10/13, Julien Vermillard <jvermillard-hxtJVsqPMRdGWvitb5QawA@public.gmane.org>:
>
> What if a client initiates TLS handshake process before we call
> sslFilter.setEnable(true). I know this won't happen easily, but it has
> possibility of race condition yet.
>
> I talked with Emmanuel and here's another option:
>
> public void messageReceived(IoSession session, Object message) {
> if (message instanceof MyStartTLSRequest) {
> // insert SSLFilter to start handshaking
> session.getFilterChain().addFirst(sslFilter);
>
> // Disable encryption temporarilly. This attribute will be clear=
ed
> after Session.write()
> session.setAttribute( SSLFilter.DISABLE_ENCRYPTION , Boolean.TRUE=
);
>
> // write StartTLSResponse
> session.write(new MyStartTLSResponse(OK));
> }
> }
>
> Trustin
> --
> what we call human nature is actually human habit
> --
> http://gleamynode.net/

Trustin Lee

2005-10-13, 5:45 pm

2005/10/13, Rodrigo Kumpera <kumpera-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>:
>
> I think this kind of problem will be present in many more situations
> than just SSL. The situation, if I understand correctly, it's a state
> transition in the way the stream must be processed.

I agree, but there are no known filters which are related with this issue,
so it might be too early to think about it. It would be great if you can
give use some example use case you can imagine. They will be mostly content
transformation filters which requires StartTLS-like negotiation.

I think mina ether need a filter passthrou schema (eg, 'tls-ok'
> ignores the ssl-filter) or a way to have proper ordering of events so
> the ssl-filter will onyl apply to packets after 'tls-ok' and
> 'start-tls'.

Could you explain a bit more about passthrough schema?

About packet ordering, what about a way to say that a filter have a
> boundary of when to start working:
>
> public void messageReceived(IoSession session, Object message) {
> if (message instanceof MyStartTLSRequest) {
> Object reply = new MyStartTLSResponse(OK);
> // insert SSLFilter to start handshaking, it will work on all
> packet after message and reply.
> session.getFilterChain().addFirst(sslFilter, message, reply);
>
> // Disable encryption temporarilly. This attribute will be
> cleared after Session.write()
> session.setAttribute( SSLFilter.DISABLE_ENCRYPTION , Boolean.TRUE );
>
> // write StartTLSResponse
> session.write(reply);
> }
> }

This code is a little bit strange. Why should I pass 'message' to
addFirst()? And we still have session.setAttribute(...). Could you explain
about this in detail?

Thanks,
Trustin
--
what we call human nature is actually human habit
--
http://gleamynode.net/

Rodrigo Kumpera

2005-10-13, 5:45 pm

On 10/13/05, Trustin Lee <trustin-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> 2005/10/13, Rodrigo Kumpera <kumpera-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>:
>
> I agree, but there are no known filters which are related with this issue=
,
> so it might be too early to think about it. It would be great if you can
> give use some example use case you can imagine. They will be mostly cont=
ent
> transformation filters which requires StartTLS-like negotiation.
>
>
> Could you explain a bit more about passthrough schema?
>
UE[vbcol=seagreen]
> );
>
> This code is a little bit strange. Why should I pass 'message' to
> addFirst()? And we still have session.setAttribute(...). Could you
> explain about this in detail?

I c&p'd your example without carefully reading it, it should not have
the setAttribute call. The idea passing message to passfist is to
inform mina that all data after it should have the filter applied, or
this will happen anyway (I've never needed a start tls thing).

The passthrough schema, can either work as you suggested, using a
session attributure, or maybe with a marker interface
(MyStartTLSResponse implements NoSslFiltering). I just think it
shouldn=B4t have any impact in SSLFilter performance.

Other filter that would have such behavior is a compaction filter:
client requests the connection to be gzip'ed -I think cvs with
compaction works this way.

> Thanks,
> Trustin
> --
> what we call human nature is actually human habit
> --
> http://gleamynode.net/

Vinod Panicker

2005-10-31, 7:45 am

Sorry I wasn't around to add my 2 bits to this discussion, but I'm
kinda OK with this solution. Only question is - what will happen if
data is received in the middle of this process? Will it be given to
the SSLFilter or not?

I'm also of the opinion that a "filter bypass" solution should be
abstracted as a one-time solution to such race conditions.

What I'm doing right now as a workaround to this problem is
implementing my own filter which simply keeps buffering all the
incoming data till the sslfilter is implemented. After that, I remove
the filter and retrieve all the data.

Regards,
Vinod.

On 10/13/05, Trustin Lee <trustin-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> 2005/10/13, Julien Vermillard <jvermillard-hxtJVsqPMRdGWvitb5QawA@public.gmane.org>:
>
> What if a client initiates TLS handshake process before we call
> sslFilter.setEnable(true). I know this won't happen easily, but it has
> possibility of race condition yet.
>
> I talked with Emmanuel and here's another option:
>
> public void messageReceived(IoSession session, Object message) {
> if (message instanceof MyStartTLSRequest) {
> // insert SSLFilter to start handshaking
> session.getFilterChain().addFirst(sslFilter);
>
> // Disable encryption temporarilly. This attribute will be clear=
ed
> after Session.write()
> session.setAttribute( SSLFilter.DISABLE_ENCRYPTION , Boolean.TRUE=
);
>
> // write StartTLSResponse
> session.write(new MyStartTLSResponse(OK));
> }
> }
>
> Trustin
> --
> what we call human nature is actually human habit
> --
> http://gleamynode.net/