HTTPS calls to API v3 throw NullReferenceException in OutlookMatters.Core.Http.DefaultHttpResponse::GetPayload

[jabis]

Says most on the topic-line. Object reference not set to an instance of an object -> is not thrown as a MattermostException. Happens: when you click the refreshing channels and give password

Couldn't get proper dumps, as my lovely VS2015 decided to leak all the memory while I was debugging, and threw a bluescreen at me.

Changing to HTTPS (reconfiguring all the expected params of course to live data) also fails the following tests with NullReferenceException (same method)

Also checked with Postman & ARC that data is accessible via ip & fqdn, and checked that the request loginUrl was parsed properly ( scheme is https, port is 443, localPath /api/v3/users/login etc )

I'm not too versed with C# to help, just wanted to bring this to your attention :)

[jabis]

Here's the stack trace dumped into a messagebox. the GetPayload _response seems to be null.

[jabis]

@makmu @maxlmo I don't know which place would be the best (see snippet), but I found out how to get https handshake working with a simple if check on the Uri's around RestServiceImpl.cs - though it's repetitive, so perhaps sniffing the baseUri at save time and a global toggle to enable the SecurityProtocols.

  if (loginUrl.Scheme == "https"){
     ServicePointManager.SecurityProtocol = SecurityProtocolType.Tls | SecurityProtocolType.Tls11 | SecurityProtocolType.Tls12 | SecurityProtocolType.Ssl3;
     ServicePointManager.ServerCertificateValidationCallback = delegate { return true; };
  }

I'll leave it to you to

[makmu]

Hi jabis,

thanks for reporting this. However, the only way I was able to reproduce this problem was by using an invalid (i.e. self-signed) ssl certificate on the mattermost server. We haven't considered this scenario yet. Valid ssl certificates (authorized by an official CA that is installed in the Windows Certificate Store) should work fine though.

I'm somewhat reluctant to deactivate certificate validation in general (as your code suggests). I think we should at least warn the user about the fact that the ssl connection might be insecure (like modern browsers do in such cases). Therefore, I think the best way to handle this would be an additional dialog when logging in that warns the user when the server advertises an invalid certificate. Only if the user accepts this, the addin should do away with certificate validation and only for this session. For convenience, the dialog should also have a check-box like "Don't show this warning again", which would activate an corresponding setting in the global settings file. In the settings dialog it should be possible to reset this setting.

[jabis]

Hi, thanks for the reply - I have to point out that the certificate was letsencrypt fully valid certificate (though on test harness I used the test certificate from LE, as localhosts and dummy fqdns are not validated by LE :)

And yes that's why I suggested a global option for SSL cert validation, so that people could use (if so choose) even self signed ones. I think the issue revolves more with reverse proxying with nginx than the actual cert itself, as the cipher suites might vary on the "frontend" side quite alot. I'm not even sure the delegate was totally necessary, but I couldn't get the tests passing (for obvious reasons) without it.

EDIT: the ServiceProtocolTypes (expected ciphers) should in any case be declared, even the delegate returning true would be optional - without those the response failed on extensive testing.

[makmu]

62 contains some preliminary work to solve this issue