DTLS patch

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
15 messages Options
Reply | Threaded
Open this post in threaded view
|

DTLS patch

Haiyang Yin
Hello,

We have a server application needs DTLS protocol support. But the current OTP release still has an incomplete DTLS implementation. So we create this patch to include DTLS implementation based on current well-designed ssl architecture (which we don't consider this as a new feature, just a patch). What we have added are:

1. DTLS transport layer on top of gen_udp
2. DTLS flight retransmission and timeout mechanism
3. DTLS record fragmentation/defragmentation handling

It looks good that the patch can work with OpenSSL 1.0.2a release. If no one is working on dtls now, we would like to have this patch to be reviewed.

Following is the repository contains the patch:


To fetch the patch, refer to following git commands:

git checkout dtls_patch

Here is code review location:


There are two demo programs to show how dtls client/server works in the attachments.

Feel free to let me know if further information is needed.

Thank you,

/Haiyang


_______________________________________________
erlang-patches mailing list
[hidden email]
http://erlang.org/mailman/listinfo/erlang-patches

dumpd_dtls_c.erl (4K) Download Attachment
dumpd_dtls.erl (4K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: DTLS patch

Andreas Schultz
Hi Haiyang,

I'm sure Ingela will have plenty to say, starting with "Unit Tests".

A few comments from me:

1.) Have you tested your changes with packet loss and reordering, especially
reordering around the the Change Cipher Spec message?

Keeping the correct cipher context for re-sending old flights and decoding
incoming re-sends across CCS messages is not trivial and from reading the
code I can't convince myself that it would work.

2.) I'm not happy with the way new clients are handled. You seem to
insert them into an ETS tables without waiting for the answer to an
HelloVerifyRequest. This opens a way for resource exhaustion and denial
of service attacks. The DTLS RFC is quite explicit that the verify
cookie should be generated stateless and that the server should expend
any memory resource until the Hello was verified successfully.

3.) TLS is almost exclusively used in a pure form over TCP, the situation
is different with DTLS where some interesting deviations exist. The TCP
like socket handling is not help full with those.

For example CAPWAP (RFC 5415) multiplexes DTLS and non DTLS data over the
same UDP socket. Some mechanism to hook between the raw socket and the
DTLS logic is required for that.

4.) You have certainly added lots of code, but you have still reused some
ideas and code from my earlier versions. A small attribution wouldn't hurt.


Andreas

----- On 30 May, 2015, at 19:19, Haiyang Yin [hidden email] wrote:

> Hello,
>
> We have a server application needs DTLS protocol support. But the current OTP
> release still has an incomplete DTLS implementation. So we create this patch to
> include DTLS implementation based on current well-designed ssl architecture
> (which we don't consider this as a new feature, just a patch). What we have
> added are:
>
> 1. DTLS transport layer on top of gen_udp
> 2. DTLS flight retransmission and timeout mechanism
> 3. DTLS record fragmentation/defragmentation handling
>
> It looks good that the patch can work with OpenSSL 1.0.2a release. If no one is
> working on dtls now, we would like to have this patch to be reviewed.
>
> Following is the repository contains the patch:
>
> https://github.com/haiyang-yin/otp
>
> To fetch the patch, refer to following git commands:
>
> git clone https://github.com/haiyang-yin/otp.git
> git checkout dtls_patch
>
> Here is code review location:
>
> https://github.com/haiyang-yin/otp/compare/maint...haiyang-yin:dtls_patch
>
> There are two demo programs to show how dtls client/server works in the
> attachments.
>
> Feel free to let me know if further information is needed.
>
> Thank you,
>
> /Haiyang
>
>
> _______________________________________________
> erlang-patches mailing list
> [hidden email]
> http://erlang.org/mailman/listinfo/erlang-patches

--
--
Dipl. Inform.
Andreas Schultz
_______________________________________________
erlang-patches mailing list
[hidden email]
http://erlang.org/mailman/listinfo/erlang-patches
Reply | Threaded
Open this post in threaded view
|

Re: DTLS patch

Ingela Anderton Andin-4
Hi!

See inlined comments, to both Andreas and Haiyangs mails.

On 05/31/2015 05:22 PM, Andreas Schultz wrote:
> Hi Haiyang,
>
> I'm sure Ingela will have plenty to say, starting with "Unit Tests".

Yes you are absolutely correct we want tests, a good place to start
could be a dtls_to_openssl_SUITE corresponding to ssl_to_openssl_SUITE.

> A few comments from me:
>
> 1.) Have you tested your changes with packet loss and reordering, especially
> reordering around the the Change Cipher Spec message?
>
> Keeping the correct cipher context for re-sending old flights and decoding
> incoming re-sends across CCS messages is not trivial and from reading the
> code I can't convince myself that it would work.
>
> 2.) I'm not happy with the way new clients are handled. You seem to
> insert them into an ETS tables without waiting for the answer to an
> HelloVerifyRequest. This opens a way for resource exhaustion and denial
> of service attacks. The DTLS RFC is quite explicit that the verify
> cookie should be generated stateless and that the server should expend
> any memory resource until the Hello was verified successfully.
>
> 3.) TLS is almost exclusively used in a pure form over TCP, the situation
> is different with DTLS where some interesting deviations exist. The TCP
> like socket handling is not help full with those.
>
> For example CAPWAP (RFC 5415) multiplexes DTLS and non DTLS data over the
> same UDP socket. Some mechanism to hook between the raw socket and the
> DTLS logic is required for that.
>
> 4.) You have certainly added lots of code, but you have still reused some
> ideas and code from my earlier versions. A small attribution wouldn't hurt.

