Bug 1938011

Thanks for a bug report. The upstream evo-ews bug [1] says the libsoup change landed for 2.65.1+. As the latest libsoup release in RHEL 8 is libsoup-2.62.3-2.el8, it does not contain the change. I can try to backport it, though I'm afraid there were done more follow up changes in that code.

[1] https://gitlab.gnome.org/GNOME/evolution-ews/-/issues/38

The crypto policies have an impact on this for sure. They can disable algs which the server can work with, then the connection is rejected due to no matching alg/hash function.

Could you run Evolution from a terminal on an affected machine like this:

   $ EWS_DEBUG=2 evolution >&log.txt

with an enabled EWS account, please? It logs raw communication, thus it can contain private information, thus some headers should be cut/garbled, especially the "Authorization:" header, which can contain encoded private information. I'm not interested in a whole communication log, I'd like to see the error response the server returned.

There can be also tried to connect to the server using gnutls-cli tool, it can be run like (using outlook.office365.com as an example server):

   $ gnutls-cli outlook.office365.com:443

which may fail to connect to the server when it cannot reach the server with more strict crypto policies.

Finally, I suppose curl can use ntlm version 2 as well, thus running:

   $ curl -v -k --ntlm --user USER:PASSWORD -X GET https://outlook.office365.com/EWS/Services.wsdl

then it may download a file, which requires an authorization (there is "Authorization: NTLM ...." in the log it prints).

I opened bug #1971533 against glib2, to be able to compute MD5 also in the FIPS mode (or any crypto-policies mode, which disables MD5 for the GnuTLS).

The libsoup fix works without it too, but only if MD5 is enabled in the crypto-policies.

Andrew, could you confirm it, please? I'm unsure from your text whether the application only did not crash, or whether you also properly connected to the server using NTLMv2.

Hi Simo, can you look at this bug please?

