dotnet / runtime

.NET is a cross-platform runtime for cloud, mobile, desktop, and IoT apps.
https://docs.microsoft.com/dotnet/core/
MIT License
14.96k stars 4.65k forks source link

OpenSSL 3.x deprecated RC2 ciphers #106248

Open sla89 opened 1 month ago

sla89 commented 1 month ago

Hello everyone,

We want to use the dotnet runtime in a very secure Linux environment with OpenSSL 3.2.1 without any legacy algorithms.

Our .NET C# application needs some cryptographic algorithms, so the dotnet runtime tries to load legacy ciphers like RC2 because they are marked as required, see here

Do you think there is a way to make RC2 not mandatory anymore for such use cases? I know that they are part of the dotnet environment..

To fix the issue at the moment we have built OpenSSL with RC2 enabled but patched the functions in a way that we can ensure they are not used (you just never know if any 3rd party component might be calling these ciphers).

Thanks and best regards, Stefan

dotnet-policy-service[bot] commented 1 month ago

Tagging subscribers to this area: @dotnet/area-system-security, @bartonjs, @vcsjones See info in area-owners.md if you want to be subscribed.

krwq commented 1 month ago

This sounds like a reasonable request, we can as well throw PlatformNotSupportedException when it's not available - I'm not aware of anything other than RC2 class itself using it by default.

@bartonjs @vcsjones?

@sla89 would you be interested in contributing? (you'll probably need to adjust tests a bit)

vcsjones commented 1 month ago

If the request is "fail gracefully when a function is not available", perhaps.

The interesting question is how much should we do to support OpenSSL builds that have stripped out non-required functionality. One could also apply this logic to building OpenSSL with no-des, no-md5, no-rc2, no-dsa etc to disable "legacy" algorithms.

What about no-hmac if someone feels they don't need HMAC for whatever reason? Or something else that they just decided "meh, don't need it".

Clearly there is a bigger philosophical question here, and I am not sure the answer is entirely straight forward. On one hand, sure, we can support missing algorithms as long as the runtime itself does not need the algorithm. On the other hand, requiring algorithms gives a more consistent behavior to .NET and offers a better cross platform experience.

sla89 commented 1 month ago

I would create a PR if we can agree on a common behaviour.

I am not sure how much I can contribute to make a decision here but I agree with @vcsjones that you could even disable other algorithms of OpenSSL and then the same question comes up for those.

Perhaps the behaviour should simply be more tolerant of legacy providers in OpenSSL which currently include MD2, MD4, MDC2, RMD160, CAST5, BF (Blowfish), IDEA, SEED, RC2, RC4, RC5 and DES (but not 3DES).

krwq commented 1 month ago

If the request is "fail gracefully when a function is not available", perhaps.

The interesting question is how much should we do to support OpenSSL builds that have stripped out non-required functionality. One could also apply this logic to building OpenSSL with no-des, no-md5, no-rc2, no-dsa etc to disable "legacy" algorithms.

What about no-hmac if someone feels they don't need HMAC for whatever reason? Or something else that they just decided "meh, don't need it".

Clearly there is a bigger philosophical question here, and I am not sure the answer is entirely straight forward. On one hand, sure, we can support missing algorithms as long as the runtime itself does not need the algorithm. On the other hand, requiring algorithms gives a more consistent behavior to .NET and offers a better cross platform experience.

I think we should strive to gracefully fail. It doesn't seem to make sense to throw an exception when user doesn't use RC2 and RC2 is not available. I think we should start with best effort and allow users to fix errors specific to their configuration and incrementally flush this out - I also don't want us to spend unreasonable amount of time to support/test every possible configuration - as long as changes are relatively small and non-invasive and they don't break existing builds I'm ok with patches fixing issues like this (especially it's reasonable request to be able block old legacy algorithms).

@bartonjs any thoughts?

bartonjs commented 1 month ago

It doesn't seem to make sense to throw an exception when user doesn't use RC2 and RC2 is not available.

If we were built directly against OpenSSL ("distro-specific", vs our "portable" delay-loaded), but we end up on a system where an algorithm is removed, then the ld-library-loader will reject the load of our .so due to a missing symbol, and our portable bind is basically doing the same thing.

We could take the stance that all of the algorithms should be treated as optional, and understand that we can return NULL from our shim calls to get them, as something like