I am sure these are all valid comments to consider. Andreas has made
many valuable contributions to the ssl application in the past and has a
good knowledge in the subject.

>> We have a server application needs DTLS protocol support. But the current OTP
>> release still has an incomplete DTLS implementation. So we create this patch to
>> include DTLS implementation based on current well-designed ssl architecture
>> (which we don't consider this as a new feature, just a patch). What we have
>> added are:
>>
>> 1. DTLS transport layer on top of gen_udp
>> 2. DTLS flight retransmission and timeout mechanism
>> 3. DTLS record fragmentation/defragmentation handling
>>
>> It looks good that the patch can work with OpenSSL 1.0.2a release. If no one is
>> working on dtls now, we would like to have this patch to be reviewed.
>>
>> Following is the repository contains the patch:
>>
>> https://github.com/haiyang-yin/otp
>>
>> To fetch the patch, refer to following git commands:
>>
>> git clone https://github.com/haiyang-yin/otp.git
>> git checkout dtls_patch
 >>
>> Here is code review location:
>>
>> https://github.com/haiyang-yin/otp/compare/maint...haiyang-yin:dtls_patch
>>
>> There are two demo programs to show how dtls client/server works in the
>> attachments.
>>
>> Feel free to let me know if further information is needed.

We would prefer if you could make it into a pull-request, or maybe
several pull-requests. I think the first commit moving functions from
tls_connection into ssl_connection could be one pull-request. That can
be considered part of the refactoring work that I was doing in order to
make our DTLS-implementation (that I alas have not  had time to finish
due to other things being prioritized). So we could include that as a
first step.

The other parts I and the OTP team will need some more time to think
about your implementation suggestion. But what ever the conclusion will
be, it is a way to give this issue some more focus, and hopefully move
it forward towards a functioning DLTS implementation.

Regards Ingela Erlang/OTP Team Ericssson AB


_______________________________________________
erlang-patches mailing list
[hidden email]
http://erlang.org/mailman/listinfo/erlang-patches
Reply | Threaded
Open this post in threaded view
|

Re: DTLS patch

Haiyang Yin
Hi Andreas/Ingela,

Thanks you very much for your comments and time. I very appreciate it. I have some inline comments marked with [YHY]:

1.) Have you tested your changes with packet loss and reordering, especially
reordering around the the Change Cipher Spec message?

Keeping the correct cipher context for re-sending old flights and decoding
incoming re-sends across CCS messages is not trivial and from reading the
code I can't convince myself that it would work.

