Skip to content

CWE 214: ssl session secret scrubbing/zeroization hardening #3494

Description

@ravikanthreddy89

Security issue notifications

This is filed as a public issue deliberately. It is a defense-in-depth / hardening gap, not an exploitable vulnerability: there is no memory-disclosure primitive here, and exploitation presupposes an attacker who can already read freed heap memory (via core dump, swap, hypervisor snapshot, or a separate UAF/OOB read).

Problem:

ssl_session_st::secret — the TLS 1.2 master secret, and the resumption secret in TLS 1.3 — is never zeroized. It remains in freed heap memory for the lifetime of the allocation.

The field is a plain array (ssl/internal.h):

uint8_t secret_length = 0;
uint8_t secret[SSL_MAX_MASTER_KEY_LENGTH] = {0};

The destructor releases only ex_data and X509 state (ssl/ssl_session.cc:842):

ssl_session_st::~ssl_session_st() {
  CRYPTO_free_ex_data(&g_ex_data_class, this, &ex_data);
  x509_method->session_clear(this);
}

And the issue might be amplified by SSL_SESSION_dup (ssl/ssl_session.cc:79):

new_session->secret_length = session->secret_length;
OPENSSL_memcpy(new_session->secret, session->secret, session->secret_length);

Every duplicate carries the secret into a fresh allocation and handled through the same non-cleansing destructor, potentially making a single handshake to leave several independent copies uncleansed/zeroized.

Solution:

Zeroize in the destructor, alongside the existing cleanup:

ssl_session_st::~ssl_session_st() {
  CRYPTO_free_ex_data(&g_ex_data_class, this, &ex_data);
  x509_method->session_clear(this);
  OPENSSL_cleanse(secret, sizeof(secret));
}
  • Does this change any public APIs? If yes, explain.
  • Which algorithm(s) will this impact?

Requirements / Acceptance Criteria:

What must a solution address in order to solve the problem? How do we know the solution is complete?

  • RFC links: Links to relevant RFC(s)
  • Related Issues: Link any relevant issues
  • Will the Usage Guide or other documentation need to be updated?
  • Testing: How will this change be tested? Call out new integration tests, functional tests, or particularly
    interesting/important unit tests.
    • Will this change trigger AWS LibCrypto Formal Verification changes? Changes to the implementation of verified
      algorithms will require additional changes.
    • Should this change be fuzz tested? Will it handle untrusted input? Create a separate issue to track the fuzzing
      work.

Out of scope:

Is there anything the solution will intentionally NOT address?

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions