opencryptoki / libica

Crypto library for s390x.
Other
9 stars 17 forks source link

Fips140-3 compliance #89

Closed jschmidb closed 2 years ago

jschmidb commented 2 years ago

This pull request provides patches to make libica FIPS 140-3 compliant. It upgrades libica to new minor level 4.1.0.

jschmidb commented 2 years ago

Just pushed the fixes related to your comments so far:

jschmidb commented 2 years ago

Just saw that the Travis build fails with the new updates, because the eddsa_test and x_test fail. This is not the case on my test system, will check ...

ifranzki commented 2 years ago

@ifranzki Refer to new commit "Restrict EC/ED/X curves in fips mode" for API restrictions.

This looks good.

However, I still wonder if you would not also need some restrictions for EC curves: First, restrict the curves that are allowed with FIPS (if not already done). Second: You run the EC selftest using CPACF only. Is it OK for FIPS to perform the API calls for EC operations with another code paths than the one that was self-tested? I.e. wouldn't you also have to use CPACF for all EC operations when in fips mode? If you allow to perform EC operations with the card and/or SW, then it runs code paths that have not been self tested. I don't know the FIPS requirements good enough to answer that.....

jschmidb commented 2 years ago

However, I still wonder if you would not also need some restrictions for EC curves: First, restrict the curves that are allowed with FIPS (if not already done).

Yes, the idea here was to check with curve_supported_via_openssl(nid) to filter out curves that are not supported by openssl (in fips mode)

Second: You run the EC selftest using CPACF only. Is it OK for FIPS to perform the API calls for EC operations with another code paths than the one that was self-tested? I.e. wouldn't you also have to use CPACF for all EC operations when in fips mode? If you allow to perform EC operations with the card and/or SW, then it runs code paths that have not been self tested. I don't know the FIPS requirements good enough to answer that.....

Yes, the idea here was to check with curve_supported_via_cpacf(nid) to only allow the CPACF path, which is self-tested.

The full check for fips mode is:

#ifdef ICA_FIPS
    if (fips >> 1)
        return EACCES;
    if (!curve_supported_via_openssl(privkey->nid) ||
        !curve_supported_via_cpacf(privkey->nid))
        return EPERM;
#endif /* ICA_FIPS */

But when thinking about this, I'm realizing that for ED/X this check works, but could be simpler: as of now we restrict ED/X in fips mode, simply because they are not approved by NIST, not because openssl does not support them in fips mode.

So the routine:

static inline int check_fips(unsigned int nid)
{
#ifdef ICA_FIPS
    if (fips >> 1 || !curve_supported_via_openssl(nid) ||
        !curve_supported_via_cpacf(nid))
        return 1;
    else
        return 0;
#else
    return 0;
#endif
}

could be just like:

#ifdef ICA_FIPS
    return 1;    <- even independent of the global 'fips' variable
#else
    return 0;
#endif
}
ifranzki commented 2 years ago

Yes, the idea here was to check with curve_supported_via_cpacf(nid) to only allow the CPACF path, which is self-tested.

OK, makes some sense, but for example in ica_ecdsa_sign() I see that you added:

if (!curve_supported_via_openssl(privkey->nid) ||
        !curve_supported_via_cpacf(privkey->nid))
        return EPERM;

So you reject all curves not supported by OpenSSL nor by CPACF. So far so good.

But with that, the operation could still be performed by the card (e.g. if icapath = 1, and ica_offload_enabled = 1, and the curve is also supported by the card), no ?

I guess you somehow need to force it to use CPACF (or maybe OpenSSL if we assume the OpenSSL is FIPS tested as well), but never the card.

jschmidb commented 2 years ago

Yes, you are right. I think I see the gap: if we are e.g. running on a pre-z15 machine with a CCA card, the icapath is indeed 1 (because sw callbacks are disabled), but inside ecdsa_sign_hw() the CEX path would be taken, because msa9 is not available. Or, as you say, if ica_offloads are enforced. I'm adding a fix to prevent ecdsa_sign_hw/verify_hw from going through CEX adapters.

jschmidb commented 2 years ago

I removed the "libica 4.1.0" version patch and will now merge. Next steps are merging Holger's pr and providing a new patch for EC pairwise key / public key checking as required by SP800-56Arev3. The version patch then comes on top again when there are no more remaining questions about FIPS 140-3.