[YHY] No, the change we made was based on existing dtls code and minor changes, e.g. fragmented handshake messages, retransmission of old flights and etc. The existing code dealt with cases specified by RFC on the basis of record sequence number and handshake message sequence number. I agree reordering is much more complicated than current code as more information needs to be cached in order to reorder/reconstruct handshake messages. As we move on, certainly that part in dtls_handshake.erl would be enhanced. Very good point to bring this up.

2.) I'm not happy with the way new clients are handled. You seem to
insert them into an ETS tables without waiting for the answer to an
HelloVerifyRequest. This opens a way for resource exhaustion and denial
of service attacks. The DTLS RFC is quite explicit that the verify
cookie should be generated stateless and that the server should expend
any memory resource until the Hello was verified successfully.

[YHY] Our purpose of designing dtls_transport module is to have a thin protocol-agnostic layer to just pass the datagram packets. Cookie mechanism makes it hard for spoofing DoS attack but no defense from valid IP addresses. You are right the resource leak in ETS table in this case, to overcome this, we can start a timer to wait for CLIENT HELLO and close the socket (remove it from ETS table) for both DoS cases. This provides another way of invalidating the Cookies if the attacker tries to collect a number of cookies from different addresses (See discussion in DTLS RFC). Any comments ?

3.) TLS is almost exclusively used in a pure form over TCP, the situation
is different with DTLS where some interesting deviations exist. The TCP
like socket handling is not help full with those.

For example CAPWAP (RFC 5415) multiplexes DTLS and non DTLS data over the
same UDP socket. Some mechanism to hook between the raw socket and the
DTLS logic is required for that.

[YHY] Sorry I am not familiar with CAPWAP, is it the service discovery (clear text) and later DTLS session established to convey encrypted data ? To me, this might be resolved in dtls_connection module. When raw datagram packets passed to handle_info callback, it can check data based on session state on which the data is encrypted or not. It is flexible but sacrifice the performance in raw socket level. This is very important because it will impact how application developers use the interface. I hope further discussion required for this topic. 

4.) You have certainly added lots of code, but you have still reused some
ideas and code from my earlier versions. A small attribution wouldn't hurt.

[YHY] The existing DTLS code is well-written. We guess it just you didn't have enough time to polish it further. We actually didn't change too much and it works well in sunny cases. Having said that our change is a patch but not a new feature. 

Regarding the test cases, we will follow ssl_to_openssl_SUITE to add dtls_to_openssl_SUITE later, it is always a good idea to have test cases.

This is a start point toward complete DTLS implementation as Ingela mentioned. We are sure more comments and ideas would come out when we dig deep further.

We will continue follow up on this,

Cheers,

/Haiyang

On Mon, Jun 1, 2015 at 9:32 AM, Ingela Anderton Andin <[hidden email]> wrote:
Hi!

See inlined comments, to both Andreas and Haiyangs mails.

On 05/31/2015 05:22 PM, Andreas Schultz wrote:
Hi Haiyang,

I'm sure Ingela will have plenty to say, starting with "Unit Tests".

Yes you are absolutely correct we want tests, a good place to start
could be a dtls_to_openssl_SUITE corresponding to ssl_to_openssl_SUITE.

A few comments from me:

1.) Have you tested your changes with packet loss and reordering, especially
reordering around the the Change Cipher Spec message?

Keeping the correct cipher context for re-sending old flights and decoding
incoming re-sends across CCS messages is not trivial and from reading the
code I can't convince myself that it would work.

2.) I'm not happy with the way new clients are handled. You seem to
insert them into an ETS tables without waiting for the answer to an
HelloVerifyRequest. This opens a way for resource exhaustion and denial
of service attacks. The DTLS RFC is quite explicit that the verify
cookie should be generated stateless and that the server should expend
any memory resource until the Hello was verified successfully.

3.) TLS is almost exclusively used in a pure form over TCP, the situation
is different with DTLS where some interesting deviations exist. The TCP
like socket handling is not help full with those.

For example CAPWAP (RFC 5415) multiplexes DTLS and non DTLS data over the
same UDP socket. Some mechanism to hook between the raw socket and the
DTLS logic is required for that.

4.) You have certainly added lots of code, but you have still reused some
ideas and code from my earlier versions. A small attribution wouldn't hurt.

