[pki] add country code validation on signature check

* Also validate against the CN rather than the simple name, and require an exact match
This commit is contained in:
Pete Batard 2017-09-11 12:13:47 +01:00
parent f89f97d4ab
commit e3fbfb30d3
4 changed files with 35 additions and 21 deletions

View File

@ -40,8 +40,11 @@
#define szOID_NESTED_SIGNATURE "1.3.6.1.4.1.311.2.4.1" #define szOID_NESTED_SIGNATURE "1.3.6.1.4.1.311.2.4.1"
#endif #endif
// Signatures names we accept (may be suffixed, but the signature should start with one of those) // Signatures names we accept. Must be the the exact name, including capitalization,
// that CertGetNameStringA(CERT_NAME_ATTR_TYPE, szOID_COMMON_NAME) returns.
const char* cert_name[3] = { "Akeo Consulting", "Akeo Systems", "Pete Batard" }; const char* cert_name[3] = { "Akeo Consulting", "Akeo Systems", "Pete Batard" };
// For added security, we also validate the country code of the certificate recipient.
const char* cert_country = "IE";
typedef struct { typedef struct {
LPWSTR lpszProgramName; LPWSTR lpszProgramName;
@ -133,16 +136,17 @@ const char* WinPKIErrorString(void)
} }
// Mostly from https://support.microsoft.com/en-us/kb/323809 // Mostly from https://support.microsoft.com/en-us/kb/323809
char* GetSignatureName(const char* path) char* GetSignatureName(const char* path, const char* country_code)
{ {
static char szSubjectName[128]; static char szSubjectName[128];
static char szCountry[3];
char *p = NULL, *mpath = NULL; char *p = NULL, *mpath = NULL;
BOOL r; BOOL r;
HMODULE hm; HMODULE hm;
HCERTSTORE hStore = NULL; HCERTSTORE hStore = NULL;
HCRYPTMSG hMsg = NULL; HCRYPTMSG hMsg = NULL;
PCCERT_CONTEXT pCertContext = NULL; PCCERT_CONTEXT pCertContext = NULL;
DWORD dwSize, dwEncoding, dwContentType, dwFormatType, dwSubjectSize; DWORD dwSize, dwEncoding, dwContentType, dwFormatType;
PCMSG_SIGNER_INFO pSignerInfo = NULL; PCMSG_SIGNER_INFO pSignerInfo = NULL;
DWORD dwSignerInfo = 0; DWORD dwSignerInfo = 0;
CERT_INFO CertInfo = { 0 }; CERT_INFO CertInfo = { 0 };
@ -209,15 +213,29 @@ char* GetSignatureName(const char* path)
goto out; goto out;
} }
// If a country code is provided, validate that the certificate we have is for the same country
if (country_code != NULL) {
dwSize = CertGetNameStringA(pCertContext, CERT_NAME_ATTR_TYPE, 0, szOID_COUNTRY_NAME,
szCountry, sizeof(szCountry));
if (dwSize < 2) {
uprintf("PKI: Failed to get Country Code");
goto out;
}
if (strcmpi(country_code, szCountry) != 0) {
uprintf("PKI: Unexpected Country Code (Found '%s', expected '%s')", szCountry, country_code);
goto out;
}
}
// Isolate the signing certificate subject name // Isolate the signing certificate subject name
dwSubjectSize = CertGetNameStringA(pCertContext, CERT_NAME_SIMPLE_DISPLAY_TYPE, 0, NULL, dwSize = CertGetNameStringA(pCertContext, CERT_NAME_ATTR_TYPE, 0, szOID_COMMON_NAME,
szSubjectName, sizeof(szSubjectName)); szSubjectName, sizeof(szSubjectName));
if (dwSubjectSize <= 1) { if (dwSize <= 1) {
uprintf("PKI: Failed to get Subject Name"); uprintf("PKI: Failed to get Subject Name");
goto out; goto out;
} }
uprintf("Downloaded executable is signed by '%s'", szSubjectName); uprintf("Binary executable is signed by '%s' (%s)", szSubjectName, szCountry);
p = szSubjectName; p = szSubjectName;
out: out:
@ -473,26 +491,22 @@ LONG ValidateSignature(HWND hDlg, const char* path)
GUID guid_generic_verify = // WINTRUST_ACTION_GENERIC_VERIFY_V2 GUID guid_generic_verify = // WINTRUST_ACTION_GENERIC_VERIFY_V2
{ 0xaac56b, 0xcd44, 0x11d0,{ 0x8c, 0xc2, 0x0, 0xc0, 0x4f, 0xc2, 0x95, 0xee } }; { 0xaac56b, 0xcd44, 0x11d0,{ 0x8c, 0xc2, 0x0, 0xc0, 0x4f, 0xc2, 0x95, 0xee } };
char *signature_name; char *signature_name;
size_t i, len; size_t i;
uint64_t current_ts, update_ts; uint64_t current_ts, update_ts;
// Check the signature name. Make it specific enough (i.e. don't simply check for "Akeo") // Check the signature name. Make it specific enough (i.e. don't simply check for "Akeo")
// so that, besides hacking our server, it'll place an extra hurdle on any malicious entity // so that, besides hacking our server, it'll place an extra hurdle on any malicious entity
// into also fooling a C.A. to issue a certificate that passes our test. // into also fooling a C.A. to issue a certificate that passes our test.
signature_name = GetSignatureName(path); signature_name = GetSignatureName(path, cert_country);
if (signature_name == NULL) { if (signature_name == NULL) {
uprintf("PKI: Could not get signature name"); uprintf("PKI: Could not get signature name");
MessageBoxExU(hDlg, lmprintf(MSG_284), lmprintf(MSG_283), MB_OK | MB_ICONERROR | MB_IS_RTL, selected_langid); MessageBoxExU(hDlg, lmprintf(MSG_284), lmprintf(MSG_283), MB_OK | MB_ICONERROR | MB_IS_RTL, selected_langid);
return TRUST_E_NOSIGNATURE; return TRUST_E_NOSIGNATURE;
} }
for (i = 0; i < ARRAYSIZE(cert_name); i++) { for (i = 0; i < ARRAYSIZE(cert_name); i++) {
len = strlen(cert_name[i]); if (strcmp(signature_name, cert_name[i]) == 0)
if (strncmp(signature_name, cert_name[i], len) == 0) {
// Test for whitespace after the part we match, for added safety
if ((len >= strlen(signature_name)) || isspace(signature_name[len]))
break; break;
} }
}
if (i >= ARRAYSIZE(cert_name)) { if (i >= ARRAYSIZE(cert_name)) {
uprintf("PKI: Signature '%s' is unexpected...", signature_name); uprintf("PKI: Signature '%s' is unexpected...", signature_name);
if (MessageBoxExU(hDlg, lmprintf(MSG_285, signature_name), lmprintf(MSG_283), if (MessageBoxExU(hDlg, lmprintf(MSG_285, signature_name), lmprintf(MSG_283),

View File

@ -3166,7 +3166,7 @@ int WINAPI WinMain(HINSTANCE hInstance, HINSTANCE hPrevInstance, LPSTR lpCmdLine
// Look for a .ini file in the current app directory // Look for a .ini file in the current app directory
static_sprintf(ini_path, "%s\\rufus.ini", app_dir); static_sprintf(ini_path, "%s\\rufus.ini", app_dir);
fd = fopenU(ini_path, ini_flags); // Will create the file if portable mode is requested fd = fopenU(ini_path, ini_flags); // Will create the file if portable mode is requested
vc |= (safe_strcmp(GetSignatureName(NULL), cert_name[0]) == 0); vc |= (safe_strcmp(GetSignatureName(NULL, NULL), cert_name[0]) == 0);
if (fd != NULL) { if (fd != NULL) {
ini_file = ini_path; ini_file = ini_path;
fclose(fd); fclose(fd);

View File

@ -488,7 +488,7 @@ extern BOOL IsBootableImage(const char* path);
extern BOOL AppendVHDFooter(const char* vhd_path); extern BOOL AppendVHDFooter(const char* vhd_path);
extern int SetWinToGoIndex(void); extern int SetWinToGoIndex(void);
extern int IsHDD(DWORD DriveIndex, uint16_t vid, uint16_t pid, const char* strid); extern int IsHDD(DWORD DriveIndex, uint16_t vid, uint16_t pid, const char* strid);
extern char* GetSignatureName(const char* path); extern char* GetSignatureName(const char* path, const char* country_code);
extern uint64_t GetSignatureTimeStamp(const char* path); extern uint64_t GetSignatureTimeStamp(const char* path);
extern LONG ValidateSignature(HWND hDlg, const char* path); extern LONG ValidateSignature(HWND hDlg, const char* path);
extern BOOL IsFontAvailable(const char* font_name); extern BOOL IsFontAvailable(const char* font_name);

View File

@ -33,7 +33,7 @@ LANGUAGE LANG_NEUTRAL, SUBLANG_NEUTRAL
IDD_DIALOG DIALOGEX 12, 12, 242, 376 IDD_DIALOG DIALOGEX 12, 12, 242, 376
STYLE DS_SETFONT | DS_MODALFRAME | DS_CENTER | WS_MINIMIZEBOX | WS_POPUP | WS_CAPTION | WS_SYSMENU STYLE DS_SETFONT | DS_MODALFRAME | DS_CENTER | WS_MINIMIZEBOX | WS_POPUP | WS_CAPTION | WS_SYSMENU
EXSTYLE WS_EX_ACCEPTFILES EXSTYLE WS_EX_ACCEPTFILES
CAPTION "Rufus 2.17.1195" CAPTION "Rufus 2.17.1196"
FONT 8, "Segoe UI Symbol", 400, 0, 0x0 FONT 8, "Segoe UI Symbol", 400, 0, 0x0
BEGIN BEGIN
LTEXT "Device",IDS_DEVICE_TXT,9,6,200,8 LTEXT "Device",IDS_DEVICE_TXT,9,6,200,8
@ -366,8 +366,8 @@ END
// //
VS_VERSION_INFO VERSIONINFO VS_VERSION_INFO VERSIONINFO
FILEVERSION 2,17,1195,0 FILEVERSION 2,17,1196,0
PRODUCTVERSION 2,17,1195,0 PRODUCTVERSION 2,17,1196,0
FILEFLAGSMASK 0x3fL FILEFLAGSMASK 0x3fL
#ifdef _DEBUG #ifdef _DEBUG
FILEFLAGS 0x1L FILEFLAGS 0x1L
@ -384,13 +384,13 @@ BEGIN
BEGIN BEGIN
VALUE "CompanyName", "Akeo Consulting (http://akeo.ie)" VALUE "CompanyName", "Akeo Consulting (http://akeo.ie)"
VALUE "FileDescription", "Rufus" VALUE "FileDescription", "Rufus"
VALUE "FileVersion", "2.17.1195" VALUE "FileVersion", "2.17.1196"
VALUE "InternalName", "Rufus" VALUE "InternalName", "Rufus"
VALUE "LegalCopyright", "© 2011-2017 Pete Batard (GPL v3)" VALUE "LegalCopyright", "© 2011-2017 Pete Batard (GPL v3)"
VALUE "LegalTrademarks", "http://www.gnu.org/copyleft/gpl.html" VALUE "LegalTrademarks", "http://www.gnu.org/copyleft/gpl.html"
VALUE "OriginalFilename", "rufus.exe" VALUE "OriginalFilename", "rufus.exe"
VALUE "ProductName", "Rufus" VALUE "ProductName", "Rufus"
VALUE "ProductVersion", "2.17.1195" VALUE "ProductVersion", "2.17.1196"
END END
END END
BLOCK "VarFileInfo" BLOCK "VarFileInfo"