Re: OSSL_PARAM behaviour for unknown keys
On Tue, Dec 15, 2020 at 5:46 PM Richard Levitte wrote: > Of course, checking the gettable and settable tables beforehand works > as well. They were originally never meant to be mandatory, but I > guess we're moving that way... > The only one who knows whether or not a given parameter is critically important to have been used is the application. The gettable and settable interfaces provide the ability to check that. For forward and backward compatibility it makes no sense to wire in a requirement for complete knowledge of everything that is provided. You need to be able to provide extra optional parameters that some implementations want to consume and are entirely irrelevant to other implementations to have extensibility wired into the APIs and that was one of the purposes of the new plumbing - to be able to handle things going forward. If you change things at this late stage to basically say everything has to know everything then we lose that ability. In practical terms too, we need to be able to have later releases of applications able to work with earlier releases of providers (specifically but not limited to the FIPS provider) and it would practically mean you could never reach that interchangeable provider context that is there for a stable ABI - wiring in a requirement to be aware of all parameters will simply lead to provider implementations needing to ignore things to achieve the necessary outcome. If you want to know if a specific implementation is aware of something, the interface is already there. In short - I don't see an issue as there is a way to check, and the interface is designed for forward and backward compatibility and that is more important than the various items raised here so far IMHO. Tim
Re: OSSL_PARAM behaviour for unknown keys
Whatever we decide on this, I would rather not burden provider authors with having to check for all sorts of things they aren't interested in. I've often had the fictitious algorithm BLARGH (someone should invent it, just 'cause), and while everyone with access to specs could write a provider, some might extend it as well, with extra parameters (like the CRT params for RSA keys) or flags or whatnot. If we burden the providers with the sort of checks that are discussed here, it would require every BLARGH implementation to be constantly in sync with every other BLARGH implementation. That's not a very good idea (*) So, I'm thinking that control should remain with the application / libcrypto. However, I'll also maintain that, as a matter of protocol, we can ask the providers to set the return_size field for any parameter they use, as a way to help out. That would enable the use of functions like OSSL_PARAM_modified() on the application / libcrypto side when setting parameters, just at it currently does for getting them. From a provider author point of view, I'd say that's a much lesser burden than having to have knowledge of all sorts of params any other provider might support. Of course, checking the gettable and settable tables beforehand works as well. They were originally never meant to be mandatory, but I guess we're moving that way... Cheers, Richard On Mon, 14 Dec 2020 23:20:38 +0100, David Benjamin wrote: > > I'm not very familiar with the new providers system, but I would discourage > introducing new > special return values. In my experience, callers don't do a good job of > handling this sort of > thing. The more APIs diverge from a straightforward success/failure return, > the more error-prone > they are. So a 1 vs 2 return value for "success" vs "success, but..." seems > likely to confuse > things. > > It also seems safer for unexpected parameters to be an error. While sometimes > they can be ignored, > sometimes they cannot. For example, looking at the RSA provider interface, > suppose a caller passed > in rsa-factor3, rsa-factor4, etc. A provider that didn't implement > multi-prime keys, or supported > a different number of coefficients would not notice at the parameter level. > The object wouldn't be > in a consistent state. (Relatedly, I think this example is another reason > that providers should > validate inputs on key import. See > https://github.com/openssl/openssl/issues/13615.) > https://www.openssl.org/docs/manmaster/man7/EVP_PKEY-RSA.html > > Or consider a caller that thought they were configuring a private key, but > got the parameter name > wrong. That would likely result in a public key. While self-consistent, it's > the wrong type of > object, compared to what the caller was expecting, and may result in strange > errors further down > the program flow. > > In the other direction, the DRBG example does not seem very compelling to me. > At the point the > application picks the broad family of algorithm, it should also pick the > parameters to instantiate > the actual algorithm. They're typically a unit. The convenience of passing > arguments that won't be > used seems not especially valuable, especially compared against the safety > and correctness cost in > silently misinterpreting the caller's request. An RSA provider which does not > implement CRT is a > little more plausible, but I think optional parameters are more the exception > rather than the > rule. RSA-CRT is well-established and standard (RFC8017, Appendix A.1.2), so > that provider can > simply know to ignore CRT parameters, possibly still validating them. (If > less well-established, > the caller may need to query capabilities anyway, in which case it'd know the > provider implements > a smaller interface. Though see below about how likely this is.) > > We can also look to programming languages. While languages sometimes do drop > unused and undeclared > parameters (e.g. Python **kwargs), that's usually not the default story. > > Finally, these are cryptographic primitives, not a general-purpose plugin > system. Cryptographic > primitives aren't introduced frequently. They especially aren't extended > frequently, and typically > have well-defined serializations and structures. They're also > security-sensitive. That suggests > leaning towards safety and structure rather than ad-hoc extensibility. > > David > > On Mon, Dec 14, 2020 at 4:10 PM Kurt Roeckx wrote: > > On Mon, Dec 14, 2020 at 08:20:29PM +0100, Dmitry Belyavsky wrote: > > Dear Kurt, > > > > > > On Mon, Dec 14, 2020 at 3:59 PM Kurt Roeckx wrote: > > > > > Hi, > > > > > > doc/man3/OSSL_PARAM.pod current says: > > > Keys that a I or I doesn't recognise should > > > simply be ignored. That in itself isn't an error. > > > > > > The intention of that seems to be that you just pass all the data > > > you have, and that it takes data it needs. So you can pass it
Re: OSSL_PARAM behaviour for unknown keys
Dear Kurt, On Mon, Dec 14, 2020 at 10:10 PM Kurt Roeckx wrote: > On Mon, Dec 14, 2020 at 08:20:29PM +0100, Dmitry Belyavsky wrote: > > Dear Kurt, > > > > > > On Mon, Dec 14, 2020 at 3:59 PM Kurt Roeckx wrote: > > > > > Hi, > > > > > > doc/man3/OSSL_PARAM.pod current says: > > > Keys that a I or I doesn't recognise should > > > simply be ignored. That in itself isn't an error. > > > > > > The intention of that seems to be that you just pass all the data > > > you have, and that it takes data it needs. So you can pass it data > > > that it doesn't need because it's only used in case some other > parameter > > > has some specific value. For example, depending on the DRBG mode > > > (HMAC, CTR, HASH) you have different parameters, and you can just > > > pass all the parameters for all the modes. > > > > > > I think for behaviour for a setter is not something that we want, > > > it makes it complicated for applications to check that it will > > > behave properly. I think that in general, if the applications > > > wants to set something and you don't understand it, you should > > > return an error. This is about future proofing the API. For > > > instance, a new version supports a new mode to work in and that > > > needs a new parameter. If it's build against a version that knows > > > about it, but then runs against a version that doesn't know about > > > it, everything will appear to work, but be broken. If we return > > > an error, it will be clear that it's not supported. > > > > > > An alternative method of working is that the application first > > > needs to query that it's supported. And only if it's supported > > > it should call the function. But we don't have an API to query for > > > that. You might be able to ask for which keys you can set, but it > > > doesn't cover which values you can set. I hope we at least return > > > an error for a known key with an unknown value. But it's my > > > understanding that we currently don't always return all supported > > > keys, and that the supported keys can depend on one of the set > > > parameters. > > > > > > I suggest that we change the return value to indicate that all > > > parameters have been used or not. For instance return 1 in case > > > all used, return 2 in case not all used. > > > > > > > > From my GOST implementor's experience, the provider can get a lot of > > parameters. > > Some of them are supported, some of them are not. > > > > The particular provider is the only subsystem that knows which parameters > > are supported and which are necessary for the operations. > > > > So the caller can provide some unsupported parameters, some supported and > > some totally wrong for the provider. > > These are the cases that must be distinguishable on the caller side. > > If I understand you correctly, what you're saying is that it's > sometimes ok to ignore some parameters. For instance, if you try > to create an RSA object, and you pass it CRT parameters, and the > implementation doesn't do anything with them, it can ignore them > if it wants to. > > I would say that the provider should know what those parameters > mean, so that it's not an "unknown key", it just ignores them, > at which points it can say that it understands all the parameters. > > Some might argue that they don't want to use something that > doesn't make use of the CRT parameters, but then they probably > shouldn't be using that provider to begin with. > > > After that the provided EVP object should be either in a consistent state > > or not, assuming the upcoming operation. > > The object should always be in a consistent state. I would prefer > that in case of failure the object is not created (or modified). > Which brings us to some other open points about the API we have. We > should not introduce new APIs where you can modify the state of the > object, so it can not be in a non-consistent state. It's much more > simple to get things correct in that case. But as long as we have > to support old APIs where it can be modified, the prefered > consistent state is to not mofify the object on error. Some APIs make > this very hard, so the other acceptable state is that you can free > the object. With an API that doesn't allow modification, either > you get a complete object, or you get no object. > > I hope I've got a specific point of our disagreement. There are 2 variants of using OpenSSL. 1. Algorithm-agnostic. We can deal with most of the algorithms in a more or less similar way. That was the way we dealt with various algorithms in libcrypto since 1.0 version. 2. Algorithm-specific. The API user should take into account which algorithms are supported by their application and provide some specific processing. These are two different approaches. The OpenSSL itself should be more or less algorithm-agnostic. The providers (as engines before) are definitely algorithm-specific. The openssl command line utilities in fact provide flexibility leaving the burden of parameters setup to
Re: OSSL_PARAM behaviour for unknown keys
I'm not very familiar with the new providers system, but I would discourage introducing new special return values. In my experience, callers don't do a good job of handling this sort of thing. The more APIs diverge from a straightforward success/failure return, the more error-prone they are. So a 1 vs 2 return value for "success" vs "success, but..." seems likely to confuse things. It also seems safer for unexpected parameters to be an error. While sometimes they can be ignored, sometimes they cannot. For example, looking at the RSA provider interface, suppose a caller passed in rsa-factor3, rsa-factor4, etc. A provider that didn't implement multi-prime keys, or supported a different number of coefficients would not notice at the parameter level. The object wouldn't be in a consistent state. (Relatedly, I think this example is another reason that providers should validate inputs on key import. See https://github.com/openssl/openssl/issues/13615.) https://www.openssl.org/docs/manmaster/man7/EVP_PKEY-RSA.html Or consider a caller that thought they were configuring a private key, but got the parameter name wrong. That would likely result in a public key. While self-consistent, it's the wrong type of object, compared to what the caller was expecting, and may result in strange errors further down the program flow. In the other direction, the DRBG example does not seem very compelling to me. At the point the application picks the broad family of algorithm, it should also pick the parameters to instantiate the actual algorithm. They're typically a unit. The convenience of passing arguments that won't be used seems not especially valuable, especially compared against the safety and correctness cost in silently misinterpreting the caller's request. An RSA provider which does not implement CRT is a little more plausible, but I think optional parameters are more the exception rather than the rule. RSA-CRT is well-established and standard (RFC8017, Appendix A.1.2), so that provider can simply know to ignore CRT parameters, possibly still validating them. (If less well-established, the caller may need to query capabilities anyway, in which case it'd know the provider implements a smaller interface. Though see below about how likely this is.) We can also look to programming languages. While languages sometimes do drop unused and undeclared parameters (e.g. Python **kwargs), that's usually not the default story. Finally, these are cryptographic primitives, not a general-purpose plugin system. Cryptographic primitives aren't introduced frequently. They especially aren't extended frequently, and typically have well-defined serializations and structures. They're also security-sensitive. That suggests leaning towards safety and structure rather than ad-hoc extensibility. David On Mon, Dec 14, 2020 at 4:10 PM Kurt Roeckx wrote: > On Mon, Dec 14, 2020 at 08:20:29PM +0100, Dmitry Belyavsky wrote: > > Dear Kurt, > > > > > > On Mon, Dec 14, 2020 at 3:59 PM Kurt Roeckx wrote: > > > > > Hi, > > > > > > doc/man3/OSSL_PARAM.pod current says: > > > Keys that a I or I doesn't recognise should > > > simply be ignored. That in itself isn't an error. > > > > > > The intention of that seems to be that you just pass all the data > > > you have, and that it takes data it needs. So you can pass it data > > > that it doesn't need because it's only used in case some other > parameter > > > has some specific value. For example, depending on the DRBG mode > > > (HMAC, CTR, HASH) you have different parameters, and you can just > > > pass all the parameters for all the modes. > > > > > > I think for behaviour for a setter is not something that we want, > > > it makes it complicated for applications to check that it will > > > behave properly. I think that in general, if the applications > > > wants to set something and you don't understand it, you should > > > return an error. This is about future proofing the API. For > > > instance, a new version supports a new mode to work in and that > > > needs a new parameter. If it's build against a version that knows > > > about it, but then runs against a version that doesn't know about > > > it, everything will appear to work, but be broken. If we return > > > an error, it will be clear that it's not supported. > > > > > > An alternative method of working is that the application first > > > needs to query that it's supported. And only if it's supported > > > it should call the function. But we don't have an API to query for > > > that. You might be able to ask for which keys you can set, but it > > > doesn't cover which values you can set. I hope we at least return > > > an error for a known key with an unknown value. But it's my > > > understanding that we currently don't always return all supported > > > keys, and that the supported keys can depend on one of the set > > > parameters. > > > > > > I suggest that we change the return value to indicate that all > > > parameters have been used or
Re: OSSL_PARAM behaviour for unknown keys
On Mon, Dec 14, 2020 at 08:20:29PM +0100, Dmitry Belyavsky wrote: > Dear Kurt, > > > On Mon, Dec 14, 2020 at 3:59 PM Kurt Roeckx wrote: > > > Hi, > > > > doc/man3/OSSL_PARAM.pod current says: > > Keys that a I or I doesn't recognise should > > simply be ignored. That in itself isn't an error. > > > > The intention of that seems to be that you just pass all the data > > you have, and that it takes data it needs. So you can pass it data > > that it doesn't need because it's only used in case some other parameter > > has some specific value. For example, depending on the DRBG mode > > (HMAC, CTR, HASH) you have different parameters, and you can just > > pass all the parameters for all the modes. > > > > I think for behaviour for a setter is not something that we want, > > it makes it complicated for applications to check that it will > > behave properly. I think that in general, if the applications > > wants to set something and you don't understand it, you should > > return an error. This is about future proofing the API. For > > instance, a new version supports a new mode to work in and that > > needs a new parameter. If it's build against a version that knows > > about it, but then runs against a version that doesn't know about > > it, everything will appear to work, but be broken. If we return > > an error, it will be clear that it's not supported. > > > > An alternative method of working is that the application first > > needs to query that it's supported. And only if it's supported > > it should call the function. But we don't have an API to query for > > that. You might be able to ask for which keys you can set, but it > > doesn't cover which values you can set. I hope we at least return > > an error for a known key with an unknown value. But it's my > > understanding that we currently don't always return all supported > > keys, and that the supported keys can depend on one of the set > > parameters. > > > > I suggest that we change the return value to indicate that all > > parameters have been used or not. For instance return 1 in case > > all used, return 2 in case not all used. > > > > > From my GOST implementor's experience, the provider can get a lot of > parameters. > Some of them are supported, some of them are not. > > The particular provider is the only subsystem that knows which parameters > are supported and which are necessary for the operations. > > So the caller can provide some unsupported parameters, some supported and > some totally wrong for the provider. > These are the cases that must be distinguishable on the caller side. If I understand you correctly, what you're saying is that it's sometimes ok to ignore some parameters. For instance, if you try to create an RSA object, and you pass it CRT parameters, and the implementation doesn't do anything with them, it can ignore them if it wants to. I would say that the provider should know what those parameters mean, so that it's not an "unknown key", it just ignores them, at which points it can say that it understands all the parameters. Some might argue that they don't want to use something that doesn't make use of the CRT parameters, but then they probably shouldn't be using that provider to begin with. > After that the provided EVP object should be either in a consistent state > or not, assuming the upcoming operation. The object should always be in a consistent state. I would prefer that in case of failure the object is not created (or modified). Which brings us to some other open points about the API we have. We should not introduce new APIs where you can modify the state of the object, so it can not be in a non-consistent state. It's much more simple to get things correct in that case. But as long as we have to support old APIs where it can be modified, the prefered consistent state is to not mofify the object on error. Some APIs make this very hard, so the other acceptable state is that you can free the object. With an API that doesn't allow modification, either you get a complete object, or you get no object. Kurt
Re: OSSL_PARAM behaviour for unknown keys
Dear Kurt, On Mon, Dec 14, 2020 at 3:59 PM Kurt Roeckx wrote: > Hi, > > doc/man3/OSSL_PARAM.pod current says: > Keys that a I or I doesn't recognise should > simply be ignored. That in itself isn't an error. > > The intention of that seems to be that you just pass all the data > you have, and that it takes data it needs. So you can pass it data > that it doesn't need because it's only used in case some other parameter > has some specific value. For example, depending on the DRBG mode > (HMAC, CTR, HASH) you have different parameters, and you can just > pass all the parameters for all the modes. > > I think for behaviour for a setter is not something that we want, > it makes it complicated for applications to check that it will > behave properly. I think that in general, if the applications > wants to set something and you don't understand it, you should > return an error. This is about future proofing the API. For > instance, a new version supports a new mode to work in and that > needs a new parameter. If it's build against a version that knows > about it, but then runs against a version that doesn't know about > it, everything will appear to work, but be broken. If we return > an error, it will be clear that it's not supported. > > An alternative method of working is that the application first > needs to query that it's supported. And only if it's supported > it should call the function. But we don't have an API to query for > that. You might be able to ask for which keys you can set, but it > doesn't cover which values you can set. I hope we at least return > an error for a known key with an unknown value. But it's my > understanding that we currently don't always return all supported > keys, and that the supported keys can depend on one of the set > parameters. > > I suggest that we change the return value to indicate that all > parameters have been used or not. For instance return 1 in case > all used, return 2 in case not all used. > > >From my GOST implementor's experience, the provider can get a lot of parameters. Some of them are supported, some of them are not. The particular provider is the only subsystem that knows which parameters are supported and which are necessary for the operations. So the caller can provide some unsupported parameters, some supported and some totally wrong for the provider. These are the cases that must be distinguishable on the caller side. After that the provided EVP object should be either in a consistent state or not, assuming the upcoming operation. And the possibility to find out whether the state is consistent and suitable for the upcoming operation or not is a must and should be provided by an API. -- SY, Dmitry Belyavsky
OSSL_PARAM behaviour for unknown keys
Hi, doc/man3/OSSL_PARAM.pod current says: Keys that a I or I doesn't recognise should simply be ignored. That in itself isn't an error. The intention of that seems to be that you just pass all the data you have, and that it takes data it needs. So you can pass it data that it doesn't need because it's only used in case some other parameter has some specific value. For example, depending on the DRBG mode (HMAC, CTR, HASH) you have different parameters, and you can just pass all the parameters for all the modes. I think for behaviour for a setter is not something that we want, it makes it complicated for applications to check that it will behave properly. I think that in general, if the applications wants to set something and you don't understand it, you should return an error. This is about future proofing the API. For instance, a new version supports a new mode to work in and that needs a new parameter. If it's build against a version that knows about it, but then runs against a version that doesn't know about it, everything will appear to work, but be broken. If we return an error, it will be clear that it's not supported. An alternative method of working is that the application first needs to query that it's supported. And only if it's supported it should call the function. But we don't have an API to query for that. You might be able to ask for which keys you can set, but it doesn't cover which values you can set. I hope we at least return an error for a known key with an unknown value. But it's my understanding that we currently don't always return all supported keys, and that the supported keys can depend on one of the set parameters. I suggest that we change the return value to indicate that all parameters have been used or not. For instance return 1 in case all used, return 2 in case not all used. Kurt