I am sure these are all valid comments to consider. Andreas has made many valuable contributions to the ssl application in the past and has a good knowledge in the subject.

We have a server application needs DTLS protocol support. But the current OTP
release still has an incomplete DTLS implementation. So we create this patch to
include DTLS implementation based on current well-designed ssl architecture
(which we don't consider this as a new feature, just a patch). What we have
added are:

1. DTLS transport layer on top of gen_udp
2. DTLS flight retransmission and timeout mechanism
3. DTLS record fragmentation/defragmentation handling

It looks good that the patch can work with OpenSSL 1.0.2a release. If no one is
working on dtls now, we would like to have this patch to be reviewed.

Following is the repository contains the patch:

https://github.com/haiyang-yin/otp

To fetch the patch, refer to following git commands:

git clone https://github.com/haiyang-yin/otp.git
git checkout dtls_patch
>>
Here is code review location:

https://github.com/haiyang-yin/otp/compare/maint...haiyang-yin:dtls_patch

There are two demo programs to show how dtls client/server works in the
attachments.

Feel free to let me know if further information is needed.

We would prefer if you could make it into a pull-request, or maybe several pull-requests. I think the first commit moving functions from tls_connection into ssl_connection could be one pull-request. That can be considered part of the refactoring work that I was doing in order to make our DTLS-implementation (that I alas have not  had time to finish due to other things being prioritized). So we could include that as a first step.

The other parts I and the OTP team will need some more time to think about your implementation suggestion. But what ever the conclusion will
be, it is a way to give this issue some more focus, and hopefully move it forward towards a functioning DLTS implementation.

Regards Ingela Erlang/OTP Team Ericssson AB




_______________________________________________
erlang-patches mailing list
[hidden email]
http://erlang.org/mailman/listinfo/erlang-patches
Reply | Threaded
Open this post in threaded view
|

Re: DTLS patch

Max Lapshin-2
I want to connect webrtc to erlang. Will this implementation help me to launch udp listener?


_______________________________________________
erlang-patches mailing list
[hidden email]
http://erlang.org/mailman/listinfo/erlang-patches
Reply | Threaded
Open this post in threaded view
|

Re: DTLS patch

Haiyang Yin
Hi Max,

Do you mean to launch udp listener for webrtc, which will use DTLS for data transfer ? or something else ?

/Haiyang
  

On Sat, Jun 6, 2015 at 8:13 AM, Max Lapshin <[hidden email]> wrote:
I want to connect webrtc to erlang. Will this implementation help me to launch udp listener?



_______________________________________________
erlang-patches mailing list
[hidden email]
http://erlang.org/mailman/listinfo/erlang-patches
Reply | Threaded
Open this post in threaded view
|

Re: DTLS patch

Max Lapshin-2
I think, yes.  Or maybe convert udp socket to a DTLS listener.

_______________________________________________
erlang-patches mailing list
[hidden email]
http://erlang.org/mailman/listinfo/erlang-patches
Reply | Threaded
Open this post in threaded view
|

Re: DTLS patch

Haiyang Yin
The patch I provided could do what you want. However, that needs to be discussed by OTP team regarding the design according to Ingela. Since I saw not only you ask this kind of questions, I hope the DTLS support for Erlang could get attention sooner or later :). No matter what change can be made eventually.

On Tue, Jun 9, 2015 at 1:26 AM, Max Lapshin <[hidden email]> wrote:
I think, yes.  Or maybe convert udp socket to a DTLS listener.


_______________________________________________
erlang-patches mailing list
[hidden email]
http://erlang.org/mailman/listinfo/erlang-patches
Reply | Threaded
Open this post in threaded view
|

Re: DTLS patch

Max Lapshin-2
I don't know exactly what to do with DTLS in my case, because I need to use the same udp socket for different kinds of communication.

So I need to change protocol handler on socket.

Right now I'm thinking about reimplementing DTLS by my hands and this idea is scarying  me a lot.


_______________________________________________
erlang-patches mailing list
[hidden email]
http://erlang.org/mailman/listinfo/erlang-patches
Reply | Threaded
Open this post in threaded view
|

Re: DTLS patch

Roland Karlsson-2
Why do you want to reuse the socket?
Why not just close it and open a new?

