From b56193a27c8cb3180710f26f03af7084415479ce Mon Sep 17 00:00:00 2001
From: Mark Jansen <mark.jansen@reactos.org>
Date: Sun, 18 Feb 2018 12:56:33 +0100
Subject: [PATCH] [RAPPS] Some fixes to our completely broken cert pinning
 attempt. - We should not open a new connection to request a certificate. -
 Update the issuer / subject for the LE certificate. - User proper types. -
 Require all fields that we check to be present.

Without checking the public key or the entire certificate it's still not secure, but we are a step closer.
Dedicated to Joachim Henze
CORE-14350
---
 base/applications/rapps/loaddlg.cpp | 81 ++++++++++++-----------------
 1 file changed, 34 insertions(+), 47 deletions(-)

diff --git a/base/applications/rapps/loaddlg.cpp b/base/applications/rapps/loaddlg.cpp
index d9d5aec84a4..8e2e76ac446 100644
--- a/base/applications/rapps/loaddlg.cpp
+++ b/base/applications/rapps/loaddlg.cpp
@@ -47,8 +47,8 @@
 #include "misc.h"
 
 #ifdef USE_CERT_PINNING
-#define CERT_ISSUER_INFO "BE\r\nGlobalSign nv-sa\r\nGlobalSign Domain Validation CA - SHA256 - G2"
-#define CERT_SUBJECT_INFO "Domain Control Validated\r\n*.reactos.org"
+#define CERT_ISSUER_INFO "US\r\nLet's Encrypt\r\nLet's Encrypt Authority X3"
+#define CERT_SUBJECT_INFO "svn.reactos.org"
 #endif
 
 enum DownloadStatus
@@ -331,55 +331,42 @@ HRESULT WINAPI CDownloadDialog_Constructor(HWND Dlg, BOOL *pbCancelled, REFIID r
 }
 
 #ifdef USE_CERT_PINNING
-static BOOL CertIsValid(HINTERNET hInternet, LPWSTR lpszHostName)
+static BOOL CertIsValid(HINTERNET hFile, LPWSTR lpszHostName)
 {
-    HINTERNET hConnect;
-    HINTERNET hRequest;
     DWORD certInfoLength;
-    BOOL Ret = FALSE;
-    INTERNET_CERTIFICATE_INFOW certInfo;
+    INTERNET_CERTIFICATE_INFOA certInfo;
+    int ValidFlags = 0;
 
-    hConnect = InternetConnectW(hInternet, lpszHostName, INTERNET_DEFAULT_HTTPS_PORT, NULL, NULL, INTERNET_SERVICE_HTTP, INTERNET_FLAG_SECURE, 0);
-    if (hConnect)
+    /* Despite what the header indicates, the implementation of INTERNET_CERTIFICATE_INFO is not Unicode-aware. */
+    certInfoLength = sizeof(certInfo);
+    if (!InternetQueryOptionA(hFile,
+                              INTERNET_OPTION_SECURITY_CERTIFICATE_STRUCT,
+                              &certInfo,
+                              &certInfoLength))
     {
-        hRequest = HttpOpenRequestW(hConnect, L"HEAD", NULL, NULL, NULL, NULL, INTERNET_FLAG_SECURE, 0);
-        if (hRequest != NULL)
-        {
-            Ret = HttpSendRequestW(hRequest, L"", 0, NULL, 0);
-            if (Ret)
-            {
-                certInfoLength = sizeof(certInfo);
-                Ret = InternetQueryOptionW(hRequest,
-                                           INTERNET_OPTION_SECURITY_CERTIFICATE_STRUCT,
-                                           &certInfo,
-                                           &certInfoLength);
-                if (Ret)
-                {
-                    if (certInfo.lpszEncryptionAlgName)
-                        LocalFree(certInfo.lpszEncryptionAlgName);
-                    if (certInfo.lpszIssuerInfo)
-                    {
-                        if (strcmp((LPSTR) certInfo.lpszIssuerInfo, CERT_ISSUER_INFO) != 0)
-                            Ret = FALSE;
-                        LocalFree(certInfo.lpszIssuerInfo);
-                    }
-                    if (certInfo.lpszProtocolName)
-                        LocalFree(certInfo.lpszProtocolName);
-                    if (certInfo.lpszSignatureAlgName)
-                        LocalFree(certInfo.lpszSignatureAlgName);
-                    if (certInfo.lpszSubjectInfo)
-                    {
-                        if (strcmp((LPSTR) certInfo.lpszSubjectInfo, CERT_SUBJECT_INFO) != 0)
-                            Ret = FALSE;
-                        LocalFree(certInfo.lpszSubjectInfo);
-                    }
-                }
-            }
-            InternetCloseHandle(hRequest);
-        }
-        InternetCloseHandle(hConnect);
+        return FALSE;
     }
-    return Ret;
+
+    if (certInfo.lpszSubjectInfo)
+    {
+        if (strcmp(certInfo.lpszSubjectInfo, CERT_SUBJECT_INFO) == 0)
+            ValidFlags |= 1;
+        LocalFree(certInfo.lpszSubjectInfo);
+    }
+    if (certInfo.lpszIssuerInfo)
+    {
+        if (strcmp(certInfo.lpszIssuerInfo, CERT_ISSUER_INFO) == 0)
+            ValidFlags |= 2;
+        LocalFree(certInfo.lpszIssuerInfo);
+    }
+    if (certInfo.lpszProtocolName)
+        LocalFree(certInfo.lpszProtocolName);
+    if (certInfo.lpszSignatureAlgName)
+        LocalFree(certInfo.lpszSignatureAlgName);
+    if (certInfo.lpszEncryptionAlgName)
+        LocalFree(certInfo.lpszEncryptionAlgName);
+
+    return ValidFlags == 3;
 }
 #endif
 
@@ -768,7 +755,7 @@ DWORD WINAPI CDownloadManager::ThreadFunc(LPVOID param)
         // are we using HTTPS to download the RAPPS update package? check if the certificate is original
         if ((urlComponents.nScheme == INTERNET_SCHEME_HTTPS) &&
             (wcscmp(InfoArray[iAppId].szUrl, APPLICATION_DATABASE_URL) == 0) &&
-            (!CertIsValid(hOpen, urlComponents.lpszHostName)))
+            (!CertIsValid(hFile, urlComponents.lpszHostName)))
         {
             MessageBox_LoadString(hMainWnd, IDS_CERT_DOES_NOT_MATCH);
             goto end;