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 <m...@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 >