Skip to content

Commit 0908d63

Browse files
learn-moreJoachimHenze
authored andcommitted
[0.4.8] cherry-pick [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 cherry-picked by Joachim Henze
1 parent bfcab8b commit 0908d63

File tree

1 file changed

+34
-47
lines changed

1 file changed

+34
-47
lines changed

base/applications/rapps/loaddlg.cpp

Lines changed: 34 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -47,8 +47,8 @@
4747
#include "misc.h"
4848

4949
#ifdef USE_CERT_PINNING
50-
#define CERT_ISSUER_INFO "BE\r\nGlobalSign nv-sa\r\nGlobalSign Domain Validation CA - SHA256 - G2"
51-
#define CERT_SUBJECT_INFO "Domain Control Validated\r\n*.reactos.org"
50+
#define CERT_ISSUER_INFO "US\r\nLet's Encrypt\r\nLet's Encrypt Authority X3"
51+
#define CERT_SUBJECT_INFO "svn.reactos.org"
5252
#endif
5353

5454
enum DownloadStatus
@@ -331,55 +331,42 @@ HRESULT WINAPI CDownloadDialog_Constructor(HWND Dlg, BOOL *pbCancelled, REFIID r
331331
}
332332

333333
#ifdef USE_CERT_PINNING
334-
static BOOL CertIsValid(HINTERNET hInternet, LPWSTR lpszHostName)
334+
static BOOL CertIsValid(HINTERNET hFile, LPWSTR lpszHostName)
335335
{
336-
HINTERNET hConnect;
337-
HINTERNET hRequest;
338336
DWORD certInfoLength;
339-
BOOL Ret = FALSE;
340-
INTERNET_CERTIFICATE_INFOW certInfo;
337+
INTERNET_CERTIFICATE_INFOA certInfo;
338+
int ValidFlags = 0;
339+
340+
/* Despite what the header indicates, the implementation of INTERNET_CERTIFICATE_INFO is not Unicode-aware. */
341+
certInfoLength = sizeof(certInfo);
342+
if (!InternetQueryOptionA(hFile,
343+
INTERNET_OPTION_SECURITY_CERTIFICATE_STRUCT,
344+
&certInfo,
345+
&certInfoLength))
346+
{
347+
return FALSE;
348+
}
341349

342-
hConnect = InternetConnectW(hInternet, lpszHostName, INTERNET_DEFAULT_HTTPS_PORT, NULL, NULL, INTERNET_SERVICE_HTTP, INTERNET_FLAG_SECURE, 0);
343-
if (hConnect)
350+
if (certInfo.lpszSubjectInfo)
344351
{
345-
hRequest = HttpOpenRequestW(hConnect, L"HEAD", NULL, NULL, NULL, NULL, INTERNET_FLAG_SECURE, 0);
346-
if (hRequest != NULL)
347-
{
348-
Ret = HttpSendRequestW(hRequest, L"", 0, NULL, 0);
349-
if (Ret)
350-
{
351-
certInfoLength = sizeof(certInfo);
352-
Ret = InternetQueryOptionW(hRequest,
353-
INTERNET_OPTION_SECURITY_CERTIFICATE_STRUCT,
354-
&certInfo,
355-
&certInfoLength);
356-
if (Ret)
357-
{
358-
if (certInfo.lpszEncryptionAlgName)
359-
LocalFree(certInfo.lpszEncryptionAlgName);
360-
if (certInfo.lpszIssuerInfo)
361-
{
362-
if (strcmp((LPSTR) certInfo.lpszIssuerInfo, CERT_ISSUER_INFO) != 0)
363-
Ret = FALSE;
364-
LocalFree(certInfo.lpszIssuerInfo);
365-
}
366-
if (certInfo.lpszProtocolName)
367-
LocalFree(certInfo.lpszProtocolName);
368-
if (certInfo.lpszSignatureAlgName)
369-
LocalFree(certInfo.lpszSignatureAlgName);
370-
if (certInfo.lpszSubjectInfo)
371-
{
372-
if (strcmp((LPSTR) certInfo.lpszSubjectInfo, CERT_SUBJECT_INFO) != 0)
373-
Ret = FALSE;
374-
LocalFree(certInfo.lpszSubjectInfo);
375-
}
376-
}
377-
}
378-
InternetCloseHandle(hRequest);
379-
}
380-
InternetCloseHandle(hConnect);
352+
if (strcmp(certInfo.lpszSubjectInfo, CERT_SUBJECT_INFO) == 0)
353+
ValidFlags |= 1;
354+
LocalFree(certInfo.lpszSubjectInfo);
355+
}
356+
if (certInfo.lpszIssuerInfo)
357+
{
358+
if (strcmp(certInfo.lpszIssuerInfo, CERT_ISSUER_INFO) == 0)
359+
ValidFlags |= 2;
360+
LocalFree(certInfo.lpszIssuerInfo);
381361
}
382-
return Ret;
362+
if (certInfo.lpszProtocolName)
363+
LocalFree(certInfo.lpszProtocolName);
364+
if (certInfo.lpszSignatureAlgName)
365+
LocalFree(certInfo.lpszSignatureAlgName);
366+
if (certInfo.lpszEncryptionAlgName)
367+
LocalFree(certInfo.lpszEncryptionAlgName);
368+
369+
return ValidFlags == 3;
383370
}
384371
#endif
385372

@@ -768,7 +755,7 @@ DWORD WINAPI CDownloadManager::ThreadFunc(LPVOID param)
768755
// are we using HTTPS to download the RAPPS update package? check if the certificate is original
769756
if ((urlComponents.nScheme == INTERNET_SCHEME_HTTPS) &&
770757
(wcscmp(InfoArray[iAppId].szUrl, APPLICATION_DATABASE_URL) == 0) &&
771-
(!CertIsValid(hOpen, urlComponents.lpszHostName)))
758+
(!CertIsValid(hFile, urlComponents.lpszHostName)))
772759
{
773760
MessageBox_LoadString(hMainWnd, IDS_CERT_DOES_NOT_MATCH);
774761
goto end;

0 commit comments

Comments
 (0)