Nicola we already have a interface that sets blinding to on or off..
> On 9 Apr 2021, at 9:48 am, Nicola Tuveri <nic....@gmail.com> wrote: > > Thanks for the feedback Tim! > I see your points and I value the discussion. > > Reading your message I realized that probably I got carried away in some of > the details in the lengthy email, and lost focus on part of the message I > wanted to convey. > > Setting aside the matter of secure by default designs, > I don't oppose having a property system to let users select whatever > implementation from a menu of available implementations. > But when these are boolean properties as proposed in the PR, they should be > objective properties with a universally clear definition that is immutable > over time and clear cut between yes/no. > > This is not the case for blinding=yes because it is not a boolean property. > In most cases a blinding implementation does "some" blinding at some level, > but does not blind every single computation. > Moreover the "amount" of blinding that qualifies an implementation as > performing the minimum required blinding changes over time, over threat > models, different standardizing bodies/different subjective considerations: > it's not an objective property of an algorithm implementation. > blinding=no has better semantics because you could define it as "this > implementation intentionally avoids any kind of blinding", and that would be > an objective, universal, and immutable definition. > Same applies for consttime=yes vs consttime=no. > Then I went on blabbing about the downsides of the =no variants of this, > because they are cases that don't admit a well defined =yes counterpart. I > included that to justify why I don't believe that flipping the flag logic > here would be enough to make the PR acceptable (IMO). > > I would not oppose this PR if it was defining a property string to mark > another objective quality of the implementation: e.g. > supports_cofactor_ecdh=yes vs =no, or compressed_format=yes, batching=yes, > multiprime=yes, padding=this_or_that_padding_standard as I don't argue about > having the provider=whatever that qualifies the origin of an implementation, > or labvalidated=cert_program_xyz, ISOxxxxx_score=3.14 and similar things. > Those properties have clear definitions and could be perfectly useful for > apps and end-users to choose among a rich menu of alternatives. No objection > there. > > You make another point with which I tend to disagree strongly: some > actionable information must be better than no information at all. > > I agree with it when it is stated like that, but I believe that is not the > case if the "some information" has a high risk of being actually > "disinformation". > In such cases the extra information can actually do more harm than good, e.g. > false sense of security, misled choices, risks of ossification, technical > debt, maintenance burden, etc. > > What leads me to qualify blinding=yes as a property string with high risks of > quickly degrading into disinformation? The fact that it has ill-defined > semantics as I verbosely tried to argument. > > The bits about the scope of well-definedness (apologies I fail to phrase this > in a better way) are both to support the above argument and to start a > discussion on how to establish criteria that would allow coordination among > the project and our community (incouding different provider authors and the > end-users) so that there is process to create well-defined shared property > strings for those qualities that cannot be reduced to definitions given by > some existing standardization body. > In such a propquery program, I am of the opinion that "applies > countermeasures to prevent some side-channel attacks" should be deemed > unacceptable because of the reasons stated above, and rejected. > > > > Thanks again for you feedback, I hope this time I did a better job of > separating the concerns I have about the semantics of the proposed property > string and on the need to establish some guidance to have some degree of > coherence in the definition of property strings. > I see it already as a problem that in this and the previous email a lot of my > arguments had to be prefixed with "in my opinion" because we lack a framework > to guide our own choices. > > > Nicola > > > P.S. aside from the main topic, I understand provider authors are free to > define and use properties as they see fit, and we are not limiting that > freedom in my proposal. But we need a framework to guide our decisions in > soundly designing and defining our own properties, and also a system to allow > coordinated properties when desirable (which is relevant here because > blinding=yes is an attempt at establishing a shared property definition for > end-users and 3rd party providers, given that its only purpose is to let > end-users decide among some of our implementations or some yet to come 3rd > party implementation). > > On Fri, Apr 9, 2021, 01:50 Tim Hudson <t...@cryptsoft.com > <mailto:t...@cryptsoft.com>> wrote: > Nicola, you are (in my view) conflating multiple items. > > For the provider and property design approach it is rather simple. > - Algorithm implementations can vary. > - Selection between algorithm implementations when multiple providers are > available is performed by properties > > Algorithm implementations should declare whatever set of properties they feel > is appropriate for their implementation. > Applications (and in this context most likely directly by end-user > configuration) should be able to select which properties are considered most > important for their context. > That decision capability must be left to the end user as only the end user > knows the security context in which they are operating - we don't know that > ourselves. > > The vast majority of your lengthy email below is actually focused on one > issue - what should the default behaviour be for selection of implementations > - and your viewpoint that we should not mark different properties at all that > might impact security. I don't think that position is supportable - in that > you are basically arguing that we should never declare anything about > properties of implementations and should never select between different > implementations except at a "provider level" approach. Your approach is that > all implementations should be considered equal and that pretty much defies > logic in my view. > > Different implementations have different characteristics. Even looking at > something like constant time - not all of our implementations are constant > time. > > Your statement that the approach of declaring properties "promotes insecure > by default" is simply flawed logic - and following the same logic I could > state that your approach "promotes ignorance by default" as it effectively > states that users shouldn't know the properties of the implementation in a > manner that allows selection to be performed. > > Not all implementations are the same and different implementations can > implement different mitigations. Properties allow us to declare those > mitigations and allow users to make different decisions on the basis of the > properties that providers declare for algorithms. Having that information > available has to be a better thing than having nothing available - as with > nothing available then no selection between alternatives is possible. > > Separately arguing about what the default set of properties should be (i.e. > what mitigations should we configure as required by default) would make sense > to do so - but arguing that the information for making such decisions > shouldn't be present simply makes no sense. > > Tim. > > > On Fri, Apr 9, 2021 at 3:02 AM Nicola Tuveri <nic....@gmail.com > <mailto:nic....@gmail.com>> wrote: > Background > ========== > > [PR#14759](https://github.com/openssl/openssl/pull/14759 > <https://urldefense.com/v3/__https://github.com/openssl/openssl/pull/14759__;!!GqivPVa7Brio!PiGg3zyLqwziPdnGYV3o9fY0UuVI8YpAii4YGV0a0aFmPrP9Ltzf_YobVMeQe80UgA$>) > (Set > blinding=yes property on some algorithm implementations) is a fix for > [Issue#14654](https://github.com/openssl/openssl/issues/14654 > <https://urldefense.com/v3/__https://github.com/openssl/openssl/issues/14654__;!!GqivPVa7Brio!PiGg3zyLqwziPdnGYV3o9fY0UuVI8YpAii4YGV0a0aFmPrP9Ltzf_YobVMeNBLf63Q$>) > which > itself is a spin-off of > [Issue#14616](https://github.com/openssl/openssl/issues/14616 > <https://urldefense.com/v3/__https://github.com/openssl/openssl/issues/14616__;!!GqivPVa7Brio!PiGg3zyLqwziPdnGYV3o9fY0UuVI8YpAii4YGV0a0aFmPrP9Ltzf_YobVMf53in-dQ$>). > > The original issue is about _Deprecated low level key API's that have no > replacements_, among which the following received special attention and > were issued a dedicated issue during an OTC meeting: > > ~~~c > // RSA functions on RSA_FLAG_* > void RSA_clear_flags(RSA *r, int flags); > int RSA_test_flags(const RSA *r, int flags); > void RSA_set_flags(RSA *r, int flags); > > // RSA_FLAG_* about blinding > #define RSA_FLAG_BLINDING > #define RSA_FLAG_NO_BLINDING > > // RSA functions directly on blinding > int RSA_blinding_on(RSA *rsa, BN_CTX *ctx); > void RSA_blinding_off(RSA *rsa); > BN_BLINDING *RSA_setup_blinding(RSA *rsa, BN_CTX *ctx); > ~~~ > > The decision the sprung Issue#14616 and PR#14759 was to use the > propquery mechanism to let providers advertise algorithms as > `blinding=yes` to select secure implementations if there are insecure > ones present as well. > > Similarly it was discussed the potential `consttime=yes` property that > would work in a similar way: if applied properly for our current > implementations that are not fully constant time it would allow a > user/sysadmin/developer to prefer a third party implementation for the > same algorithm with better security guarantees. > In some contexts the consttime implementation might be seriously > penalizing in terms of performance and in the contexts where const time > is not required this would allow to select accordingly. > > Definition for the blinding property > ------------------------------------ > > The current definition of the `blinding` property applies to > provider-native algorithm implementations for the `asym_cipher` and > `signature` operations: > > ```pod > =head2 Properties > > The following property is set by some of the OpenSSL signature > algorithms. > > =over 4 > > =item "blinding" > > This boolean property is set to "yes" if the implementation performs > blinding to prevent some side-channel attacks. > ``` > > Rationale > ========= > > Property queries are our decision making process for implementation > selection, and has been part of the design for 3.0 since the Brisbane > design meetings: I am not opposing the use of the property query > mechanism to select algorithms here, but the semantics of the properties > we decide to adopt (and thus endorse and encourage also for use by 3rd > parties). > In particular I see the following issues with choices like > `blinding=yes` or `consttime=yes`. > > Design promotes insecure by default > ----------------------------------- > > This design is a slippery slope into patterns that go against the > "secure by default" best practices. > Users/administrators/developers should not have to opt-in for the safer > implementation, but the other way around: the opt-in should be for the > less safe but more performant implementations after evaluating the > consequences of such a choice in specific contexts. > We shouldn't have users having to query for `consttime=yes` algorithms > but rather for `consttime=no` explicitly in specific conditions. > So if this was the only issue with PR#14759 my recommendation would be > to rather flag the non-blinding implementations as such rather than the > other way around as is currently done. > > The scenario in which 3rd party providers offer insecure algorithms not > flagged as such with `consttime=no` or `blinding=no` IMO would then fall > under the "know your providers" umbrella: if you are installing provider > X you should be well aware that you are trusting X's authors "to do the > right thing". > > The project history showed us how the risk for insecure-by-default > designs and its consequences are not just an hypothetical matter of > debate: for example, `BN_FLG_CONSTTIME` has been with us since version > 0.9.8f (and even before that under the `BN_FLG_EXP_CONSTTIME` name), > well over 14 years, in which the decision of having the flag off by > default and requiring to manually enabling it when desired has been the > cause of many vulnerabilities and fixes, that are still a concern today, > as we fixed yet another instance of forgetting to set it just some weeks > ago (PR#13889). > > In this case, though, I expect that just flipping the default is not > enough to accept this PR: the problem of going with the negative > versions of these flags is that such design can't be made future proof > as we wish to do: we can't know in advance all possible properties that > should be declared as `=no` by an algorithm released today as part of > our providers when new relevant properties might be defined in the > future. > E.g., what if after 3.0 is released a ECCKiila provider was released and > opted to tag its implementations as `formally_verified`? > They could add `formally_verified=yes` but then they would fall into the > "insecure by default" pattern: users would need to opt-in for the safer > version instead of getting it by default, but there is no other option > given that our and others' providers' offerings were not preemptively > flagged with `formally_verified=no`. > > Scope of propquery semantics > ---------------------------- > > This follows directly from the previous example. The design of the > propquery mechanism seems (IMO) to be limited to provide > __well-defined__ semantics for the properties only within the scope of a > single provider (and a single provider version at that). > > If a given provider offered at runtime for the same algorithm two or > more versions, one flagged as `consttime=no` and one with > consttime guarantees, then it could provide documentation to its > userbase on how to setup default propqueries in the configuration file > at installation time or to application developers to embed them in their > code depending on the use case, to select the `consttime=no` offering > when such a thing is perceived as desirable for whatever (ill-guided :P) > reason. > > But doing this across providers does not really work, because other > provider authors might have not flagged their insecure implementations > with `consttime=no`, or because there might be very different opinions > on what qualifies as "fully consttime" (that evolve over time with > different threat models and attacker capabilities): > provX's `consttime=yes` might be "more consttime" than provY's > `consttime=yes`. > For example provX is similar to our default provider, and > offers an EC algorithm built on top of a generic BIGNUM module so its > generic implementation supports any valid EC curve named or custom and > orgX made sure that the ECDH/ECDSA and ECmath layers of the > implementation follow state-of-the-art practices regarding > secret-dependant code execution or data access so they flag their > EC implementation as `consttime=yes`, even though the underlying BIGNUM > module has limited consttime guarantees, so in certain circumstances a > dynamic `realloc` or some other seemingly minor thing could leak a few > secret bits to a motivated attacker. > provY is similar to our default provider, and provides an EC algorithm, > but instead of having a generic codepath they only support a subset of > named curves (e.g. only P256 and P521) each with a dedicated > implementation that removes the need for a generic BIGNUM layer and > instead embeds its specific consttime field arithmetic in each curve > implementation (e.g. they provide our `ecp_nistz256.c` and > `ecp_nistp521.c` implementations only). provY's ECDH/ECDSA, ECmath and > FFmath layers all follow best consttime practices so they flag as > `consttime=yes` their EC offering. > Now `constime=yes` is not well-defined anymore across providers, making > it quite useless for users and providers to use `consttime=yes` or > `consttime=no` to pick among offerings from different providers. > > The same would be true within the same organization and provider, across > provider versions. Let's say tomorrow we were to merge a PR that made > all of BIGNUM resistant against all currently known timing side-channel > techniques. Coincidentally we decided to mark all our pubkey providers > as `consttime=yes` after verifying that all the layers above BIGNUM are > also timing-resistant to current techniques. > Fast-forward a year and a new CPU design flaw is discovered, giving a > remote attacker a side-channel with unprecedented resolution and > SNR: our previous `consttime=yes` semantic would not stand the > test of time and the disruption of semantics could have ripple-effects > within our providers and to the stack of applications that embedded > `consttime=yes` as a selection mechanism to pick among offerings from > various provider versions with now ill-defined qualifiers. > > If we can agree, even just on some level or some specific cases like > these, that the definitions of properties in propqueries have their > scope limited to the single provider version, what is the point of > exposing `blinding=*` to our users, given that at runtime we don't offer > alternative implementations for the same algorithm with different > properties? > (I mean, yes we have alternative choices at runtime in some cases, but > not at the algorithm level in the provider API, they are buried well > within each algorithm implementation that offers such choices) > > Too vague/coarse semantics > -------------------------- > > Elaborating further from the examples above, there is also a conceptual > issue at the base of the OTC discussion that led to PR#14759, namely > that "blinding is a property of an algorithm: either an implementation > does blinding or it doesn't". > > But this is not entirely true: blinding is applied to individual > operations at various levels. > RSA is a special case that has the luxury of resting its security on > basically a single mathematical operation (modular exponentiation) over > a single mathematical layer of abstraction (finite field math provided > by BIGNUM): so it is enough to apply blinding to the modular > exponentiation to say "this algorithm implementations does blinding". > That is not true anymore already for DSA signing (that relies on the > same single finite field math abstraction provided by BIGNUM, but > already rests on two operation, exponentiation and modular inversion) > and one more layer deep with ECDH/ECDSA where you have two operations > (scalar multiplication and modular inversion), of which the first > is composed of math operations coming from the EC arithmetic layer of > abstraction, and each of those operation themselves are built as the > composition of many operations from the same FF arithmetic layer from > BIGNUM. > The number of levels of abstraction involved in an implementation keeps > rising steeply when we move into the realm of upcoming post-quantum > algorithms. > When you have multiple operations from multiple layers, applying binding > efficiently is non-trivial, one maybe blinds only the EC scalarmul but > not the FF modinv, because the first takes the bulk of the computation > time and common consensus at some point in time is that there is little > point in blinding the modinv if its SNR is deemed too low to be > exploitable. But what happens when this does not hold anymore? What is > the meaning of `blinding=yes` for an EC implementation that applied > blinding only to scalarmul and not to modinv because that was the best > practice at the time it was written? > What if blinding is applied to the scalarmul but the FF operation to > perform the blinding already leaks a small amount because BIGNUM? > A small leakage might not be considered a problem with 2048+ RSA > keys (shared opinion might be that 1 or 3 bits of knowledge wouldn't > give the attacker a noticeable boost compared to other tools in their > arsenal) but the same few bits have been proven sufficient to break even > P384 or P521 keys (which are above the security level of RSA 2048) with > limited effort and costs: what is the meaning of `blinding=yes` in the > case where the blinding implementation itself reveals information on the > secret it should protect? > > What if the blinding operation itself is not leaking a single bit by > itself, but it magnifies some effect from the underlying level of > abstraction (e.g., blinding scalarmul could make the nonce bigger than > the prime order subgroup size `n`, and the specific scalarmul > implementation must guarantee that its scalar input is less than `n` to > compute correctly, so it has a special branch to be robust and handle > bigger scalars coming from blinding, creating a magnifying lens for an > attacker when the branch is taken, as that says something on the > probability of the high order bits of the scalar being set)? > This might seem as having nothing to do with the semantics of > `blinding=yes`, but it does for providers like ours where we apply > blinding at top abstraction layer (and in the case of certain paths also > in strategic points in the second layer) but then have a number of > different sub-algorithm implementations that behave very differently: > depending on the curve name (and encoding format when parsing a key) you > might end up in the generic prime/binary curves implementation, or in > one of the specific ones like nistz256 or nistp521. > > Applying `blinding=yes` like this: > > ```c > {"ECDSA","provider=default,blinding=yes",ossl_ecdsa_signature_functions} > ``` > > is already making the semantics of the `blinding` property ill-defined, > and affecting its effectiveness/purpose as a decision mechanism to be > exposed to users: we have different codepaths under the same umbrella > flagged as applying blinding, but even within it that blinding has very > different shades of "yes". > > As @t8m said "Security is not a boolean value" and my point here is that > neither is `blinding` or `consttime` and similar: they come in shades > (even the `formally_verified` one: maybe the EC layer is formally > verified by tools, but the underlying FF module might come with > different guarantees, etc.) and the threshold by which SOTA deems the > quality to become "yes*" or "no*" changes over time (e.g., it was fine > to only blind scalarmul yesterday, so implementation X was considered as > a blinding implementation, but today also modinv blinding is required so > the same implementation X is now deemed non-blinding). > > Alternative > ----------- > > Support for the specific `RSA_FLAG_BLINDING`&co functionality could > instead just be dropped. > My understanding is that its usefulness is already limited to legacy > compatibility with engines which anyway would not play nicely with the > new provider/core architecture. > > More in general, as a mechanism to select secure implementations if > there are insecure ones present as well, the problem can already be > solved applying the "know your providers" mantra in combination with the > use of the `provider=default`, `provider=provX` properties to pick > across the offerings of multiple providers. > > This wouldn't cover the use case (is this even supported/tested right > now?) of a single provider registering the same algorithm twice with > different alternative implementations and properties: in this case > though it would be possible to use a property definition local to the > specific provider version, and its documentation would inform the users > of how to use propquery to select what and the related conditions and > caveats. Luckily this last case does not affect our implementations, as > we don't offer competing algorithms at the provider level. > > Although I wouldn't propose to do it, if we really really wanted to > retain the functionality of `RSA_FLAG_BLINDING` and have > algorithms exposing knobs with the capability of turning on/off (or > 1000/500/0) whatever degree of blinding they support, we could achieve > this with existing ctrls/params as we do for many other tunable knobs. > Using ctrls/params over doing this on a propquery level has the > advantage that the definition/semantics can be scoped with more > granularity than offered at the provider level. > As a user/sysadmin, if I have done the evaluation of the specific > context that could lead me to responsibly decide to pick no-blinding, > no-consttime, no-secure or similar hazardous choices, I would expect to > be anyway in a situation where I have so much control over my > system, software and environment that patching the code to pass a param > or call a ctrl function deeply inside my application shouldn't be a > blocker at all. > > Summary > ------- > > I am opposed to merging PR#14759 for several reasons: > > - it lends itself to insecure-by-default patterns, repeating mistakes > from the past > - the validity scope for similar properties is somewhat limited to a > single provider version, so its effectiveness against its purpose of > being a selection mechanism for users to pick among competing > algorithms offered by different providers is limited (and has the > potential to hunt us back in the future) > - its definition and semantics are vague, its applicability as an > algorithm property too coarse to be well-defined even inside our own > providers > - `secure`, `blinding`, `consttime`, etc. are not boolean properties of > algorithms (even if I really wish they could be) > - it's one more vague concept for everyone to understand, and one more > item with not-too-clear logic for the project to handle and maintain > for the foreseeable future > - we have already established alternatives in the form of > per-implementation params or ctrls to tune some security parameters > that can alter the execution flow of our implementations > > Also, from a more generic usability perspective and thinking of > supporting those in our Community developing 3rd party providers, I > think we should also > > - [in the vote] take a position on what are good/bad ideas when devising > provider-specific properties, and have some official guidelines in the > documentation about it by the time final 3.0.0 is release, and > - [not in the vote, could be a separate discussion] document what > mechanisms we plan to put in place to coordinate efforts > which will allow for some carefully evaluated properties to have > well-defined semantics (with versioning?) across providers/vendors > (some form of registry of properties, with well-defined criteria for > maintaining it?) > > Proposed vote text > ================== > > Do not merge PR#14759, prevent declaring properties similar to > `blinding=yes` or `consttime=yes` in our implementations and > discourage 3rd parties from adopting similar designs. > > > > > Please provide feedback on the vote text, I would like to open the vote > on Monday and have some public debate about its points via the list > rather then compress it on the OTC meeting which already has enough > items in agenda! > > Thanks, > > Nicola