Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Clear method store / query cache confusion #18151

Closed
wants to merge 8 commits into from

Commits on Apr 29, 2022

  1. Drop ossl_provider_clear_all_operation_bits() and all uses of it

    This is a misused function, as it was called during query cache flush,
    when the provider operation bits were meant to record if methods for a
    certain operation has already been added to the method store.
    
    Fixes openssl#18150
    levitte committed Apr 29, 2022
    Copy the full SHA
    980f273 View commit details
    Browse the repository at this point in the history
  2. Refactor method construction pre- and post-condition

    The existing pre- and post-condition functions are supposed to check if
    methods have already been created and stored, using provider operation
    bits.  This is supposed to only be done for "permanent" method stores.
    
    However, the way the pre-condition was called, it could not know if the
    set of implementations to be stored is likely to end up in a "permanent"
    or a temporary store.  It needs access to the |no_store| flag returned
    by the provider's operation query function, because that call was done
    after the pre-condition was called.
    
    This requires a bit of refactoring, primarly of |algorithm_do_this()|,
    but also of |ossl_method_construct_precondition()|.
    
    Fixes openssl#18150
    levitte committed Apr 29, 2022
    Copy the full SHA
    0ed4622 View commit details
    Browse the repository at this point in the history
  3. Don't empty the method store when flushing the query cache

    When evp_method_store_flush() flushed the query cache, it also freed
    all methods in the EVP method store, through an unfortunate call of
    ossl_method_store_flush_cache() with an argument saying that all
    methods should indeed be dropped.
    
    To undo some of the confusion, ossl_method_store_flush_cache() is
    renamed to ossl_method_store_cache_flush_all(), and limited to do
    only that.  Some if the items in the internal ALGORITHM structure are
    also renamed and commented to clarify what they are for.
    
    Fixes openssl#18150
    levitte committed Apr 29, 2022
    Copy the full SHA
    bd1319e View commit details
    Browse the repository at this point in the history
  4. Make it possible to remove methods by the provider that provides them

    This adds ossl_method_store_remove_all_provided(), which selectively
    removes methods from the given store that are provided by the given
    provider.
    
    This also adds the EVP specific evp_method_store_remove_all_provided(),
    which matches ossl_method_store_remove_all_provided() but can also
    retrieve the correct store to manipulate for EVP functions.
    
    This allows us to modify ossl_provider_self_test() to do the job it's
    supposed to do, but through clearly defined functions instead of a
    cache flushing call that previously did more than that.
    levitte committed Apr 29, 2022
    Copy the full SHA
    081f47c View commit details
    Browse the repository at this point in the history
  5. Complete the cleanup of an algorithm in OSSL_METHOD_STORE

    The `alg_cleanup` didn't properly clear the OPENSSL_SA leaf that it
    had just freed the contents of.  Fortunately, `ossl_sa_ALGORITHM_doall_arg()`
    allows us to pass the store pointer itself as an extra argument, which
    allows a modified `alg_cleanup` to complete the job.
    levitte committed Apr 29, 2022
    Copy the full SHA
    0848d28 View commit details
    Browse the repository at this point in the history
  6. For child libctx / provider, don't count self-references in parent

    In child library contexts, which contain child "clones" of the
    providers the application has in store, one of these children will
    always be the provider that creates the child library context; let's
    call them self-refering child providers.
    
    For these self-refering child providers, we don't increment the parent
    provider reference count, nor do we free the parent provider, as those
    become self defeating and hinder the teardown and unloading process
    when the application cleans up.
    
    For non self-refering child providers, we must retain this propagation
    of reference count to the parent, so that aren't torn down too early,
    i.e. when there's still a "foreign" reference (fetched algorithm).
    levitte committed Apr 29, 2022
    Copy the full SHA
    64bc496 View commit details
    Browse the repository at this point in the history

Commits on May 4, 2022

  1. squash! Make it possible to remove methods by the provider that provi…

    …des them
    
    ossl_provider_deactivate() is also modified to remove methods associated
    with the deactivated provider, and not just clearing the cache.
    levitte committed May 4, 2022
    Copy the full SHA
    ed7d80f View commit details
    Browse the repository at this point in the history
  2. Add method store cache flush and method removal to non-EVP operations

    evp_method_store_flush() and evp_method_store_remove_all_provided()
    only cover EVP operations, but not encoders, decoders and store loaders.
    This adds corresponding methods for those as well.  Without this, their
    method stores are never cleaned up when the corresponding providers are
    deactivated or otherwise modified.
    levitte committed May 4, 2022
    Copy the full SHA
    8939ab2 View commit details
    Browse the repository at this point in the history