(In reply to Milan Crha from comment #11)
> I opened bug #1971533 against glib2, to be able to compute MD5 also in the
> FIPS mode (or any crypto-policies mode, which disables MD5 for the GnuTLS).

That sort of defeats the point of having crypto-policies, though. I'm far from an expert here, but if MD5 is disabled in crypto-policies, and NTLMv2 requires MD5-based HMAC, well then it should fail... right? (Well, of course glib should not segfault, but we'll handle that in bug #1971533.)

I think it's expected that enabling FIPS mode breaks applications that require non-FIPS cryptography.

Simo, do you agree?

Correct.
NTLMv2 is generally not approved in FIPS mode (uses MD5, RC4 and even DES in some cases) and should not be used in FIPS mode.

People can create a custom policy if they need to tweak their choices of algorithms,
but in FIPS mode the blocking is often done at the library level and is not influenced by crypto policy.

Some libraries like OpenSSL provides methods to override the FIPS settings, but it requires the application to support setting the right properties via APIs.

In general we want the system to do "the right" thing in FIPS mode at any rate.

Thanks for the explanation. In that case it makes perfect sense and the fix for glib2 will be just to not abort the application.

(In reply to Milan Crha from comment #14)
> Thanks for the explanation. In that case it makes perfect sense and the fix
> for glib2 will be just to not abort the application.

I think we actually have to abort. g_compute_hmac_for_data() is not a fallible API. We can't return the correct result, and there is no way to indicate error, so I think crashing is the only thing we can plausibly do. At least, that's the best I can think of.

My proposal is to crash with a proper error message via g_error(), rather than with a null pointer dereference, so it looks intentional rather than broken.

(In reply to Michael Catanzaro from comment #15)
> My proposal is to crash with a proper error message via g_error(), rather
> than with a null pointer dereference, so it looks intentional rather than
> broken.

Works for me. After all the above information, which I've not been aware of, I agree with you that failing when MD5 for HMAC is disabled is correct. The application crash is not ideal, but if the API doesn't let you fail gracefully, then there are not many options.

Maybe, in this particular case, libsoup uses g_compute_hmac_for_data(). What if you'll return NULL from this function (and set the errno to something like ENOTSUP, though the NULL is self-explanatory), and I'd update the libsoup patch to handle this case and return an error, which can be shown to the user, instead of crashing?

Hm, we could indeed return NULL instead of crashing.

Unfortunately the function's return value is not nullable, so returning NULL would be problematic. Applications other than your updated libsoup would not be prepared for that and would likely crash anyway. We can't make a non-nullable return value nullable since it's an API break. Well, we could do it downstream, but that would be risky, and upstream would never accept it.

So although that would work, I still think crashing with g_error() is the least-bad solution here.

Right, "the other code" would break anyway, by dereferencing the NULL pointer (or by passing it to memcpy or such function). I'm fine with the change as you suggested it, there are really not many options, I'd only like to avoid bug reports about "app crashed", leading to: "the log says what is wrong, but you did not look at it, which you should". Having the error in the GUI might avoid such bug reports.

Nonetheless, feel free to go with the g_error() thing. Thanks.

Actually we might be able to avoid crashing after all, because g_hmac_new() is allowed to return NULL, unlike all the other GHmac functions. So I can probably adjust the patch to HMac some random string as a way to guess whether the requested digest is supported or not, and return NULL if it fails.

I will add more error checking to crash the application with g_error() if the underlying GnuTLS calls fail for any other reason, but that's unlikely to happen. So this should allow libsoup to avoid crashing.

(In reply to Michael Catanzaro from comment #19)
> So I can probably adjust the patch to HMac some random string as a way to guess
> whether the requested digest is supported or not, and return NULL if it
> fails.

If you look into the backtrace above, you'll see the crash happened in the GnuTLS code, due to passing NULL as the GnuTLS structure. Meaning you can see whether the hash function is supported or not when you get NULL or non-NULL from the GnuTLS initi function, and/or when you check the return value of the init function, which is not the case right now.

In any case, libsoup uses the wrapper function, g_compute_hmac_for_data(). It doesn't call g_hmac_new() directly.

Oh you're right. That makes this even easier. So I'll just have g_hmac_new() return NULL when gnutls_hmac_init() fails, rather than returning a broken GHmac. g_compute_hmac_for_data() already knows to handle that. OK, I think that settles this nicely. Further updates in bug #1971533.

Ah, one more bug! g_hmac_update() accepts -1 as length parameter, but GnuTLS does not. We currently pass it on, resulting in a crash. I'll fix that too.

P.S. Workaround if you want to use NTMLv2 without disabling FIPS mode systemwide is to use the environment variable GNUTLS_FORCE_FIPS_MODE=0. (Of course, if you need FIPS mode, you're probably not *allowed* to disable it. But the option is there.)

I lost track of this bug report, I'm sorry.

Could someone confirm my summary of the above investigation, please?

1) the build with libsoup patch works in non-FIPS mode;
2) the NTLMv2 is not supposed to work in the FIPS mode.

That means I can create an official build of the libsoup and consider this fixed, right?

(In reply to Milan Crha from comment #24)
> 2) the NTLMv2 is not supposed to work in the FIPS mode.

This was confirmed in comment #13.

@amike Could you confirm the patched libsoup works in the NTLMv2, please? It's the last bit missing to make an official build. I do not have a server to try this with.

I made a new scratch build here:
https://brewweb.engineering.redhat.com/brew/taskinfo?taskID=45867821

Note the evolution and the evolution-data-server processes will need restart after the update of the package, which can be done with:

   $ evolution --force-shutdown

Two months forward, could anyone confirm the patched libsoup works with an NTLMv2 server when the FIPS is disabled, please?

Since the problem described in this bug report should be
resolved in a recent advisory, it has been closed with a
resolution of ERRATA.

For information on the advisory (libsoup bug fix and enhancement update), and where to find the updated
files, follow the link below.

If the solution does not work for you, open a new bug report.

https://access.redhat.com/errata/RHBA-2023:2950