Re: Add ssl_(supported|shared)_groups to sslinfo

> On Fri, May 08, 2026 at 02:36:10PM -0700, Cary Huang wrote:
> Hi
> 
> Given that sslinfo is designed to expose diagnostic information
> about the current TLS connection, I am supportive of extending
> its functionality.

Thanks for looking into it.

> > /* Send the negotiated group first */
> > if (call_cntr == 0)
> > {
> > 	nid = SSL_get_negotiated_group(ssl);
> > 	group_type = CStringGetTextDatum("negotiated");
> > }
> > /* Then the shared groups */
> > else if (call_cntr < fctx->nshared + 1)
> > {
> > 	nid = SSL_get_shared_group(ssl, call_cntr - 1);
> > 	group_type = CStringGetTextDatum("shared");
> > }
> > /* And finally the supported groups */
> > else if (call_cntr < fctx->nsupported + fctx->nshared + 1)
> > {
> > 	nid = fctx->supported_groups[call_cntr - fctx->nshared - 1];
> > 	group_type = CStringGetTextDatum("supported");
> > }
> > else
> > 	SRF_RETURN_DONE(funcctx);
> > 
> > /*
> >  * SSL_group_to_name can return NULL in case of an error, e.g. when no
> >  * such name was registered for some reason.
> >  */
> > group_name = SSL_group_to_name(ssl, nid);
> > if (group_name == NULL)
> > 	ereport(ERROR,
> > 			(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
> > 			 errmsg("unknown OpenSSL group at position %d",
> > 					call_cntr)));
> 
> It is possible that SSL_get_negotiated_group() and
> SSL_get_shared_group() would return NID_undef when there is no
> negotiated group. The current code will pass that to
> SSL_group_to_name() and raise an error if it returns NULL.
> 
> Instead of failing the whole function, would it be better to
> just omit that row since the function returns a SETOF record?
> 
> if nid == NID_undef, we could just omit the row instead of
> making a call to SSL_group_to_name(), which most likely will
> fail.

It makes sense to me in general, but it looks there are some arguments
against this:

* ssl_extension_info function is also an SRF and returns an error if
  faced NID_undef. It's probably a good idea to be concistent with the
  existing functionality. 

* From what I see SRF API doesn't allow to "skip" a record, available
  options are either to finish the set with SRF_RETURN_DONE, or to
  return a NULL record with SRF_RETURN_NEXT_NULL. That means that when
  facing NID_undef, we can stop altogether and skip all the records
  after this, or have a NULL record in the output, both don't sound
  fitting the purpose here.

With this in mind I'm inclined to leave it as it is, but open for
suggestions.