fix: allow session cache to be enabled before setting cache callbacks

[abidedavana]

s2n_config_set_session_cache_onoff() previously only set use_session_cache when all three cache callbacks were already registered, and gave no feedback otherwise. Callbacks set after the call could never take effect, making the call order a silent sharp edge.

Set use_session_cache unconditionally when enabling, and enforce the requirement that all three callbacks be present in s2n_allowed_to_cache_connection(), which already gates every use of the cache. This makes the callbacks and the on/off toggle order-independent while preserving NULL-callback safety.

Goal

Make s2n_config_set_session_cache_onoff() behave the same no matter when you set the cache callbacks.

Why

Fixes the sharp edge from #3463 — if you call s2n_config_set_session_cache_onoff() before setting the three cache callbacks, caching just silently stays off. The call still returns success, and setting the callbacks afterwards doesn't help, so there's no way to tell anything went wrong.

How

• s2n_config_set_session_cache_onoff() now sets use_session_cache unconditionally when enabling.
• The "all three callbacks have to be set" check moved into s2n_allowed_to_cache_connection(), which already guards every path that actually calls the callbacks. So a config with caching on but no callbacks never hits a NULL callback, and behaves exactly like it did before this change.
• Also updated the doc note in s2n.h and the usage guide, since both described the old ordering rule.

Callouts

• If you enable caching and never set the callbacks, caching still silently does nothing. I think that's inherent to removing the ordering requirement — erroring from the setter would break the now-valid enable-first order — but happy to change it if you'd prefer different behavior there.
• The existing s2n_allowed_to_cache_connection unit test was setting config->use_session_cache = 1 directly to get around this exact issue; it goes through the public API now.

Testing

• Added a unit test in s2n_resume_test.c: enable caching before any callbacks, register the callbacks one at a time (caching only kicks in once all three are there), then disable it again. The test fails without the fix.
• Full unit suite passes locally (cmake/ctest on Linux with OpenSSL).
• clang-format 18 and the repo lint scripts pass on the changed files.

Related

resolves #3463

release summary: Session cache callbacks can now be set after s2n_config_set_session_cache_onoff(). Configs that previously enabled caching before setting callbacks will now cache sessions.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

[kaukabrizvi]

Thanks for the contribution, this is a well-scoped fix! One request: could you add an end-to-end self-talk test that a session gets cached and resumed when the callbacks are set after s2n_config_set_session_cache_onoff()? s2n_self_talk_session_id_test.c is a good example to build on, with the correct ordering to exercise your fix.

[abidedavana]

Thanks for the contribution, this is a well-scoped fix! One request: could you add an end-to-end self-talk test that a session gets cached and resumed when the callbacks are set after s2n_config_set_session_cache_onoff()? s2n_self_talk_session_id_test.c is a good example to build on, with the correct ordering to exercise your fix.

Added the test ,new block at the end of s2n_self_talk_session_id_test.c. It sets the config up the other way round (cache on first, callbacks after), does a full handshake, then reconnects and resumes off the cached session. Checks the resumption handshake, that the session id matches, and that data still goes through. It fails without the fix, since the server never hits the cache.

[kaukabrizvi]

I added a release summary note to the description as this changes behavior on the wire. Feel free to tweak as needed.

[kaukabrizvi]

After this change, the check here is the only thing keeping s2n_resume_from_cache and s2n_store_to_cache from callin a NULL callbakc (since neither null-checks the callback). For added defense, you might also consider adding POSIX_ENSURE_REF in those two functions as a backstop.