[gnutls-help] GNU TLS and extensions/supplemental data

[Nikos Mavrogiannopoulos]

On Fri, Mar 13, 2015 at 2:53 PM, Thierry Quemerais <tquemerais at awox.com> wrote:
> Hi Nikos,
>
> Thank you for your reply.
>
> Please find attached to this Email, the patch I made to achieve my goal.
> For your information, I also found an issue in GNUTLS which does not handle supplemental data type > 255.
> This fix is included in this patch:
> -                       buf->data[sizepos] = 0;
> -                       buf->data[sizepos + 1] = p->type;
> +                       buf->data[sizepos] = (p->type >> 8) & 0xFF;
> +                       buf->data[sizepos + 1] = (p->type) & 0xFF;

Applied separately.

> To achieve my goal :
>         I changed the way supplemental callback are stored. (gnutls_supplemental.c)
>         I moved extension en supplemental structure to gnutls.h header.  (gnutls_supplemental_entry, extension_entry_st)
>         I created an opaque struct for strings (typedef struct gnutls_buffer gnutls_buffer_st;) and I added a function to append data in this buffer (Used by extension/supplemental callbacks).

Thank you. I like the patch, it is pretty simple and straightforward.
My main objection is the export of gnutls_ext_register structure,
which will require ABI break in case we add anything there. Why not
keep _gnutls_ext_register() as is, and then export a
gnutls_ext_register() which receives everything needed in the current
structure as parameters? The same for gnutls_supplemental_entry... The
risk of this structure changing is lower, but better safe than sorry.

Some more, but minor comments.
* Would it be possible to submit the a patch from git, and use the
--signoff flag (as well as send the DCO - see
http://www.gnutls.org/devel.html)
* It would be convenient for debugging to split the patch into two,
one for the extensions and on for the supplemental
* In extension_entry_st you use "short type". I think simply using an
int or unsigned int would be better.
* Do you have some small test programs to use in our test suite for
this API? That would ensure that the API you add now remains unbroken
over releases.

regards,
Nikos