[gnutls-devel] GnuTLS | support non-NULL-terminated PSKs (!917)

[Development of GNU's TLS library]

Merge request https://gitlab.com/gnutls/gnutls/merge_requests/917 was reviewed by Nikos Mavrogiannopoulos

--
  
Nikos Mavrogiannopoulos started a new discussion on lib/auth/psk.h: https://gitlab.com/gnutls/gnutls/merge_requests/917#note_226286315

> +void _gnutls_copy_psk_username(psk_auth_info_t info, const gnutls_datum_t *username)
> +{
> +	assert(sizeof(info->username) >= username->size);

I think there is an off by one here. If the equality holds, there is no space for the null.

--
  
Nikos Mavrogiannopoulos started a new discussion on lib/auth/psk_passwd.c: https://gitlab.com/gnutls/gnutls/merge_requests/917#note_226286316

> +}
> +
> +static int username_matches(const gnutls_datum_t *username,

This function doesn't have a consistent return value. At some point it returns a negative error code, later it returns the output of memcmp. It may be simpler to make it unsigned or bool and have a binary output.

--
  
Nikos Mavrogiannopoulos started a new discussion on lib/handshake-checks.c: https://gitlab.com/gnutls/gnutls/merge_requests/917#note_226286317

>  	if (cred_type == GNUTLS_CRD_PSK || cred_type == GNUTLS_CRD_SRP) {
>  		const char *username = NULL;
> +		uint16_t username_length;

Why use uint16_t here instead of int or unsigned?

--
  
Nikos Mavrogiannopoulos started a new discussion on lib/psk.c: https://gitlab.com/gnutls/gnutls/merge_requests/917#note_226286318

> +	int ret;
> +	char *user_p;
> +	gnutls_psk_client_credentials_t cred =

That doesn't seem to be used.

--
  
Nikos Mavrogiannopoulos started a new discussion on lib/auth/psk.h: https://gitlab.com/gnutls/gnutls/merge_requests/917#note_226286320

>  typedef struct psk_auth_info_st {
>  	char username[MAX_USERNAME_SIZE + 1];
> +	uint16_t len;

Just len in this structure can be confusing (len of username, or hint?). What about username_len?

--
  
Nikos Mavrogiannopoulos started a new discussion on lib/psk.c: https://gitlab.com/gnutls/gnutls/merge_requests/917#note_226286321

> + * @out: an empty gnutls datum
> + *
> + * Return a pointer to the username of the peer in the supplied datum. Does not

I'd explicitly spell out `@out` here.

--
  
Nikos Mavrogiannopoulos started a new discussion on lib/psk.c: https://gitlab.com/gnutls/gnutls/merge_requests/917#note_226286322

> + * gnutls_psk_server_get_username2:
> + * @session: is a gnutls session
> + * @out: an empty gnutls datum

What about instead: `a datum that will be filled in by this function`.


-- 
Reply to this email directly or view it on GitLab: https://gitlab.com/gnutls/gnutls/merge_requests/917
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/20191004/fb772b4e/attachment-0001.html>