Skip to content

Conversation

bneradt
Copy link
Contributor

@bneradt bneradt commented Jul 25, 2025

This applies to the cqssu (curve) and cqssg (group name) TLS log fields.

This does two things:

  • Updates server session caching logic to provide the TLS group name on
    resumption.
  • Updates the logging of the curve and group name for ticket resumption
    logic so that the TLS curve and group names are properly retrieved
    from the SSL object. For ticket resumption, there is no curve/group
    name stored to retrieve. It is properly retrieved from the SSL object.

Before this change: (1) the SSL group was always retrieved from the SSL
object when it should have been retrieved from the server session cache
ex_data for session cache resumption and (2) the curve value for cqssu
was alway "-" for TLS ticket resumption because there was no cached
value to retrieve like there is with server session caching.

Fixes: #12398

@bneradt bneradt added this to the 10.2.0 milestone Jul 25, 2025
@bneradt bneradt self-assigned this Jul 25, 2025
@bneradt bneradt added the TLS label Jul 25, 2025
@bneradt bneradt force-pushed the tls_group_logging_with_reuse branch 5 times, most recently from a4364fe to 0bad782 Compare July 28, 2025 18:40
@bneradt bneradt added the Bug label Jul 28, 2025
@bneradt bneradt force-pushed the tls_group_logging_with_reuse branch from 0bad782 to 2579056 Compare July 28, 2025 18:53
@bneradt bneradt changed the title TLS group name: handle SSL resumption TLS curve/group name logging for Ticket Resumption Jul 28, 2025
@maskit
Copy link
Member

maskit commented Jul 29, 2025

It doesn't seem like we need to add a new variable to record a group name. Although both OpenSSL and BoringSSL have different APIs for curve and group, the getters are actually just aliases unlike the setters for context objects. We should be able to use the same variable for both curve and group.

TLSBasicSupport::get_tls_group which was added by #12261 is probably unnecessary. I don't know why the previous code doesn't work for group, but the function to get curve/group id is identical.

$ git grep SSL_get_shared_curve include/
include/openssl/ssl.h.in:# define SSL_get_shared_curve          SSL_get_shared_group

Also, SSL_get0_group_name is only available on OpenSSL 3.2 or later. I see the ifdef and get_tls_group probably returns nullptr on OpenSSL 3.0, but I'd be surprised if there's no way to achieve it on 3.0.

@bneradt
Copy link
Contributor Author

bneradt commented Jul 29, 2025

Also, SSL_get0_group_name is only available on OpenSSL 3.2 or later. I see the ifdef and get_tls_group probably returns nullptr on OpenSSL 3.0, but I'd be surprised if there's no way to achieve it on 3.0.

This is a great point. I updated the patch for openssl 3.0 and 3.1 via SSL_get_negotiated_group. Since 3.2, the docs say to prefer the new SSL_get0_group_name, but for 3.0 and 3.1 we can use the former. See:

#elif HAVE_SSL_GET_NEGOTIATED_GROUP                    // OpenSSL 3.0/3.1
  int group_nid = SSL_get_negotiated_group(ssl);
  if (group_nid != NID_undef) {
    char const *group_name = OBJ_nid2sn(group_nid);
    return group_name != nullptr ? std::string_view(group_name) : "";
  }
  return "";

@bneradt bneradt force-pushed the tls_group_logging_with_reuse branch 3 times, most recently from 61d72cd to 0d8f683 Compare July 30, 2025 22:56
@bneradt bneradt removed this from ATS v10.1.x Aug 4, 2025
@bneradt bneradt requested a review from maskit August 4, 2025 22:17
This applies to the cqssu (curve) and cqssg (group name) TLS log fields.

This does two things:

* Updates server session caching logic to provide the TLS group name on
  resumption.
* Updates the logging of the curve and group name for ticket resumption
  logic so that the TLS curve and group names are properly retrieved
  from the SSL object. For ticket resumption, there is no curve/group
  name stored to retrieve. It is properly retrieved from the SSL object.

Before this change: (1) the SSL group was always retrieved from the SSL
object when it should have been retrieved from the server session cache
ex_data for session cache resumption and (2) the curve value for cqssu
was alway "-" for TLS ticket resumption because there was no cached
value to retrieve like there is with server session caching.

Fixes: apache#12398
@bneradt bneradt force-pushed the tls_group_logging_with_reuse branch from 0d8f683 to 1971378 Compare August 21, 2025 20:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

cqssu: Always empty for sessions resumed via TLS Ticket resumption
2 participants