From ced2e8779fb4e4a50f7fb39b4845e4ae7a396234 Mon Sep 17 00:00:00 2001 From: Biru Mohanathas Date: Mon, 2 Aug 2021 04:24:58 +0300 Subject: [PATCH] feat: Allow detection of MITM HTTPS proxies like ZScaler (#30174) * feat: Allow detection of MITM HTTPS proxies like ZScaler For security purposes, Figma heavily restrics the origins that are allowed to load within our Electron app. Unfortunately some corporate environments use MITM proxies like ZScaler, which intercepts our connection to `https://www.figma.com` and serves a redirect to e.g. `https://gateway.zscloud.net` before finally redirecting back to `https://www.figma.com`. In order to detect this situation and handle it gracefully, we need to be able to know whether or not the certificate for our own origin (`https://www.figma.com`) is chained to a known root. We do this by exposesing `CertVerifyResult::is_issued_by_known_root`. If the certification verification passed without the certificate being tied to a known root, we can safely assume that we are dealing with a MITM proxy that has its root CA installed locally on the machine. This means that HTTPS can't be trusted so we might as well make life easier for corporate users by loosening our origin restrictions without any manual steps. * Tweak docs wording --- docs/api/session.md | 3 ++- shell/browser/net/cert_verifier_client.cc | 1 + shell/browser/net/cert_verifier_client.h | 1 + shell/common/gin_converters/net_converter.cc | 1 + spec-main/api-session-spec.ts | 3 ++- 5 files changed, 7 insertions(+), 2 deletions(-) diff --git a/docs/api/session.md b/docs/api/session.md index a6b3afec11f..e0d6db12461 100644 --- a/docs/api/session.md +++ b/docs/api/session.md @@ -443,7 +443,8 @@ the original network configuration. * `hostname` String * `certificate` [Certificate](structures/certificate.md) * `validatedCertificate` [Certificate](structures/certificate.md) - * `verificationResult` String - Verification result from chromium. + * `isIssuedByKnownRoot` Boolean - `true` if Chromium recognises the root CA as a standard root. If it isn't then it's probably the case that this certificate was generated by a MITM proxy whose root has been installed locally (for example, by a corporate proxy). This should not be trusted if the `verificationResult` is not `OK`. + * `verificationResult` String - `OK` if the certificate is trusted, otherwise an error like `CERT_REVOKED`. * `errorCode` Integer - Error code. * `callback` Function * `verificationResult` Integer - Value can be one of certificate error codes diff --git a/shell/browser/net/cert_verifier_client.cc b/shell/browser/net/cert_verifier_client.cc index f5f301bd6e8..e81cda3e1e5 100644 --- a/shell/browser/net/cert_verifier_client.cc +++ b/shell/browser/net/cert_verifier_client.cc @@ -33,6 +33,7 @@ void CertVerifierClient::Verify( params.error_code = default_error; params.certificate = certificate; params.validated_certificate = default_result.verified_cert; + params.is_issued_by_known_root = default_result.is_issued_by_known_root; cert_verify_proc_.Run( params, base::BindOnce( diff --git a/shell/browser/net/cert_verifier_client.h b/shell/browser/net/cert_verifier_client.h index 3df4339ef06..23e8112c2d7 100644 --- a/shell/browser/net/cert_verifier_client.h +++ b/shell/browser/net/cert_verifier_client.h @@ -18,6 +18,7 @@ struct VerifyRequestParams { int error_code; scoped_refptr certificate; scoped_refptr validated_certificate; + bool is_issued_by_known_root; VerifyRequestParams(); VerifyRequestParams(const VerifyRequestParams&); diff --git a/shell/common/gin_converters/net_converter.cc b/shell/common/gin_converters/net_converter.cc index 6dc4ea5b26c..46b62711783 100644 --- a/shell/common/gin_converters/net_converter.cc +++ b/shell/common/gin_converters/net_converter.cc @@ -368,6 +368,7 @@ v8::Local Converter::ToV8( dict.Set("hostname", val.hostname); dict.Set("certificate", val.certificate); dict.Set("validatedCertificate", val.validated_certificate); + dict.Set("isIssuedByKnownRoot", val.is_issued_by_known_root); dict.Set("verificationResult", val.default_result); dict.Set("errorCode", val.error_code); return ConvertToV8(isolate, dict); diff --git a/spec-main/api-session-spec.ts b/spec-main/api-session-spec.ts index 746121164f3..4ade0c89c72 100644 --- a/spec-main/api-session-spec.ts +++ b/spec-main/api-session-spec.ts @@ -577,7 +577,7 @@ describe('session module', () => { it('rejects the request when the callback is called with -2', async () => { const ses = session.fromPartition(`${Math.random()}`); let validate: () => void; - ses.setCertificateVerifyProc(({ hostname, certificate, verificationResult }, callback) => { + ses.setCertificateVerifyProc(({ hostname, certificate, verificationResult, isIssuedByKnownRoot }, callback) => { validate = () => { expect(hostname).to.equal('127.0.0.1'); expect(certificate.issuerName).to.equal('Intermediate CA'); @@ -590,6 +590,7 @@ describe('session module', () => { expect(certificate.issuerCert.issuerCert.subject.commonName).to.equal('Root CA'); expect(certificate.issuerCert.issuerCert.issuerCert).to.equal(undefined); expect(verificationResult).to.be.oneOf(['net::ERR_CERT_AUTHORITY_INVALID', 'net::ERR_CERT_COMMON_NAME_INVALID']); + expect(isIssuedByKnownRoot).to.be.false(); }; callback(-2); });