OSSL_PARAMs

[Richard Levitte]

On Wed, 05 Jun 2019 05:07:24 +0200,
Dr Paul Dale wrote:
> 
> 
> Richard wrote:
> 
>         With most OSSL_PARAM structure being dynamically created,
>         the need for the indirection seems redundant.  E.g. could the return length be moved into
>         OSSL_PARAM?  I think so.
> 
>     The design was not only to be able to have nice compile time
>     initialization, but also to be able to pass the array as 'const
>     OSSL_PARAM *', i.e. an indication to the recipient that the array
>     itself should never be modified (less chance of compromise).  Maybe
>     that's overly paranoid, but that was a line of thinking.
> 
> This is a better reason, not that I think “const” is all that useful here.
> 
> An aside: having a const struct that has a pointer to non-const memory isn’t entirely obvious to
> many.  This is a public API, make it as simple as necessary.

Surely, this can be alleviated with a comment or appropriate documentation?

>         Moving integral values into the structure is more difficult because BIGNUMs will always
>         need to be
>         references.  Allocating additional memory will still be required.  I’ve got three obvious
>         solutions:
>        
>         1. include a void * in the OSSL_PARAM structure that needs to be freed when the structure
>         is
>         destroyed or
> 
>     It's actually perfectly possible to do this today.  We already have
>     this pointer, it's called 'data’.
> 
> And how is a pointer known to be malloced or not?  I’m trying to make this easy for users without
> losing the efficiency that is possible if it is required somewhere.
> 
> I’m looking at making this kind of code work:
> 
>     OSSL_PARAMS params[10];
>     int n = 0, max_len;
>     char my_size[20];
>    
>     scanf(“%d”, &max_len);
>     params[n++] = OSSL_PARAM_construct_utf8_string(“size”, &my_size, sizeof(my_size), NULL);
>     params[n++] = OSSL_PARAM_construct_utf8_string(“name”, NULL, max_len, NULL);
>     params[n++] = OSSL_PARAM_construct_end();
>    
>     …
>    
>     OSSL_PARAM_deconstruct_all(params);

Well, going along with the idea of having a separate set of functions
called something with '_alloc_' instead of '_construct_', I could
imagine an interface like this:

    OSSL_PARAM OSSL_PARAM_alloc_utf8_string(const char *name,
                                            void **buf, size_t bufsize,
                                            size_t *return_size);

That would allow the OSSL_PARAM array owner to keep track of allocated
data by passsing in the address to a pointer, and deallocate what
needs to be deallocated after the fact.  Or in other words:

    OSSL_PARAMS params[10];
    int n = 0, max_len;
    char my_size[20];
    void *namebuf = NULL;
   
    scanf(“%d”, &max_len);
    params[n++] = OSSL_PARAM_construct_utf8_string(“size”, &my_size, sizeof(my_size), NULL);
    params[n++] = OSSL_PARAM_alloc_utf8_string(“name”, &namebuf, max_len, NULL);
    params[n++] = OSSL_PARAM_construct_end();
   
    …
   
    OPENSSL_free(namebuf);

> It is a contrived case but I think it would make using the params easier.

Not at all contrived, I can see uses for it, but then rather for the
get_params kind of call.

>         2. have a block of data in the OSSL_PARAM structure that can be used for native types
>         (OSSL_UNION_ALIGN works perfectly for this) or
> 
>     My major concern with that, apart from having to modify the OSSL_PARAM
>     items themselves¸ is that some time in the future, we will want to add
>     another native type that's larger, which means we modify the size of a
>     OSSL_PARAM.  It's a public structure, so that can't be treated
>     lightly.
> 
> This is a valid concern.  OSSL_UNION_ALIGN isn’t appropriate.
> Likewise, uintmax_t isn’t binary compatible.

I'm glad we agree.

>     If you're thinking that the receiving side should free certain values,
>     then you need to pass a pointer to the routine to be used to free the
>     value rather than just a flag.
> 
> I agree, this is a bad idea.  
> 
> The API isn’t easy to use at the moment.  It is also error prone as soon as something complex is
> encountered (which we haven’t yet).
> Shane struggled to figure this out and get it working when trying the KDFs and he is far more
> experienced than most.

I have had some experience with the MACs...

> I’ll see if I can get some kind of diff or PR together.

-- 
Richard Levitte         levitte at openssl.org
OpenSSL Project         http://www.openssl.org/~levitte/