[gnutls-devel] GnuTLS | supported_groups: rewrite group negotiation without nested loop (!2120)

[(Deprecated) Read-only notification of GnuTLS library development activities]

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

--
  
Daiki Ueno commented on a discussion on lib/ext/supported_groups.c: https://gitlab.com/gnutls/gnutls/-/merge_requests/2120#note_3639406826

 > +		     c <= GROUP_CLASS_MAX; c++) {
 > +			if (cpos_by_class[c] < cpos) {
 > +				cpos = cpos_by_class[c];

Not really, as the `if` conditions are also different depending on server_precedence.

--
  
Daiki Ueno commented on a discussion on lib/ext/supported_groups.c: https://gitlab.com/gnutls/gnutls/-/merge_requests/2120#note_3639406883

 > +	if (group->pk == GNUTLS_PK_DH) {
 > +		return GROUP_CLASS_DH;
 > +	} else if (IS_EC(group->pk)) {

Not my style, sorry. `else` makes it clear that the above condition doesn't meet.

--
  
Daiki Ueno commented on a discussion on lib/ext/supported_groups.c: https://gitlab.com/gnutls/gnutls/-/merge_requests/2120#note_3639406889

 > -		/* we figure what is the minimum DH allowed for this session, if any */
 > -		min_dh = get_min_dh(session);
 > +	if (len != data_size)

Overlong (or padded) extension should be rejected, while the previous was too tolerate and ignored it. RFC 8446 defines Extension so that it shouldn't happen and tlsfuzzer has tests for that in some extensions.

--
  
Daiki Ueno commented on a discussion on lib/ext/supported_groups.c: https://gitlab.com/gnutls/gnutls/-/merge_requests/2120#note_3639406896

 > +		 * TLS 1.3 as an encrypted extension. */
 > +		return 0;
 > +	} else {

I think this is written so that client/server code paths are treated equally (not the way either of it is a special case), so I'm not a fan of omitting `else` here. However, using a `switch` might be better in that case. Let me change that.


-- 
Reply to this email directly or view it on GitLab: https://gitlab.com/gnutls/gnutls/-/merge_requests/2120
You're receiving this email because of your account on gitlab.com. Unsubscribe from this thread: https://gitlab.com/-/namespace/17175643/sent_notifications/6-9u91mio3yx0cj3n793dr3aa5v-a84t7/unsubscribe | Manage all notifications: https://gitlab.com/-/profile/notifications | Help: https://gitlab.com/help


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