OSSL_ECHSTORE_flush_keys() should not return an error if there are no keys in list

[gstrauss]

OSSL_ECHSTORE_flush_keys() should not return an error if there are no keys in list.

--- a/ssl/ech/ech_store.c
+++ b/ssl/ech/ech_store.c
@@ -1114,10 +1114,6 @@ int OSSL_ECHSTORE_flush_keys(OSSL_ECHSTORE *es, time_t age)
         return 0;
     }
     num = (es->entries == NULL ? 0 : sk_OSSL_ECHSTORE_ENTRY_num(es->entries));
-    if (num == 0) {
-        ERR_raise(ERR_LIB_SSL, ERR_R_PASSED_INVALID_ARGUMENT);
-        return 0;
-    }
     for (i = num - 1; i >= 0; i--) {
         ee = sk_OSSL_ECHSTORE_ENTRY_value(es->entries, i);
         if (ee == NULL) {

If I previously had valid keys, ran OSSL_ECHSTORE_flush_keys() with an expiration and ended up with 0 keys, then some time later I ran OSSL_ECHSTORE_flush_keys() again, why is that an error for OSSL_ECHSTORE_flush_keys()? As long as OSSL_ECHSTORE *es is not NULL, OSSL_ECHSTORE_flush_keys() should flush keys that have expired and should return success.

It may be a misconfiguration elsewhere that something did not generate new keys and did not load those keys, but that should be a bug elsewhere. The caller of OSSL_ECHSTORE_flush_keys() might load new keys and then might check es before calling SSL_CTX_set1_echstore(). If es is empty at that point, the caller might issue debug trace. OSSL_ECHSTORE_flush_keys() is not right the place to check this.

Also, the current ECHSTORE interfaces in OpenSSL features/ech branch are inefficient, requiring memory allocation to retrieve a copy of es in order to check OSSL_ECHSTORE_num_keys() and then OPENSSL_free(es).

[gstrauss]

ping @sftcd