On Wed, 7 Oct 2020 at 20:17, Richard Levitte <levi...@openssl.org> wrote: > > There's no reason for the EVP layer to check for the presence or the > absence or the public key, for one very simple reason: it doesn't have > access to the backend key structure and therefore has no possibility > to make any such checks. Such checks should be made in the backend.
The very case that triggered this discussion happens because the provider backend is doing the tests that libcrypto is not doing, and upholding the core&users to the documented assumption. On import/export the backend is enforcing that privkey alone is never imported/exported without its public counterpart. > That part of your proposed vote text that mentions checks in EVP > doesn't make any sense with that in mind, and I fear that it would > lead us into a rabbit hole that's not really necessary. Also, this > contradicts the intentions of the design for libcrypto vs providers, > which was that libcrypto (and therefore EVP) would be a rather thin > layer that simply passes stuff to the providers and gets results back > (there are things that complicate this a bit, but this is essentially > what we do), and leave it to the provider to decide what to do and how > to do it. Re: contradicting design principles I would also remark here that pushing quirkiness into providers to satisfy "weird" requirements that only serve the purpose of dealing with the legacy promises of libcrypto (or anyway the extra constraints enforced by libcrypto surface that is not directly facing the providers) also contradicts designs of libcrypto vs providers: libcrypto takes care of libcrypto user-facing and provider-facing API, the provider only should care about the new and well defined core-provider API. Re: why test at EVP layer? In my proposal the test at the EVP (and lower) layers is because that is part of taking care of libcrypto user-facing API: the long-standing (locally betrayed) assumption is established for `EVP_PKEY` objects, so `EVP` is the natural layer to have a first layer of testing to ensure that pervasively we can guarantee that the current documented assumption can be safely disregarded because not relevant anymore. That assumption, for which we have long-standing documentation at the EVP layer has at some point (even in the past 4 years in my limited experience) been enforced also on lower levels (it is not particularly relevant if chronologically it percolated down to the lower levels or conversely was already there all along undocumented and was put in writing only when documenting `EVP_PKEY`): that is why I included in my proposal to also build coverage for this change not only in EVP but also in the other layers. I can agree with you that some of this testing at different layers can appear quite redundant, especially in terms of which "security critical" or "potentially failing" code is tested in the current snapshot of the code: but my counter argument is that if we were less worried about redundant tests and were more rigorous in always including both positive and negative tests for all the things we put in our documentation, maybe we would have already had the tests that asserted that the code reflects our documented assumption (and in which layers) and we wouldn't have ended up violating our documented assumption by accident creating something that works in 1.1.1 by (partial) accident and that we are now considering as a potential regression in 3.0. Nicola