The more I'm looking at this, the more I like the idea (well, kinda sorta... this is kind of a meta OSSL_PARAM, and I worry about further performance implications, but looking at the code, I imagine that I might worry too much).

I would like to see a variant where the caller can provide a block of memory to be used instead of dynamic allocation, as well as a template->param that populates a OSSL_PARAM array provided by the caller. I don't imagine that should be very hard, apart from having to handles issues with the memory block or the provided OSSL_PARAM array being too small.

Also, documentation... I assume we'll see that further on.

Another thing: naming. These is a public API within libcrypto (libcrypto-private otherwise), and we have consistently prefixed those with ossl_ (and OSSL_ for types), I think that applies here as well. With that, however, we're getting very long names, so I wonder if shorter names would be possible, such as OSSL_PARAMDEF instead of PARAM_DEFINITION and OSSL_PARAMTMPL instead of PARAM_TEMPLATE.

I'm also wondering if we should consider making these entirely public... I'm not entirely sure about that idea, but it's worth giving it some thought.

Also, documentation... I assume we'll see that further on.

Currently outstanding. I'd like to fixate the functionality to try to avoid have to make gratuitous changes.

I was following @mattcaswell's naming in #9266 to some extent. The functions there are private so I agree about the nomenclature and will update.

would like to see a variant where the caller can provide a block of memory to be used instead of dynamic allocation, as well as a template->param that populates a OSSL_PARAM array provided by the caller. I don't imagine that should be very hard, apart from having to handles issues with the memory block or the provided OSSL_PARAM array being too small.

This is an interesting alternative. I worry about the space management a little. My alignment code is a bit coarse grained which could result in a fair bit of padding. It could easily be refined so that strings and BN's are packed.

I'm also wondering if we should consider making these entirely public... I'm not entirely sure about that idea, but it's worth giving it some thought.

I had the same thought and had reservations as well.

It would be useful for 3rd party providers.

It would be useful for 3rd party providers.

Agreed, it would be useful.

My concerns are around what the long term costs would be for exposing them. Once exposed, they effectively become permanent and immutable. They'd also make public some structures that might be better left hidden -- the alignment and allocation hacks e.g.

We can always choose to expose things later but once exposed we can't retract them.

How are they useful for providers?

The 3rd party providers might wrap other providers for example?

But I of course agree that the API can start as private one and be converted to public when the need is really there.

The 3rd party providers might wrap other providers for example?

Ah, so for the need to "translate" parameters. Yeah ok, that kind of makes sense. It might be a bit extreme, though, as an incoming array of parameters already form a "template" to pass further down, unless there's a serious need to change data types.

Nomenclature changes addressed.
Test case completed.

Documentation to come.

Documentation added.

Why is the word "template" used?

I'm still missing variants with caller provided memory.

I think it's just one call that changes:

OSSL_PARAM *ossl_paramtmpl_to_param(OSSL_PARAMTMPL *tmpl, OSSL_PARAM *params, size_t params_n, void *buf, size_t buf_n);

Why is the word "template" used?

It's what @mattcaswell used :)

the non-allocating conversion function is done.

I think it's just one call that changes:

Because pushing material to the template doesn't allocate a data block?

Exactly, the data block is only allocated once the final size is known.
This avoids lots of mallocs and reallocs.

I decided to split the allocating and non-allocating calls into separate functions. It kept things simple.

Exactly, the data block is only allocated once the final size is known.

... it seems I can't read code. You're right, of course.

I'm unsure about raising specific errors from the calls.

What I worry about is for an error being signalled back all the way to the application without any suitable error number / message. (note: I'm guilty of the same thing, so this is a reminder that I need to fix my own code as well)

I've raised proper errors in all places they can occur.

Regarding secure heap, I think there's a bit of an over reaction. Let's think about this for a moment, when will it be used? For transferring private keys, right? How much non-private data do you expect to transfer in the same OSSL_PARAM? I'm ready to assume that it's the public bits and nothing more. Is that bad? Also, I expect the allocated memory to be very short lived, i.e. that it will be freed directly after the appropriate _set_params has been called, which I expect will happen very soon after the OSSL_PARAM has been constructed. Can we live with that amount of allocated secure heap for that short while?

No, it's a bug in this code. Having a flag to set other behavior is wrong. There's the risk that the flag won't get restored and the 'template' re-used. There's fragmentation of the secure heap, it is really designed for private keys sized power-of-two.

I feel strongly about this because Akamai had the same thing approach and we got burned, badly because we could not understand the code after the author left years ago. We spent millions to get new keys for all our customers.

It's very straightforward to fix this, just add a new "private bignum" type. It is clean, consistent with the rest of the uses, and obvious what is going on.

What you suggest, though, leads to allocation of two blocks, one for private key material and the other for the rest. The final call that creates the params array will be less elegant.

On the other hand, this is internal functionality...

What you suggest, though, leads to allocation of two blocks, one for private key material and the other for the rest.

Well, yes. That's the way everything else works. You cannot have a single block of memory if private keys are being passed around, and it's wrong to dump everything into the secure heap if only one of the parameters is key material. To do it properly, without exposing keys and without putting public stuff where it doesn't belong, requires two blocks. No way around it.

I agree with @richsalz, the use and implementation of secure memory (limited, locked and on-swappable) really restricts its use to (some) secret and to private keys.

Ok, so if we go with a separate secure memory where only private bits are stored, that requires passing a reference to a secure memory pointer at the end, so the final builders could look like this:

OSSL_PARAM *ossl_param_bld_to_param(OSSL_PARAM_BLD *bld, void **secdata);
OSSL_PARAM *ossl_param_bld_to_param_ex(OSSL_PARAM_BLD *bld, void **secdata,
                                       OSSL_PARAM *params, size_t param_n,
                                       void *data, size_t data_n);

That final call would then look like this:

OSSL_PARAM_BLD bld;
void *secmem = NULL;
OSSL_PARAM *params = NULL;

... /* leave out all the pushes and stuff */

params = ossl_param_bld_to_param(&bld, &secmem);

... /* calling whatever _get_params or _set_params functions */

OPENSSL_free(params);
OPENSSL_secure_free(secmem);

Regardless of which variant is called, secure memory would always be allocated if there are private bits to transfer.

Let's see what @paulidale thinks of this idea.

I could live with Richard's suggestions but they are complicating things. I'd prefer some variant of #9254, where each piece of memory is allocated independently in the appropriate location(s). We'd lose the one free to get rid of everything but a function could be created to do this.

Yes, I suggested a free routine a day ago; #9305 (comment)

Out of curiosity, how much more complicated does it become with my suggestion? You need to keep track of two data blocks (so selective calculations), and you'd keep the allocated OSSL_PARAM in the unsecured data block. Or so I assume. Am I missing something?

I've updated #9254 to include secure memory (documentation and tests lacking still).

Updated to use secure storage, but lacking finished tests and documentation.

Travis is relevant, I'll fix tomorrow unless the alternative is preferred.

I'm lost on the sequence of events here... is this essentially ready for approval or are you still working out some details, @paulidale?

After fixing the conflicts, this should be ready to go.
There will be changes to sync up #9266 with this, but nothing should break because it wasn't done.

Conflicts fixed, open for final reviews.

Спасибо товарищ. Merged to master.

I'm assuming that #9254 will now be closed without a merge.
The functionality there isn't exactly overlapping but it is close.