> On 5 Jun 2019, at 12:34 pm, Richard Levitte <levi...@openssl.org> wrote: > > Aside from the discussion below, if there's one thing I would like to > change, it the double indirection for the _PTR data types. The data > types could still be used to indicate that the value isn't short > lived, but could possibly change names to something like > OSSL_PARAM_UTF8_CSTRING and OSSL_PARAM_OCTET_CSTRING (C for Constant). > > On Wed, 05 Jun 2019 01:18:56 +0200, > Dr Paul Dale wrote: >> Shane’s major complaints are about the indirection the OSSL_PARAM structure >> forces — for integers >> and return lengths and the necessity of allocating additional memory in >> parallel with the >> OSSL_PARAM. >> >> The extra indirection was intended to support const arrays of OSSL_PARAM, >> which turn out to be a >> rarity because they aren’t thread safe. > > The reason why we have this issue is our base C language version > choice. C90 doesn't allow this construct: > > int foo(whatever) > { > int haha = 0; > const OSSL_PARAM params[] = { > { 'foo', OSSL_PARAM_INTEGER, &haha, sizeof(haha), NULL }, > { NULL, 0, NULL, 0, NULL } > }; > > ... > } > The above code is great in theory, but it looks like in practice we end up dynamically allocating in most cases anyway (via the construct_ methods). And if this is the normal use case then OSSL_PARAMS could be made opaque and only accessed by API’s, then the argument about adding extra types later on should also disappear?
> Because the compiler for that language version isn't allowed to emit > code to use '&haha' in an inititializer. Newer C language versions > allow this. > > So while this is an issue for *us*, it isn't necessarily an issue for > our users, all depending on what C language version they use. > >> 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. > >> 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'. > >> 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. > > Also, with a union of native types, we're losing uniformity on MSB > first platforms. Having an exact 1:1 integer size match will be > crucial, and that complicates the code quite a bit... not to mention > that we have a compatibility problem as soon as one end has a new > native type in the union and the other doesn't. > (one would imagine that simply using uintmax_t would cover all integer > sizes apart from BIGNUM, but the potential size change of that type > with newer compilers make such a choice precarious) > >> 3. add a flag field to the OSSL_PARAM to indicate that the referenced value >> needs to be freed. > > By whom? The owner of the array should be in complete control of > what's needed already, so should be able to know what needs being > deallocated or not. > > 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. > >> The memory allocation comes to the for when reading e.g. a file and >> extracting data — either the >> reader needs a lot of local variables to hold everything or it has to >> allocated for each. The >> file’s data is transient in memory. >> >> For the most part, the receiver side APIs seem reasonable. It is the owning >> side that has the >> complications. >> >> I think I might be able come up with some owner side routines that assist >> here but allowing >> changes to the params structure would be far easier. >> >> I kind of like using the OSSL_PARAM arrays as a replacement for string ctrl >> functions if not ctrl >> as well (subject to backward compatibility concerns). >> >> Pauli >> -- >> Dr Paul Dale | Cryptographer | Network Security & Encryption >> Phone +61 7 3031 7217 >> Oracle Australia >> >> On 4 Jun 2019, at 11:26 pm, Richard Levitte <levi...@openssl.org> wrote: >> >> On Tue, 04 Jun 2019 14:57:00 +0200, >> Salz, Rich wrote: >> >> Part of the idea was that this would be a means of communication >> >> between application and provider, just like controls are with >> libcrypto sub-systems. >> >> I can probably find the email thread (or maybe it was a GitHub >> comment on my proposal for params), where you said, quite >> definitively, that this was *not* a general-purpose mechanism but >> rather a way to expose the necessary internals for opaque objects >> like RSA keys. >> >> Either I misunderstood what you said at the time, or you misunderstood >> what I said... there's definitely a disconnect here somewhere. >> >> What I wonder is why it should be exclusively only one of those >> options? >> >> Either way, the OSSL_PARAM is defined publically and openly (i.e. >> non-opaque), and we currently have the following functions in the >> public API: >> >> EVP_MD_CTX_set_params >> EVP_MD_CTX_get_params >> OSSL_PROVIDER_get_params >> >> I fully expect that more will come. I have a branch where I've >> EVP_MAC_CTX_set_params, for example, and I wouldn't be surprised if >> EVP_CIPHER_CTX_set_params and EVP_CIPHER_CTX_get_params appear before >> long (I'm actually rather surprised they haven't already), and I'm >> absolutely sure we will see similar functions for asymmetric >> algorithms. >> >> What changed your mind? >> >> Perhaps not surprisingly, I agree with Shane's assessment and am >> strongly opposed to the project foisting this on everyone at this >> time. @DavidBen, your thoughts? >> >> Maybe we're reading differently, I didn't see Shane being opposed to >> parameter passing in this way per se, just the exact form of the >> OSSL_PARAM structure, which is different. >> >> Cheers, >> Richard >> >> -- >> Richard Levitte levi...@openssl.org >> OpenSSL Project >> https://urldefense.proofpoint.com/v2/url?u=http-3A__www.openssl.org_-7Elevitte_&d=DwIFaQ&c=RoP1YumCXCgaWHvlZYR8PZh8Bv7qIrMUB65eapI_JnE&r=b1aL1L-m41VGkedIk-9Q7taAEKIshTBwq95Iah07uCk&m=9ytfNGgWmI_VuIgUOtVRqe_gd7wVOdag8ayBWLrTL_Q&s=PH8nRCRnGHZdpfCcpSTpW9mLIgviKCbEw6-5w7cc5i4&e= >> >> <https://urldefense.proofpoint.com/v2/url?u=http-3A__www.openssl.org_-7Elevitte_&d=DwIFaQ&c=RoP1YumCXCgaWHvlZYR8PZh8Bv7qIrMUB65eapI_JnE&r=b1aL1L-m41VGkedIk-9Q7taAEKIshTBwq95Iah07uCk&m=9ytfNGgWmI_VuIgUOtVRqe_gd7wVOdag8ayBWLrTL_Q&s=PH8nRCRnGHZdpfCcpSTpW9mLIgviKCbEw6-5w7cc5i4&e=> >> >> > -- > Richard Levitte levi...@openssl.org <mailto:levi...@openssl.org> > OpenSSL Project > https://urldefense.proofpoint.com/v2/url?u=http-3A__www.openssl.org_-7Elevitte_&d=DwIFaQ&c=RoP1YumCXCgaWHvlZYR8PZh8Bv7qIrMUB65eapI_JnE&r=b1aL1L-m41VGkedIk-9Q7taAEKIshTBwq95Iah07uCk&m=9ytfNGgWmI_VuIgUOtVRqe_gd7wVOdag8ayBWLrTL_Q&s=PH8nRCRnGHZdpfCcpSTpW9mLIgviKCbEw6-5w7cc5i4&e= > > <https://urldefense.proofpoint.com/v2/url?u=http-3A__www.openssl.org_-7Elevitte_&d=DwIFaQ&c=RoP1YumCXCgaWHvlZYR8PZh8Bv7qIrMUB65eapI_JnE&r=b1aL1L-m41VGkedIk-9Q7taAEKIshTBwq95Iah07uCk&m=9ytfNGgWmI_VuIgUOtVRqe_gd7wVOdag8ayBWLrTL_Q&s=PH8nRCRnGHZdpfCcpSTpW9mLIgviKCbEw6-5w7cc5i4&e=>