Skip to content

TSSslSecretSet: Update SSL_CTX TLS Secrets#8368

Merged
bneradt merged 1 commit intoapache:masterfrom
bneradt:fix_TSSslSecretSet_for_stored_ssl_contexts
Oct 1, 2021
Merged

TSSslSecretSet: Update SSL_CTX TLS Secrets#8368
bneradt merged 1 commit intoapache:masterfrom
bneradt:fix_TSSslSecretSet_for_stored_ssl_contexts

Conversation

@bneradt
Copy link
Contributor

@bneradt bneradt commented Oct 1, 2021

TSSslSecretSet previously updated the in-memory certificate data stored
in SSLSecret, but did not update the stored SSL_CTX certificates. As a
result, any newly created SSL contexts would get the updated secret
while the previously stored contexts would still use the old secrets.
This patch updates TSSslSecretSet to also update the secrets for stored
SSL_CTX objects.

This also fixes the related SSLSecret debug logging which had formatting
issues, rendering their output unreadable.

TSSslSecretSet previously updated the in-memory certificate data stored
in SSLSecret, but did not update the stored SSL_CTX certificates. As a
result, any newly created SSL contexts would get the updated secret
while the previously stored contexts would still use the old secrets.
This patch updates TSSslSecretSet to also update the secrets for stored
SSL_CTX objects.

This also fixes the related SSLSecret debug logging which had formatting
issues, rendering their output unreadable.
@bneradt bneradt added the TLS label Oct 1, 2021
@bneradt bneradt added this to the 10.0.0 milestone Oct 1, 2021
@bneradt bneradt self-assigned this Oct 1, 2021
@bneradt bneradt merged commit 1ae919a into apache:master Oct 1, 2021
@bneradt bneradt deleted the fix_TSSslSecretSet_for_stored_ssl_contexts branch October 1, 2021 21:28
bneradt added a commit that referenced this pull request Oct 1, 2021
@zwoop
Copy link
Contributor

zwoop commented Nov 9, 2021

Cherry-picked to v9.2.x

@zwoop zwoop modified the milestones: 10.0.0, 9.2.0 Nov 9, 2021
zwoop pushed a commit that referenced this pull request Nov 9, 2021
TSSslSecretSet previously updated the in-memory certificate data stored
in SSLSecret, but did not update the stored SSL_CTX certificates. As a
result, any newly created SSL contexts would get the updated secret
while the previously stored contexts would still use the old secrets.
This patch updates TSSslSecretSet to also update the secrets for stored
SSL_CTX objects.

This also fixes the related SSLSecret debug logging which had formatting
issues, rendering their output unreadable.

(cherry picked from commit 1ae919a)
moonchen pushed a commit to moonchen/trafficserver that referenced this pull request Mar 7, 2022
* asf/master: (47 commits)
  Doc: cleanup build errors. (apache#8377)
  Doc: Add proxy.config.cacvhe.mutex_retry_delay (apache#8376)
  Adds support for TCP_NOTSENT_LOWAT sockopt (apache#8354)
  Added support for promoting internal (plugin-initiated) requests. (apache#8363)
  Removed references to the throttle option from the slice plugin. (apache#8373)
  Pre-warming TLS Tunnel (apache#7661)
  Traffic Dump: update json-schema for new tuple requirements (apache#8370)
  TSSslSecretSet: Update SSL_CTX TLS Secrets (apache#8368)
  Added support for verifying cacheability before attempting to force an object into cache (apache#8364)
  [doc] Add a note for TSLifecycleHookAdd. Warn users that a contp could eventually be executed in a ET_NET when it was originally scheduled in the ET_TASK. (apache#8344)
  check size of session, and free sessions the ATS way (apache#8330)
  Locking around SSLSecret::secret_map access (apache#8358)
  Stabilize regex_revalidate Au test. (apache#559) (apache#8360)
  change MemArena::make test to remove memory leak (apache#8352)
  free sessions when timeout (apache#8356)
  test_MMH: fix memory leak in unit test (apache#8357)
  crash fix (apache#8268)
  remove unused RecConfigFileEntry struct (apache#8353)
  Rename outbound_conntrack to global_outbound_conntrack to reduce confusion. (apache#8343)
  remove unused RecConfigFileEntry from RecConfigParse (apache#8348)
  ...
moonchen pushed a commit to moonchen/trafficserver that referenced this pull request Mar 17, 2022
* asf/9.2.x: (50 commits)
  Updated ChangeLog
  Reject Transfer-Encoding in pre-HTTP/1.1 requests (apache#8451)
  Better TLS Secrets Truncation. (apache#8489)
  ssl_secret debug printing: print only the first 50 bytes (apache#8483)
  Define TS_HTTP_VALUE_BROTLI and TS_HTTP_LEN_BROTLI (apache#8477)
  Fix case of brotli (apache#8476)
  TSSslSecretSet: Update SSL_CTX TLS Secrets (apache#8368)
  Adding doc/README.md (apache#8420)
  Doc: fix typos in Strategy documentation (apache#8408)
  Refactors and promotes the Txn Control mechanism with Get() and Set() (apache#8428)
  tests: Add shbang to python scripts with a main (apache#8430)
  Remove empty tests/unit_tests directoy+makefile (apache#8429)
  Adds new API: TSVConnSslSniGet (apache#8313)
  rate_limit: convert to using TSVConnSslSniGet (apache#8414)
  Update the Multiplexer Docs for Multplexed HTTPS Connections (apache#8440)
  bigobj: use automake to build test utilities (apache#8441)
  Make sni.yaml errors cause an unrecoverable TS crash at startup. (apache#8208)
  Fix timeout checks of NetHandler::manage_active_queue() (apache#8287)
  Fix Multiplexer POST/PUT Body Handling (apache#8439)
  Document proxy.config.memory.max_usage (apache#8450)
  ...
ywkaras pushed a commit to ywkaras/trafficserver that referenced this pull request Mar 16, 2023
This PR partially reverts PR apache#8368.  SSL contexts (SSL_CTX) should only be updated by TSSslSecretUpdate()
(which is only called for the main, not the related secret).  The update in TSSslSecretSet() is not merely redundant,
it causes errors.  When an update is done, OpenSSL will check that the cert public key matches the private key.  Since
TSSslSecretSet() can only set one at time, if it also updates, they will not match.

Contexts for a loading confguration never have to be updated, becasue no SSL_CTX's have been created using it yet.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants