> 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.
