WIP: Deprecate DSA_sign_setup() in the documentation

[romen]

Fixes #6447 by updating the documentation about DSA_sign_setup() as the current version is misleading and can be easily interpreted by users as suggesting to reuse the DSA nonce, which should never happen.

This should apply to master (1.1.1) and 1.1.0.

I'm not familiar with the original rationale for suggesting to precompute kinv and r, but I believe that even in 1.0.2 the paragraph about DSA_sign_setup() should be rephrased to explicitly warn users that they should not use it to precompute a single nonce and then cal several DSA_sign().
To cryptographers this might seem obvious, but we should not expect the users of the library to be cryptography experts.

I marked the PR as WIP because I need feedback about ENGINE implementations of DSA_meth: what I wrote is true for the default OpenSSL implementation, as a user has no way to feed the precomputed kinv and r back to DSA_sign, but I can imagine HW ENGINEs using this function to alter the internal state and separate the setup phase from the message/digest consuming phase.

If the above is a concern, I would rewrite the documentation to distinguish between the default OpenSSL DSA implementation (for which what I wrote is true) and alternative ENGINE implementations: to write something meaningful in this case I would definitely welcome details or suggestions on how to properly phrase what to expect from non-default implementations.

Also, depending on the answer to the above concern, I would also open an issue for the next major release about completely removing DSA_sign_setup as a public function.

Checklist

• documentation is added or updated

[HPaulS]

I can imagine HW ENGINEs using this function to alter the internal state and separate the setup phase from the message/digest consuming phase.

I could be wrong about this but I don't see the issue here. On general software design principles (not being a cryptograpgpher) I would ask the question: What does DSA_sign_setup() know that DSA_do_sign() does not? And the answer is surely 'nothing' so I can't see why it would ever be needed - just have DSA_do_sign() do whatever is needed, whatever the underlying implementation might be.

Is that a useful, neutral third-party viewpoint. I'd like to think so. Just get rid of it, I say.

[romen]

@HPaulS I can show an example of why different ENGINE implementations matter in updating the documentation for this function.

Reading this or this (which I think refer to 1.0.2 or before), the idea behind that "precomputation in time-critical scenarios" sentence is that DSA signing is made of two computationally expensive parts:

1. nonce generation and modular arithmetic voodoo on it: generate a new long random bit string k, modular exponentiation to compute r, modular inversion to compute kinv;
2. more modular arithmetic voodoo putting together the results of the previous step, the private key and the message digest: this is simpler voodoo, involving only modular additions and multiplications.

How much the computational complexity of the first step outweighs the second depends on the host architecture and the DSA modulus size.
To squeeze performance gains where the architecture offers cryptographic coprocessors one implementation could offload the setup phase to the coprocessor, in parallel run the message through the digest function on the CPU, then run the do_sign step once both outputs are ready.
In a purely software implementation that exploits multithreading and trades computational complexity for memory consumption, a thread could be dedicated to fill a queue with precomputed (r,kinv) pairs (to be used only once and then discarded) while one or more threads directly perform the do_sign step drastically reducing the latency to fulfil a request requiring a signature (as long as the precomputation queue is not depleted).

Both scenarios can be implemented using OpenSSL ENGINEs (as the (r,kinv) pairs could be passed through an internal context), and may require the DSA_sign_setup() function to let the client application (or higher level OpenSSL APIs) fully benefit from the ENGINE capabilities.

This is why I am asking feedback from the OpenSSL dev team, as I don't have a full picture of what existing alternative implementations do and expect.

@richsalz, @mattcaswell what are your thoughts on the matter?

[HPaulS]

@romen OK, I see where you're coming from. In fact I can see benefits in being able to run DSA_sign_setup () and DSA_do_sign () independently on any platform (to prepare one DSA * while signing another in a different thread, say). I have an idea.

1. DSA_sign_setup () needs to lose those two trailing parameters and be passed the message to be signed instead.

2. DSA_sign_setup () needs to stash its results in the opaque struct behind DSA * somehow. (Is that a problem?)

3. Those results need to be discarded once they have been consumed by `DSA_do_sign () for safety reasons. Implementing item 2 lets you enforce this. This behaviour should be documented.

Then just update the docs with the new API (and how it behaves) and deprecate DSA_sign_setup () in the strongest possible terms in 1.0.2 and earlier, job done. What do you think?

[mattcaswell]

I marked the PR as WIP because I need feedback about ENGINE implementations of DSA_meth: what I wrote is true for the default OpenSSL implementation, as a user has no way to feed the precomputed kinv and r back to DSA_sign, but I can imagine HW ENGINEs using this function to alter the internal state and separate the setup phase from the message/digest consuming phase.

While yes that is technically true, I think its a corner case that most users don't need to worry about. I don't think we need to elaborate it in the docs.

[HPaulS]

@mattcaswell @romen My$0.02, for what it's worth, regarding the API, rather then the docs themselves.

My benchmark (performed on Windows with 1.0.2, since that's the latest version I could test this with directly) showed that at least 99% of the time required to sign a message is spent in DSA_sign_setup(). DSA_do_sign() itself (if it doesn't have to calculate kinv and r itself) takes very little time indeed, by comparison.

So, if the decision is to get rid of DSA_sign_setup() in the API completely (and that might or might not be what Matt is implying, that's not clear to me) then I would agree. It should go, please ignore my post above.

If I need to open a new issue to request this, please let me know and I will do so. And I'm sorry if I have caused any confusion or extra work for you all by jumping in with both feet as I did.

[mattcaswell]

So, if the decision is to get rid of DSA_sign_setup() in the API completely (and that might or might not be what Matt is implying, that's not clear to me) then I would agree. It should go, please ignore my post above.

It should go from 1.2.0 (whenever that is), but cannot go from current releases due to ABI compatibility rules. An issue reminding us to do that when we get to 1.2.0 (which we can flag with the 1.2.0 label) might be helpful.

[romen]

@HPaulS I agree that also the current API is misleading, but no change whatsoever can happen in the API now, we need to wait until the next major release, as changing the public API affects binary compatibility, and that cannot happen in a minor release.

@mattcaswell approved the documentation change as it is now, simply deprecating the usage of DSA_sign_setup()

[romen]

@mattcaswell you ninjaed me!! :)

I will open the separate issue to request a change in the API for the next major version.

[HPaulS]

Thank you guys, over and out.

[richsalz]

for master (and 1.1.0 if you want).

[romen]

@richsalz do I need to open a separate pr for 1.1.0 or can this be cherry picked?

[richsalz]

I hope Matt can/will cherry-pick.

[mattcaswell]

Pushed. @romen - over to you to raise a new issue for 1.2.0. Thanks!

[romen]

I opened #6464 to track the API revision for the next major release

[romen]

See #6465 for a 1.0.2 counterpart of this PR.

[levitte]

Please note that there is a macro DEPRECATEDIN_1_2_0 that can be used to mark future deprecation. I see no issue using it for the declaration of DSA_sign_setup, and that gives an early signal as to what's going to happen, and also makes sure we don't forget.

[levitte]

The deprecation mark should only be done in the master / 1.1.1 branch, though...

[romen]

@levitte I opened #6467 to apply your suggestion