[WIP] DTLS 1.0 preliminary patches

[Nikos Mavrogiannopoulos]

Jonathan Bastien-Filiatrault wrote:

Hello Jonathan,

 Due to lack of time I did some quick glimpse, and attach some quick
comments.

>>> Being interested in DTLS and GnuTLS I have decided to try to implement
>>> DTLS in the GnuTLS library.

That's really good news!

>>> Unfortunately the lower end of the record layer and buffer/transport
>>> layer seems rather messy to my untrained eye. I am having trouble
>>> imagining implementing UDP buffering easely. I would need to buffer the
>>> whole packet then iterate over the records contained within the packet.
>>>
>>> The main problem seems to be layering violations between the handshake,
>>> record and buffer layers. Would it be better if _gnutls_{recv,send}_int
>>> dealt with whole records (and possibly return prematurely if more data
>>> or buffer space is required) ? _gnutls_{recv,send}_int could also deal
>>> with the SSLv2.0 record encapsulation. The handhake layer would
>>> therefore deal with those two functions for sending/receiving from the
>>> lower layer. The handshake layer buffering would also be moved to
>>> gnutls_handshake.c.
>>>
>>> Am I making any sense ?
>>
>> I agree that code is hairy, and I'm hoping Nikos can give you some
>> advice.  I don't feel strongly about changing this code, but it will
>> need significant testing before we can feel comfortable with the
>> changes.  Maybe as a bonus side-effect, the code will become more
>> readable... ;)
> 
> Actually, I started to read the code yesterday and it seems to make more
> sense than I originally thought. I would be more inclined to
> short-circuit a lot of the buffer layer to a UDP buffering layer.
> Datagram transport requires its own buffers for retransmission and
> reassembly. I think it would be possible to do this without disturbing
> much of the record and handshake layer and without code duplication (I
> would still use functions such as _gnutls_read).

It's good that you understood the code. This part is one of the oldest
parts and quite unreadable. If you have any questions on that let me
know. I really don't know what will be faster to do for DTLS...
rewriting this part of code or extending it. Anyway if you have any
specific questions on functionality or purpose let me know...

>> Re 0002-Add-DTLS1.0-protocol-entry.patch: This breaks the API.  Can you
>> re-order the DTLS addition so it is after GNUTLS_TLS1_2 and add a '=
>> 100' after it so there is room for TLS 1.3 etc?  Also, please drop the
>> GNUTLS_DTLS1 mapping, I think it helps to be specific about version
>> numbers at all places.  I think this patch could be added quickly
>> without problem.
> Alright, but DTLS1.0 needs to be sandwiched between TLS1.1 and TLS1.2,
> mostly for ver < GNUTLS_TLS1_2 checks. Since TLS1.2 is still
> experimental, could this breakage be tolerated ? I am wide open for a
> suggestions in this case...

In general I'd agree with simon that DTLS should be distinct from
TLSx.y. For the specific tests maybe we should move those into
designated functions such as if (_check_for_feature_xyz(tlsversion)) {
... }. And a more complex matching algorithm will be present there.

>> Re 0004-Add-gnutls_session_datagram-function.patch: this just toggles
>> one way.  DTLS is really a completely new protocol, not just a different
>> transport method for TLS.  So maybe there should really be a new
>> function that replaces gnutls_init?  How about gnutls_init_dtls?  It
>> would return a gnutls_session_t for DTLS.

If there no issues in initialization the obvious place for me to be done
would be gnutls_priority_set() and gnutls_protocol_set_priority(). There
the actual version of the protocol that will be used is given and if
DTLS is there the function should act accordingly. That way the same API
can be used for both.

> I was thinking more of a set_transport in the long run with a DGRAM or
> STREAM argument and with a transport protocol selector for UDP, SCTP,
> DCCP and UDP-Lite. I guess gnutls_init_dtls could take those arguments
> instead.

You mean for setting different push/pull functions? Those could be in
predefined macros that are explicitly called.

regards,
Nikos