Thoughts on OSSL_ALGORITHM

[Benjamin Kaduk]

I also like the provider data approach.

-Ben

On Sat, Mar 23, 2019 at 09:11:23AM +1000, Dr Paul Dale wrote:
> I’ve no issue having a provider data field there.  It will be useful for more than just this (S390 AES e.g. holds data differently to other implementations).
> 
> I also don’t think forcing separate functions is a big problem — most providers will only implement one or two algorithm families which will help control the redundancy.
> 
> I don’t think we should be doing a string lookup every time one of these is called.
> 
> 
> Of the three, the provider data feels clean and unique functions fast.
> 
> I’d like to avoid mandating another level of indirection (it’s slow), which is a risk with provider data.
> 
> 
> My thought: add the provider data field.  Use that when it can be done directly, use unique functions otherwise.
> The example with key and iv lengths would be a direct use.  Code that dives through a function pointer or a switch statement would be an example of not.
> 
> 
> 
> Pauli
> -- 
> Dr Paul Dale | Cryptographer | Network Security & Encryption 
> Phone +61 7 3031 7217
> Oracle Australia
> 
> 
> 
> > On 23 Mar 2019, at 1:45 am, Matt Caswell <matt at openssl.org> wrote:
> > 
> > Currently we have the OSSL_ALGORITHM type defined as follows:
> > 
> > struct ossl_algorithm_st {
> >    const char *algorithm_name;      /* key */
> >    const char *property_definition; /* key */
> >    const OSSL_DISPATCH *implementation;
> > };
> > 
> > I'm wondering whether we should add an additional member to this structure: a
> > provider specific handle. i.e.
> > 
> > struct ossl_algorithm_st {
> >    const char *algorithm_name;      /* key */
> >    const char *property_definition; /* key */
> >    const OSSL_DISPATCH *implementation;
> >    void *handle;
> > };
> > 
> > The reason to do this is because providers are likely to want to share the same
> > implementation across multiple algorithms, e.g. the init/update/final functions
> > for aes-128-cbc are likely to look identical to aes-256-cbc with the only
> > difference being the key length. A provider could use the handle to point to
> > some provider side structure which describes the details of the algorithm (key
> > length, IV size etc). For example in the default provider we might have:
> > 
> > typedef struct default_alg_handle_st {
> >    int nid;
> >    size_t keylen;
> >    size_t ivlen;
> > } DEFAULT_ALG_HANDLE;
> > 
> > DEFAULT_ALG_HANDLE aes256cbchandle = { NID_aes_256_cbc, 32, 16 };
> > DEFAULT_ALG_HANDLE aes128cbchandle = { NID_aes_128_cbc, 16, 16 };
> > 
> > static const OSSL_ALGORITHM deflt_ciphers[] = {
> >    { "AES-256-CBC", "default=yes", aes_cbc_functions, &aes256cbchandle },
> >    { "AES-128-CBC", "default=yes", aes_cbc_functions, &aes128cbchandle },
> >    { NULL, NULL, NULL }
> > };
> > 
> > Then when the "init" function is called (or possibly at newctx), the core passes
> > as an argument the provider specific handle associated with that algorithm.
> > 
> > An alternative is for the provider to pass the algorithm name instead, but this
> > potentially requires lots of strcmps to identify which algorithm we're dealing
> > with which doesn't sound particularly attractive.
> > 
> > A second alternative is for each algorithm to always have unique functions
> > (which perhaps call some common functions underneath). This sounds like lots of
> > unnecessary redundancy.
> > 
> > Thoughts?
> > 
> > Matt
>