Configuring FFImageLoading's HttpClient throws a null reference exception

[adrianknight89]

I just updated from 2.7.2 to 3.1.0 and noticed the following configuration throws a null reference exception on Android.

Add this to MainActivity:

var configuration = new Configuration
{
    FadeAnimationEnabled = false,
    HttpClient = new System.Net.Http.HttpClient(new NativeMessageHandler())
};

I'm using FFImageLoading 2.4.11.982 and XF 4.

Adding @daniel-luberda in case this issue belongs to his code.

[alexrainman]

Please check the readme. Version 3.x introduces breaking changes.

[adrianknight89]

I did read it though I'm not sure how the changes affect the above code snippet as I'm not doing anything more than a bare minimum setup. I also verified that I'm using TLS 1.2.

[alexrainman]

I will try to reproduce your problem but, you have at least to provide your server certificate public keys in the native handler config.

[adrianknight89]

I'm a bit confused. Previously, using the default constructor of NativeMessageHandler was enough. Now, we have to provide a CustomSSLVerification?

[alexrainman]

Please, read the documentation again. Version 3.x is focused in security and provides new standards to pin server certificates.

[Pinox]

Hi Alex,

I have the same issue. I dont think it's related to FFimageloading as I'm getting the error on login.

I get the following error :

"System.Reflection.TargetInvocationException: Exception has been thrown by the target of an invocation. ---> System.NullReferenceException: Object reference not set to an instance of an object. at ModernHttpClient.NativeMessageHandler..ctor (System.Boolean throwOnCaptiveNetwork, ModernHttpClient.CustomSSLVerification customSSLVerification, ModernHttpClient.NativeCookieHandler cookieHandler, System.Net.IWebProxy proxy)

I normally use:

var modernHttpClientHandler = new NativeMessageHandler()

        {
            Timeout = new TimeSpan(0, 0, 15),
            //EnableUntrustedCertificates = true,
            DisableCaching = true
        };

I changed above code to:

new NativeMessageHandler(false, new CustomSSLVerification())

        {
            DisableCaching = true,
            Timeout = new TimeSpan(0, 0, 9)
        };

I noticed the changes to certificate pinning, I am currently running with no certificate requirements in development and letsencrypt certificate in production.

Reading the docs I would say the above code should work as is without issues and it's not (in development with no certificate). I dont want to include certificate pinning in my code as all my clients are then dependant on this and would be unable to connect to server when the server certificate change every 3 months that's normally the case with letsencrypt.

Am I missing something here or is certificate pinning on the client side now a must ? Also, it's not clear to me what the minimum configuration should look like without creating a dependency to the server certificate

In a micro services architecture , do we have to keep track of all certificates that are provided from different services. Say I have 5 services running each with their own implementation (SSL server certificate) ? What about 3rd party services that can change their certificates at a whim ?

[adrianknight89]

Also, the documentation for the NativeMessageHandler constructor says custom SSL verification is turned off by default for performance reasons. If this is now enforced with version 3.x, then the parameterless constructor should be obsolete and the documentation for this constructor should be updated.

[alexrainman]

@adrianknight89 @Pinox from version 3.x, CustomSSLVerification is required. At least you have to provide your server certificate chain public keys. It is not based on ServiceManager certificate validation callback, which has performance related issues.

If CustomSSLVerification.Pins is null the crash is happening so, I am going to update the plugin to correct that but, you will get a certificate pinning failure if you don't provide them anyway.

Getting a valid certificate is easy and free so, do it for your test environment too. The documentation is updated to match the new requirements.

Pinning certificate chain public keys has its advantages: they never change, even if your server rotates the certificates, the keys doesn't change so, you don't have to update your app. I am also providing support for client certificates so we can have TLS Mutual Authentication.

If version 3.x doesn't fit your needs then downgrade to version 2.7.2 which works in the old fashion way and doesn't have any known bugs.

[Pinox]

@alexrainman ,

Please consider an escape hatch in ModernHttpClient for guys that would like a less aggressive approach to security. Certificate pinning sounds good in theory but has significant inherent risks. When pinning a public key chain , we are assuming that the certificate authority, it's intermediate certificates and your leaf certificate do not change.

So if you are not careful in your execution of renewing your certificate or if your certificate provider suddenly change their certificate then you are stuck as your public keys will change in such instances. Also, if your keys get compromised or if god forbid you lose them then you are forced to change your public key, so now you have to think about backup keys otherwise your users won't be able to access your site.

In my opinion going offline is a much more severe threat than getting exposed to a sophisticated man-in-the-middle attack.

Google initiated certificate pinning and eventually abandoned it because of the lack of an build-in recovery mechanism. I know you stated version 2.7.2 is good but then many users will be stuck if something "new" breaks it.

[alexrainman]

@Pinox @adrianknight89 I am going to merge the previous functionality with the current one, in a way that if you want to use public keys pinning, it will work, and if you doesn't, then it will still work.

[alexrainman]

Done! Version 3.2.0 released.

[alexrainman]

@adrianknight89 @Pinox please, let me know what do you think about the current version.

[adrianknight89]

@alexrainman Did you mean 2.8.0? I don't see 3.2.0. I tested 2.8.0 and it's working fine for me.

[alexrainman]

Yes. I removed 3.x iterations as i am not planning to update this for a while.

Good to know it works as expected.