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 <
Post by Ingela Anderton AndinHi!
See inlined comments, to both Andreas and Haiyangs mails.
Post by Andreas SchultzHi 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.
Post by Andreas Schultz1.) 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
Post by Andreas SchultzPost by Haiyang YinOTP
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
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.
https://github.com/haiyang-yin/otp
git clone https://github.com/haiyang-yin/otp.git
git checkout dtls_patch
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