[gnutls-devel] GnuTLS | WIP: Record layer separation for QUIC TLS API (!1086)

[Development of GNU's TLS library]

Merge request https://gitlab.com/gnutls/gnutls/merge_requests/1086 was reviewed by Daiki Ueno

--
  
Daiki Ueno started a new discussion on lib/includes/gnutls/gnutls.h.in: https://gitlab.com/gnutls/gnutls/merge_requests/1086#note_231306381

> + * This configures the read and write secrets for the given encryption level.
> + */
> +void gnutls_set_secret_hook_function(gnutls_session_t session, gnutls_secret_hook_func set_encrytion);

perhaps this was my suggestion, but I would rename this function to `gnutls_session_set_secret_hook_function`, as suggested in CONTRIBUTING.md:
https://gitlab.com/gnutls/gnutls/blob/master/CONTRIBUTING.md#function-names

--
  
Daiki Ueno started a new discussion on lib/ext/pre_shared_key.c: https://gitlab.com/gnutls/gnutls/merge_requests/1086#note_231306388

>  {
> -	int ret;
> +	int ret, retval;

can't you simply reuse `ret` everywhere?

--
  
Daiki Ueno started a new discussion on lib/handshake-tls13.c: https://gitlab.com/gnutls/gnutls/merge_requests/1086#note_231306394

> +	if (retval < 0)
> +		return gnutls_assert_val(retval);
> +

I think this part is not necessary anymore, as we don't hook in this function.

--
  
Daiki Ueno started a new discussion on lib/quic-api.c: https://gitlab.com/gnutls/gnutls/merge_requests/1086#note_231306397

> +#include <gnutls/gnutls.h>
> +
> +void gnutls_set_secret_hook_function(gnutls_session_t session,  gnutls_secret_hook_func func) {

since this is an API function, add gdoc style comment explaining the function here:
https://gitlab.com/gnutls/gnutls/blob/master/CONTRIBUTING.md#api-documentation

--
  
Daiki Ueno started a new discussion on lib/includes/gnutls/gnutls.h.in: https://gitlab.com/gnutls/gnutls/merge_requests/1086#note_231306399

> + */
> +int gnutls_is_quic(gnutls_session_t);
> +

these functions are not implemented so far and shall be added in separate MRs. I suggest removing these declarations for now.

--
  
Daiki Ueno started a new discussion on lib/constate.c: https://gitlab.com/gnutls/gnutls/merge_requests/1086#note_231306401

>  
>  	if (stage == STAGE_UPD_OURS || stage == STAGE_UPD_PEERS)
>  		return _tls13_update_keys(session, stage,

If you want to handle key update, I think you need to call secret_hook in `_tls13_update_keys`.


-- 
Reply to this email directly or view it on GitLab: https://gitlab.com/gnutls/gnutls/merge_requests/1086
You're receiving this email because of your account on gitlab.com.


-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.gnupg.org/pipermail/gnutls-devel/attachments/20191016/15cbbf06/attachment-0001.html>