const EVP_CIPHER* CryptoNative_EvpRC2Ecb(void)
{
    // No error queue impact.
#if FEATURE_DISTRO_AGNOSTIC_SSL
    if (API_EXISTS(EVP_rc2_ecb))
    {
        return EVP_rc2_ecb();
    }

    return NULL;
#elif OPENSSL_NO_RC2
    return NULL;
#else
    return EVP_rc2_ecb();
#endif
}

which maybe someone can macro-ize nicely so it's just

const EVP_CIPHER* CryptoNative_EvpRC2Ecb(void)
{
    // No error queue impact.
    return CHECK_ALGORITHM(EVP_rc2_ecb, OPENSSL_NO_RC2);
}

and then update our flows to understand that algorithms are optional.

But, it's not a great experience for .NET to have systems in this configuration. RC2 is used in a lot of existing PFXes (just today I was reminded that Windows Server 2016 doesn't support AES, so it makes PFXes with certs encrypted with RC2-40 and keys with 3DES)... it not being present on Android was a pain for our cert/PFX test suite.

All algorithms that existed in .NET Framework (RC2, DES, 3DES, DSA, RSA, ECDSA, ECDH) are assumed to always be present (granted, for RC2, it's already not "always", but Android is a minority)... there are no IsSupported properties for any of these algorithms.

My personal preference is to say that all existing algorithms are "required". If we think we're going to reasonably enter an era where algorithms start disappearing, then the moment we move one of them to optional we should prepare for doing it for all of them, and add IsSupported properties to all of our types.

Needless to say, if we're doing anything here, it's too late for .NET 9.

bartonjs commented 1 month ago

I'm not aware of anything other than RC2 class itself using it by default.

A lot of our PFX tests do. Though we already have test configuration for that, because Android.

vcsjones commented 1 month ago

I do think it is reasonable to consider handling disappearing algorithms.

and add IsSupported properties to all of our types.

We've previously rejected that for a "legacy" algorithm, DSA, here https://github.com/dotnet/runtime/issues/53017#issuecomment-845592794

I think that line of thinking is still reasonable. Particularly,

I do think that the IsSupported is only useful for "this is too new, it's not available yet" (you can't have your first choice). When the algorithm has been forced on you there's almost no benefit in checking it.

bartonjs commented 1 month ago

We've previously rejected that for a "legacy" algorithm, DSA, here https://github.com/dotnet/runtime/issues/53017#issuecomment-845592794

Hm. That guy seemed to know what he was talking about, perhaps I should listen to him :smirk:.

You're (I'm?) right... IsSupported is mainly for choosing the original algorithm (presumably "encryption"), with a pairing like

private static string ChooseAlgorithm()
{
    return
        Aes2.IsSupported ? "AES2" :
        AesCcm.IsSupported ? "AES-CCM" :
        "AES-CBC+HMAC";
}

private static ConfiguredAlgorithm OpenKey(string algorithm, Opaque kdfStuff)
{
     return algorithm switch
     {
        "AES-CBC+HMAC" => AesCbcHmac(kdfStuff),
        "AES-CCM" => AesCcm(kdfStuff),
        "AES2" => Aes2(stuff),
        "3DES-CBC" => UnauthenticatedDes3Ede(kdfStuff),
        "RC2-ECB" => CaesarPlusPlus(kdfStuff),
        _ => throw new WhatchooTalkinBoutWillisException(),
      };
}

So it's probably not useful to retrofit IsSupported onto anything that no one would have as a positive choice.

DES could probably disappear without anyone noticing (like DSA basically did). 3DES would be harder. Probably no one really wants RC2, unless they have a PFX that uses it... which goes back to why it's generally bad for platforms to fully remove legacy algorithms (sometimes you just have old stuff lying around that you want to recover).

It's still not great for the platform to "encourage" algorithms to disappear... but I guess past me has helped convince now-me (future me?) that it's not the end of the world here.

vcsjones commented 1 month ago

That guy seemed to know what he was talking about

Quoting that guy remains the most effective tool I have to convince you of something.

It's still not great for the platform to "encourage" algorithms to disappear

I do think a good "line" here, that was previously called out, are things in the legacy OSSL 3 provider, which is basically RC2 and DES. OpenSSL has already given a soft signal things should work without these algorithms.