01/01: [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.

[Mark Jansen]

https://git.reactos.org/?p=reactos.git;a=commitdiff;h=b56193a27c8cb3180710f…

commit b56193a27c8cb3180710f26f03af7084415479ce
Author:     Mark Jansen <mark.jansen(a)reactos.org>
AuthorDate: Sun Feb 18 12:56:33 2018 +0100
Commit:     Mark Jansen <mark.jansen(a)reactos.org>
CommitDate: Sun Feb 18 13:01:02 2018 +0100

    [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 d9d5aec84a..8e2e76ac44 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;
+
+    /* 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))
+    {
+        return FALSE;
+    }
 
-    hConnect = InternetConnectW(hInternet, lpszHostName, INTERNET_DEFAULT_HTTPS_PORT,
NULL, NULL, INTERNET_SERVICE_HTTP, INTERNET_FLAG_SECURE, 0);
-    if (hConnect)
+    if (certInfo.lpszSubjectInfo)
     {
-        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);
+        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);
     }
-    return Ret;
+    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;