/Roland



On Sat, 11 Jul 2015 15:48:26 +0300
Max Lapshin <[hidden email]> wrote:

> I don't know exactly what to do with DTLS in my case, because I need to use
> the same udp socket for different kinds of communication.
>
> So I need to change protocol handler on socket.
>
> Right now I'm thinking about reimplementing DTLS by my hands and this idea
> is scarying  me a lot.

_______________________________________________
erlang-patches mailing list
[hidden email]
http://erlang.org/mailman/listinfo/erlang-patches
Reply | Threaded
Open this post in threaded view
|

Re: DTLS patch

Andreas Schultz
In reply to this post by Haiyang Yin

Am 11.07.2015 15:47 schrieb Roland Karlsson <[hidden email]>:
>
> Why do you want to reuse the socket?
> Why not just close it and open a new?

There are protocols where you have to multiplex DTLS and normal data over the same socket

Andreas

>
> /Roland
>
>
>
> On Sat, 11 Jul 2015 15:48:26 +0300
> Max Lapshin <[hidden email]> wrote:
>
> > I don't know exactly what to do with DTLS in my case, because I need to use
> > the same udp socket for different kinds of communication.
> >
> > So I need to change protocol handler on socket.
> >
> > Right now I'm thinking about reimplementing DTLS by my hands and this idea
> > is scarying  me a lot.
>
> _______________________________________________
> erlang-patches mailing list
> [hidden email]
> http://erlang.org/mailman/listinfo/erlang-patches 
_______________________________________________
erlang-patches mailing list
[hidden email]
http://erlang.org/mailman/listinfo/erlang-patches
Reply | Threaded
Open this post in threaded view
|

Re: DTLS patch

Max Lapshin-2
Because client will send data to the same port and it doesn't care that I use library that doesn't allow me something.



_______________________________________________
erlang-patches mailing list
[hidden email]
http://erlang.org/mailman/listinfo/erlang-patches
Reply | Threaded
Open this post in threaded view
|

Re: DTLS patch

Max Lapshin-2
Well, implementing DTLS is a very nice thing =)

I'm trying to look if it is possible to reuse existing SSL implementation for it, but it seems that it is rather hard, because erlang SSL is a very self-contained thing, designed for isolated usage, not like a library on top of existing socket.



_______________________________________________
erlang-patches mailing list
[hidden email]
http://erlang.org/mailman/listinfo/erlang-patches
Reply | Threaded
Open this post in threaded view
|

Re: DTLS patch

Andreas Schultz
Hi,

----- Original Message -----
> From: "Max Lapshin" <[hidden email]>
> To: "Andreas Schultz" <[hidden email]>
> Cc: "Roland Karlsson" <[hidden email]>, "erlang-patches" <[hidden email]>, "Haiyang Yin"
> <[hidden email]>
> Sent: Monday, July 13, 2015 11:18:42 AM
> Subject: Re: [erlang-patches] DTLS patch

> Well, implementing DTLS is a very nice thing =)

> I'm trying to look if it is possible to reuse existing SSL implementation for
> it, but it seems that it is rather hard, because erlang SSL is a very
> self-contained thing, designed for isolated usage, not like a library on top of
> existing socket.

Well, the TLS code has a concept of a transport call back module for abstracting
the underlying socket. There is no documentation for that and it's (IMO) not very
consistent either.

For my version, I used that and implemented a UDP socket wrapper call back module.
For your case, you could use that socket wrapper as a starting point and modify it.
That's what I do for CAPWAP DTLS support.

I believe Haiyang Yin patch has a very similar mechanism. The cb module is called
dtls_transport and utilizes dtls_socket_manager and dtls_socket_server. Just
extract them, rename and alter to your needs, then pass your version as cb_info
argument into the ssl socket setup.

Andreas
_______________________________________________
erlang-patches mailing list
[hidden email]
http://erlang.org/mailman/listinfo/erlang-patches
Reply | Threaded
Open this post in threaded view
|

Re: DTLS patch

Max Lapshin-2
Ok, thanks!


_______________________________________________
erlang-patches mailing list
[hidden email]
http://erlang.org/mailman/listinfo/erlang-patches