Re: Thoughts on OSSL_ALGORITHM

2019-03-22 Thread Tim Hudson
"handle" is the wrong name for this - if you want to have private const
data then do that rather than something which might be abused for instance
specific information. It could just be an int even or a short. It doesn't
have to be a pointer.

That would reduce the likely of it being used to hold non-const data.

Tim.

On Sat, 23 Mar. 2019, 9:11 am Dr Paul Dale  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  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
>
>
>


Re: Thoughts on OSSL_ALGORITHM

2019-03-22 Thread Dr Paul Dale
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  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



Re: Thoughts on OSSL_ALGORITHM

2019-03-22 Thread Matt Caswell



On 22/03/2019 15:45, Matt Caswell wrote:
> 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.

I meant "An alternative is for the core to pass the algorithm name instead..."

Matt


Thoughts on OSSL_ALGORITHM

2019-03-22 Thread Matt Caswell
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