The OSSL_PARAM structure needs to be visible and not subject to change. Providers shouldn’t necessarily have a dependency on functions from libcrypto.
Pauli -- Dr Paul Dale | Cryptographer | Network Security & Encryption Phone +61 7 3031 7217 Oracle Australia > On 5 Jun 2019, at 12:47 pm, SHANE LONTIS <shane.lon...@oracle.com> wrote: > > > >> On 5 Jun 2019, at 12:34 pm, Richard Levitte <levi...@openssl.org >> <mailto: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 >>> <mailto: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 <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=> >>> >>> >> -- >> 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=>