microsoft / AL

Home of the Dynamics 365 Business Central AL Language extension for Visual Studio Code. Used to track issues regarding the latest version of the AL compiler and developer tools available in the Visual Studio Code Marketplace or as part of the AL Developer Preview builds for Dynamics 365 Business Central.
MIT License
741 stars 243 forks source link

Https connections are made with certificate checking disabled in SaaS environment. #7848

Closed rdebath closed 1 month ago

rdebath commented 1 month ago

Please include the following with each issue:

1. Describe the bug Https connections are made with certificate checking disabled. A default OnPrem environment appears to correctly throw errors.

Incorrect certificates tested : Incorrect host, Revoked, No common name, No Subject. The below AL code is the simplest way of download a page from a URL, this is using the default setup. There does not appear to be any way to turn certificate checking back on from the AL language and the underlying DotNet libraries are not directly accessible in a SaaS environment.

These URLs are fetched without complaint in both sandbox and production SaaS instances.

TLS Certificate checking appears to be completely turned off in Live environments, while this is a reasonable option for Sandbox environments it is not acceptable in production.

Impact: Certificate is not checked, if DNS can be polluted or replaced any host connected from Business central can be subjected to a MITM attack. This may include connections to Shopify, government websites and any other third party.

MSRC: Stated that they required a PoC for a Microsoft application and closed the report when presented with this.

Please note that https://github.com/microsoft/AL/issues/7830 is the opposite of this report requesting that OnPrem connections be made insecure.

2. To Reproduce Steps to reproduce the behavior:

Attach the function below to a button and call it with one of the given URL (or any other that does not have a valid certificate)

Test URLs

https://wrong.host.badssl.com/
https://revoked.badssl.com/
https://no-common-name.badssl.com/
https://no-subject.badssl.com/
    procedure GetURLData(URLValue: Text)
    var
        HttpClient: HttpClient;
        HttpResponseMessage: HttpResponseMessage;
        Response: Text;
    begin
        HttpClient.Get(URLValue, HttpResponseMessage);
        HttpResponseMessage.Content.ReadAs(Response);
        message('%1', Response);
    end;

Note: Because the developers need to copy and paste the code snippet, including a code snippet as a media file (i.e. .gif) is not sufficient.

3. Expected behavior These URLs should throw an error stating that the certificate cannot be validated.

4. Actual behavior The HTML for presenting the page is provided with no indication of any issue.

5. Versions:

Final Checklist

Please remember to do the following:

Internal work item: AB#548138

BazookaMusic commented 1 month ago

Accepting due to this being security related. I will route this to the correct team, even though it is outside the scope of the repository as it is about runtime behavior.

BazookaMusic commented 1 month ago

I will close this issue as this is not the correct place to track the work. This repo is about the compiler and associated developer tools while the issue is about the SaaS service and the runtime. We will track it and investigate it internally as soon as possible via the linked work item.

You can also open a support request via the process below for updates via customer support:

https://www.microsoft.com/en-us/dynamics-365/blog/it-professional/2019/09/25/new-process-to-submit-support-requests-for-dynamics-365-business-central/

rdebath commented 1 month ago

Greaaat. That "new process" fails at the first click, the support request button is missing. Next, that looks like standard support, yeah, right. Sure they'll understand the problem 'cause this repo doesn't exist.

Anyway the correct solution would be to expose the ability to add a custom trusted certificate; either true self-signed or a private CA. This is compiler related as you have taken control of the HttpClient base type an there should be a way to configure the TLS certificate checking of this type.

BazookaMusic commented 1 month ago

Standard support would create a ticket that will be forwarded to a team. Tickets with customer impact are prioritized, which is why I suggested this.

The correct solution is for the service to do certificate validation on SaaS and the responsible team is already on it.

As for who owns what, please understand that this is an internal matter. I'm not trying to deflect the issue, I'm trying to route it correctly so that it gets solved asap. The nudge to contact support was a courtesy for you to get some form of contact with the other team who is not on GitHub and help with the prioritization.

dannoe commented 3 weeks ago

The correct solution is for the service to do certificate validation on SaaS and the responsible team is already on it.

This is not the correct solution. There are cases were you are not in control of the "target" certificate and you have to accept a self-signed certificate. This will break our customers. And I am also guessing that this will break a lot more customers of big partners.

JesperSchulz commented 3 weeks ago

The fix for this issue has been checked in to the master branch. It will be available in the bcinsider.azurecr.io/bcsandbox-master Docker image starting from platform build number 26.0.24040.0 and VS Code Extension Version .

If you don’t have access to these images you need to become part of the Ready2Go program: aka.ms/readytogo

For more details on code branches and docker images please read: https://blogs.msdn.microsoft.com/nav/2018/05/03/al-developer-previews-multiple-releases-and-github/ https://freddysblog.com/2020/06/25/working-with-artifacts/

rdebath commented 3 weeks ago

As this is an issue only on SaaS we'd need details on how to simulate this in a Docker sandbox. Assuming I start with Freddy's images is it just a change to the customsettings.config if so what? Can you supply an example as configured for a SaaS service tier? (With redacted keys of course).

Also @JesperSchulz -- You need to update your boilerplate, the 'Ready2Go' URL no longer exists (and the program IIRC)

BazookaMusic commented 3 weeks ago

@jehelles can you help here?

demiliani commented 2 weeks ago

Why version 26.0 and not in a minor CU in v25.x?

BazookaMusic commented 2 weeks ago

@demiliani This is exactly why we close these bugs about platform issues and don't handle them in these issues. The automation is meant to handle the release of the compiler and the message it produced is wrong.

In this case, the change will go in the 25 platform and later. This is handled by a different team whose workflow is not compatible with github.

demiliani commented 2 weeks ago

@demiliani This is exactly why we close these bugs about platform issues and don't handle them in these issues. The automation is meant to handle the release of the compiler and the message it produced is wrong.

In this case, the change will go in the 25 platform and later. This is handled by a different team whose workflow is not compatible with github.

Thanks for confirming :)