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; > int n = 0, max_len; > char my_size; > > 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; int n = 0, max_len; char my_size; 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 levi...@openssl.org OpenSSL Project http://www.openssl.org/~levitte/