Should FIPS_mode() and FIPS_mode_set() be updated, deprecated, or completely removed..

[Tomas Mraz]

On Wed, 2020-03-04 at 13:47 +1000, SHANE LONTIS wrote:
> FIPS_mode() and FIPS_mode_set() are functions that were used by the
> old FOM.
> 
> In OpenSSL_111 they were stripped down to do almost nothing
> i.e:-
> 
> int FIPS_mode(void)      
>  {      
>      /* This version of the library does not support FIPS mode. */   
>   
>      return 0;      
>  }
> 
>  int FIPS_mode_set(int on)      
>  {
>  if (r == 0)      
>     return 1;    
> 
>  CRYPTOerr(0, CRYPTO_R_FIPS_MODE_NOT_SUPPORTED); 
>  return 0;      
> }
> 
> The original plan for these API’s is listed in the design document
> for 3.0
> i.e- the set would - set the global property and then do a fetch of a
> particular algorithm (this is problematic in itself since the
> algorithm may not exist for a 3rd party fips provider which could
> just implement a single algorithm).
> And FIPS_mode() would just return true if the global property for
> fips was set.
> 
> This got some pushback and after discussion with a few other otc
> members - it was decided that the functions should be deprecated
> since it would be confusing to a user because there are multiple
> library contexts allowed each with their own fips property that can
> be changed at
> any time.
> 
> This is done in https://github.com/openssl/openssl/pull/11075 and
> there is a related discussion in the comments.
> 
> This PR has also been rejected the deprecation and discusses
> - FIPS_mode_set() function could be completely removed.
> - FIPS_mode() - query using the default library context OR completely
> remove.
> 
> I have no issue with both functions being deleted as they no longer
> really mean the same thing as they did before.
> Each library context has its own default properties - so querying
> FIPS_mode() could only return what the default library context’s fips
> properties are - it doesnt mean every library context is in fips
> mode, or even that the fips module is loaded. 
> If the functions are removed then it may require a OMC vote since
> this could be viewed as a breaking change..

I believe that making these function return an error unconditionally is
a breaking change as well and for that reason I prefer removing them
completely or actually removing the FIPS_mode_set() and making the
FIPS_mode() to query the fips properties of the default context.

As for the use-cases the FIPS_mode() call has in the applications I
know of this implementation would be completely OK. If the call is
removed these applications would call the
EVP_default_properties_is_fips_enabled on the default context anyway.
The applications do not call FIPS_mode() to query for the FIPS
compliance but they use it to change their behavior i.e. prefer or
select different algorithms to be used.

The current implementation in the PR - unconditionally returning error
- is completely useless. It would make some (not much) sense if we
aimed for ABI compatibility with 3.0 however that is definitely not the
case so the only reasonable thing is to either try to emulate the
behavior as much as possible or remove completely so the users of the
API would know immediately that they have to be changed.

-- 
Tomáš Mráz
No matter how far down the wrong road you've gone, turn back.
                                              Turkish proverb
[You'll know whether the road is wrong if you carefully listen to your
conscience.]