commit b778b9c1b3d21450025e2cb22587bf79714912eb
author David Benjamin <davidben@google.com> Thu May 20 11:31:05 2021 -0400
committer Adam Langley <agl@google.com> Fri May 21 18:18:26 2021 +0000
tree 75845498d466634a114cd58169b0451bd9bfa839
parent 49ee62fe13adacfc89d3d612f79e3a8425b500c5

Const-correct SSL_get_srtp_profiles.

This is part of a very deep dependency chain. I'm sniffing at making all
the add_clienthello callbacks const. Between HelloVerifyRequest,
HelloRetryRequest, and soon ECH, we're creating lots of ClientHellos per
connection. That's probably easiest to manage if constructing a
ClientHello had no side effects.

Update-Note: The change to the return type isn't quite compatible, but I
only found one caller of this function, which has since been fixed. (If
we need to return a non-const value for compatibility, we can do that
and document that the caller should not mutate the output.)

Change-Id: I21f18f7438920a5b03d874fa548f054af3a42c4a
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/47664
Reviewed-by: Adam Langley <agl@google.com>

include/openssl/ssl.h

diff --git a/include/openssl/ssl.h b/include/openssl/ssl.h
index 3ef6d8d..2aac4e9 100644
--- a/include/openssl/ssl.h
+++ b/include/openssl/ssl.h
@@ -3054,8 +3054,8 @@
 OPENSSL_EXPORT int SSL_set_srtp_profiles(SSL *ssl, const char *profiles);
 
 // SSL_get_srtp_profiles returns the SRTP profiles supported by |ssl|.
-OPENSSL_EXPORT STACK_OF(SRTP_PROTECTION_PROFILE) *SSL_get_srtp_profiles(
-    SSL *ssl);
+OPENSSL_EXPORT const STACK_OF(SRTP_PROTECTION_PROFILE) *SSL_get_srtp_profiles(
+    const SSL *ssl);
 
 // SSL_get_selected_srtp_profile returns the selected SRTP profile, or NULL if
 // SRTP was not negotiated.

ssl/d1_srtp.cc

diff --git a/ssl/d1_srtp.cc b/ssl/d1_srtp.cc
index 96d7d51..12c8075 100644
--- a/ssl/d1_srtp.cc
+++ b/ssl/d1_srtp.cc
@@ -202,7 +202,7 @@
          ssl_ctx_make_profiles(profiles, &ssl->config->srtp_profiles);
 }
 
-STACK_OF(SRTP_PROTECTION_PROFILE) *SSL_get_srtp_profiles(SSL *ssl) {
+const STACK_OF(SRTP_PROTECTION_PROFILE) *SSL_get_srtp_profiles(const SSL *ssl) {
   if (ssl == nullptr) {
     return nullptr;
   }

ssl/t1_lib.cc

diff --git a/ssl/t1_lib.cc b/ssl/t1_lib.cc
index 970e7ba..42bfe6d 100644
--- a/ssl/t1_lib.cc
+++ b/ssl/t1_lib.cc
@@ -1720,7 +1720,8 @@
 
 static bool ext_srtp_add_clienthello(SSL_HANDSHAKE *hs, CBB *out) {
   SSL *const ssl = hs->ssl;
-  STACK_OF(SRTP_PROTECTION_PROFILE) *profiles = SSL_get_srtp_profiles(ssl);
+  const STACK_OF(SRTP_PROTECTION_PROFILE) *profiles =
+      SSL_get_srtp_profiles(ssl);
   if (profiles == NULL ||
       sk_SRTP_PROTECTION_PROFILE_num(profiles) == 0) {
     return true;
@@ -1776,11 +1777,8 @@
     return false;
   }
 
-  STACK_OF(SRTP_PROTECTION_PROFILE) *profiles = SSL_get_srtp_profiles(ssl);
-
-  // Check to see if the server gave us something we support (and presumably
-  // offered).
-  for (const SRTP_PROTECTION_PROFILE *profile : profiles) {
+  // Check to see if the server gave us something we support and offered.
+  for (const SRTP_PROTECTION_PROFILE *profile : SSL_get_srtp_profiles(ssl)) {
     if (profile->id == profile_id) {
       ssl->s3->srtp_profile = profile;
       return true;