Hi Nikos,

Please find attached to this Email a clean patch for adding custom extension 
and custom supplemental data from public GNUTLS API.
I take care of all your comments in order to give you a patch as you expected 
it.

0001: Fix data type > 255 in supplemental data
0002 0003: Add a way to set customs extensions from public API
0004: Add a way to add custom supplemental data from public API.

For each ones, I added a test application mini-extension.c and 
mini-supplementaldata.c for your unit tests

I'm hoping this patch includes all what you expected.

Best regards,
Thierry.

-----Original Message-----
From: Thierry Quemerais 
Sent: Monday, March 16, 2015 9:40 AM
To: 'Nikos Mavrogiannopoulos'
Cc: [email protected]
Subject: RE: [gnutls-help] GNU TLS and extensions/supplemental data

Hi Nikos,

I got your point.

I will change that and I will do what you suggested:  Create an API 
gnutls_ext_register that receive in parameters all what is needed.
I will also include a test file for your test suite and I will try to separate 
this patch as 3 different patches.

Regards,
Thierry

-----Original Message-----
From: [email protected] [mailto:[email protected]] On 
Behalf Of Nikos Mavrogiannopoulos
Sent: Friday, March 13, 2015 4:45 PM
To: Thierry Quemerais
Cc: [email protected]
Subject: Re: [gnutls-help] GNU TLS and extensions/supplemental data

On Fri, Mar 13, 2015 at 2:53 PM, Thierry Quemerais <[email protected]> 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

Attachment: 0001-Handle-supplemental-data-type-255.patch
Description: 0001-Handle-supplemental-data-type-255.patch

Attachment: 0002-Added-a-way-to-add-custom-extensions-from-public-API.patch
Description: 0002-Added-a-way-to-add-custom-extensions-from-public-API.patch

Attachment: 0003-Fixed-extension-test.patch
Description: 0003-Fixed-extension-test.patch

Attachment: 0004-Added-a-way-to-add-custom-supplemental-data-from-pub.patch
Description: 0004-Added-a-way-to-add-custom-supplemental-data-from-pub.patch

_______________________________________________
Gnutls-help mailing list
[email protected]
http://lists.gnupg.org/mailman/listinfo/gnutls-help

Reply via email to