Problem in openmSupply desktop with certs

[nishaDangol-Sussol]

What went wrong? 😲

Problem in openmSupply desktop with certs

Expected behaviour 🤔

If you have correct certs used, you must not see such error?
PS: it works fine with browser, problem's only with oms-desktop exe

How to Reproduce 🔨

Steps to reproduce the behaviour:

1. Get oms server running. Have the correct certs file as well
2. Now open oms-desktop exe
3. You'll see the server in the available list. Click it
4. See error

It was working good for v1.6, so wonder if this is a problem with v1.7-rc3; need to investigate/check further!

Your environment 🌱

• Version: v1.7-rc3 exe
• mSupply version: v713-rc2

• Platform:desktop windows

• Database type: sqlite

[clemens-msupply]

I just tried it out by installing:

• Open_mSupply_Server_v1.7.00-rc3.exe + Open_mSupply_Desktop_v1.7.00-rc3.exe
• Open_mSupply_Standalone_v1.7.00-rc3.exe
  Both are working for me.

Could you please clarify what exactly you were doing? e.g. what do you mean by "Have the correct certs file as well" do you run omSupply as a public server with a public url? (@mark-prins is the omSupply desktop client supposed to be able to connect to such a server?)

[mark-prins]

Looking at the error - the problem would be that the cert is for a domain name (xxx.msupply.org perhaps?) but.. if you connect using desktop and IP discovery, then it connects using the IP ( as per the error above ).

It's a pretty unique scenario! I don't see clients being able to connect a desktop client to a public server in this way.
Two possible workarounds:

1. we are only checking

      error != 'net::ERR_CERT_INVALID' &&
      error != 'net::ERR_CERT_AUTHORITY_INVALID'

as potential SSL errors to allow through, in electron. Expand the list to include ERR_CERT_COMMON_NAME_INVALID

2. Add the ability to enter a random server address / url for connecting to. The user could then enter the valid domain name.

I'm thinking that we do both. What do others think?

[raviSussol]

I was thinking a different idea that we should do a http connection (instead of https connection) to the available connection servers since ssl connection can't be established in the same sub-net IPs for different servers anyway. So showing available IPs with https and trying to do a ssl connection wouldn't make sense IMO?

Besides, having an option 2 (ability to add random server address/url) would be a good idea or we'll be needing that in the future at some point anyway I think. Specially, if the server is being hosted in the different cloud machine.

[mark-prins]

Thanks Ravi - was hoping that someone would see this issue! the server will be running on SSL though so we cannot connect via HTTP

[clemens-msupply]

We want ssl in any case, you don't know in which network open mSupply will run, e.g. if the local network is also publicly available as a wifi spot. To connect to self-signed certs we do our own certificate pinning, i.e. we assume there is no attacker while setting up the system, and that we are getting the correct pub cert from the server when connecting the very first time. We then remember this cert and fail if the cert has changed, e.g. this could be because there is an attacker...

Will have a look if ERR_CERT_COMMON_NAME_INVALID opens up other things as well...

As I understand there are two issues:

• Fix connection to a local server that is also a public server with a proper URL and cert (would be nice if we could get the server domain instead of the ip though, e.g. advertised it during discovery along with the ip...)
• Have a UI to allow entering any public server URLs

[clemens-msupply]

Is this actually a realistic scenario at the moment?

I am a bit reluctant to accept any ERR_CERT_COMMON_NAME_INVALID, especially if we might want to support using the electron app as a web browser in the future. Then it might be useful to have this check in but think both features should be implemented at the same time to get it right...

[mark-prins]

Yes, I'm with you @clemens-msupply. We could look at supplying the domain name in the discovery packet, though I don't think we have the domain on the server, so that would require extra config.

Simplest would be to allow specifying the server to connect to, and that fits a couple of use-cases now, so I think is helpful.
This is a pretty niche case, unlikely to be a problem for normal users

[clemens-msupply]

I don't think this is a pressing issue, but more of a nice to have. So I will remove it from the v2 milestone. Think this is mostly a problem for testers? @nisha-dangol does this need to be addressed soon?

[nishaDangol-Sussol]

I don't think this is a pressing issue, but more of a nice to have. So I will remove it from the v2 milestone. Think this is mostly a problem for testers? @nisha-dangol does this need to be addressed soon?

I haven't seen this original issue recently in v2.0.0-rc5, but have been seeing another issue on fresh installation of standalone mostly:
https://github.com/msupply-foundation/open-msupply/assets/59193490/c6843cf2-b1f5-47b6-9b0a-deb21aca8bcd

Since on installation of standalone, it automatically starts the server and desktop-client (without giving me any time to change the updated certs manually), this error pops up continuosly.
Anything that can be done for this instead?