From 35da381a117a007b5c071833cf40a340d12e250a Mon Sep 17 00:00:00 2001 From: Pete Batard Date: Fri, 1 Sep 2017 18:27:34 +0100 Subject: [PATCH] [pki] check timestamp chronology during update validation * Done to address the second "vulnerability" proposed in #1009, independently of the protocol used. --- res/localization/rufus.loc | 4 + src/parser.c | 73 ++++++++++++++++ src/pki.c | 168 ++++++++++++++++++++++++++++++++++++- src/resource.h | 3 +- src/rufus.h | 3 + src/rufus.rc | 10 +-- 6 files changed, 253 insertions(+), 8 deletions(-) diff --git a/res/localization/rufus.loc b/res/localization/rufus.loc index 625566fc..c09ea44e 100644 --- a/res/localization/rufus.loc +++ b/res/localization/rufus.loc @@ -565,6 +565,10 @@ t MSG_297 "Truncated ISO detected" t MSG_298 "The ISO file you have selected does not match its declared size: %s of data is missing!\n\nIf you obtained " "this file from the Internet, you should try to download a new copy and verify that the MD5 or SHA checksums match " "the official ones.\n\nNote that you can compute the MD5 or SHA in Rufus by clicking the '#' button." +t MSG_299 "Timestamp validation error" +t MSG_300 "Rufus could not validate that the timestamp of the downloaded update is more recent than the one for the " + "current executable.\n\nIn order to prevent potential attack scenarios, the update process has been aborted and " + "the download will be deleted. Please check the log for more details." ################################################################################ ############################# TRANSLATOR END COPY ############################## diff --git a/src/parser.c b/src/parser.c index dc61e1f1..d53cc22e 100644 --- a/src/parser.c +++ b/src/parser.c @@ -1265,3 +1265,76 @@ char* replace_char(const char* src, const char c, const char* rep) res[j] = 0; return res; } + +static void* get_oid_data_from_asn1_internal(const uint8_t* buf, size_t buf_len, const void* oid, + size_t oid_len, uint8_t asn1_type, size_t* data_len, BOOL* matched) +{ + void* ret; + size_t pos = 0, len, len_len, i; + uint8_t tag; + BOOL is_sequence; + + while (pos < buf_len) { + is_sequence = buf[pos] & 0x20; // Only need to handle the sequence attribute + tag = buf[pos++] & 0x1F; + + // Compute the length + len = 0; + len_len = 1; + if (tag == 0x05) { // ignore "NULL" tag + pos++; + } else { + if (buf[pos] & 0x80) { + len_len = buf[pos++] & 0x7F; + // The data we're dealing with is not expected to ever be larger than 64K + if (len_len > 2) { + uprintf("get_oid_data_from_asn1: Length fields larger than 2 bytes are unsupported"); + return NULL; + } + for (i = 0; i < len_len; i++) { + len <<= 8; + len += buf[pos++]; + } + } else { + len = buf[pos++]; + } + + if (len > buf_len - pos) { + uprintf("get_oid_data_from_asn1: Overflow error (computed length %d is larger than remaining data)", len); + return NULL; + } + } + + if (len != 0) { + if (is_sequence) { + ret = get_oid_data_from_asn1_internal(&buf[pos], len, oid, oid_len, asn1_type, data_len, matched); + if (ret != NULL) + return ret; + } else { + // NB: 0x06 = "OID" tag + if ((!*matched) && (tag == 0x06) && (len == oid_len) && (memcmp(&buf[pos], oid, oid_len) == 0)) { + *matched = TRUE; + } else if ((*matched) && (tag == asn1_type)) { + *data_len = len; + return (void*) &buf[pos]; + } + } + pos += len; + } + }; + + return NULL; +} + +/* + * Parse an ASN.1 binary buffer and return a pointer to the first instance of OID data of type 'asn1_type', + * matching the binary OID 'oid' (of size 'oid_len'). If successful, the length or the returned data is + * placed in 'data_len'. + * If 'oid' is NULL, the first data element of type 'asn1_type' is returned. + */ +void* get_oid_data_from_asn1(const uint8_t* buf, size_t buf_len, const uint8_t* oid, size_t oid_len, + uint8_t asn1_type, size_t* data_len) +{ + BOOL matched = (oid == NULL); + return get_oid_data_from_asn1_internal(buf, buf_len, oid, oid_len, asn1_type, data_len, &matched); +} diff --git a/src/pki.c b/src/pki.c index 00d01418..c4ec4013 100644 --- a/src/pki.c +++ b/src/pki.c @@ -119,6 +119,8 @@ const char* WinPKIErrorString(void) return "Not digitally signed."; case TRUST_E_EXPLICIT_DISTRUST: return "One of the certificates used was marked as untrusted by the user."; + case TRUST_E_TIME_STAMP: + return "The timestamp could not be verified."; default: static_sprintf(error_string, "Unknown PKI error 0x%08lX", error_code); return error_string; @@ -137,7 +139,6 @@ char* GetSignatureName(const char* path) PCCERT_CONTEXT pCertContext = NULL; DWORD dwSize, dwEncoding, dwContentType, dwFormatType, dwSubjectSize; PCMSG_SIGNER_INFO pSignerInfo = NULL; - PCMSG_SIGNER_INFO pCounterSignerInfo = NULL; DWORD dwSignerInfo = 0; CERT_INFO CertInfo = { 0 }; SPROG_PUBLISHERINFO ProgPubInfo = { 0 }; @@ -221,7 +222,6 @@ out: safe_free(ProgPubInfo.lpszPublisherLink); safe_free(ProgPubInfo.lpszMoreInfoLink); safe_free(pSignerInfo); - safe_free(pCounterSignerInfo); if (pCertContext != NULL) CertFreeCertificateContext(pCertContext); if (hStore != NULL) @@ -231,6 +231,154 @@ out: return p; } +// The timestamping authorities we use are RFC 3161 compliant +static uint64_t GetRFC3161TimeStamp(PCMSG_SIGNER_INFO pSignerInfo) +{ + // Binary representation of szOID_TIMESTAMP_TOKEN or "1.2.840.113549.1.9.16.1.4" + const uint8_t OID_RFC3161_timeStamp[] = { 0x2a, 0x86, 0x48, 0x86, 0xf7, 0x0d, 0x01, 0x09, 0x10, 0x01, 0x04 }; + BOOL r; + DWORD n, dwSize; + PCRYPT_CONTENT_INFO pCounterSignerInfo = NULL; + uint64_t ts = 0ULL; + uint8_t *timestamp_token; + size_t timestamp_token_size; + char* timestamp_str; + size_t timestamp_str_size; + + // Loop through unathenticated attributes for szOID_RFC3161_counterSign OID + for (n = 0; n < pSignerInfo->UnauthAttrs.cAttr; n++) { + if (lstrcmpA(pSignerInfo->UnauthAttrs.rgAttr[n].pszObjId, szOID_RFC3161_counterSign) == 0) { + // Get size + r = CryptDecodeObject(PKCS_7_ASN_ENCODING, PKCS_CONTENT_INFO, + pSignerInfo->UnauthAttrs.rgAttr[n].rgValue[0].pbData, + pSignerInfo->UnauthAttrs.rgAttr[n].rgValue[0].cbData, + 0, NULL, &dwSize); + if (!r) { + uprintf("PKI: Could not get CounterSigner (timestamp) data size: %s", WinPKIErrorString()); + continue; + } + + // Allocate memory. + pCounterSignerInfo = calloc(dwSize, 1); + if (pCounterSignerInfo == NULL) { + uprintf("PKI: Unable to allocate memory for CounterSigner (timestamp) data"); + continue; + } + + // Now read the CounterSigner message data + r = CryptDecodeObject(PKCS_7_ASN_ENCODING, PKCS_CONTENT_INFO, + pSignerInfo->UnauthAttrs.rgAttr[n].rgValue[0].pbData, + pSignerInfo->UnauthAttrs.rgAttr[n].rgValue[0].cbData, + 0, (PVOID)pCounterSignerInfo, &dwSize); + if (!r) { + uprintf("PKI: Could not retrieve CounterSigner (timestamp) data: %s", WinPKIErrorString()); + continue; + } + + // Get the RFC 3161 timestamp message + timestamp_token = get_oid_data_from_asn1(pCounterSignerInfo->Content.pbData, + pCounterSignerInfo->Content.cbData, OID_RFC3161_timeStamp, sizeof(OID_RFC3161_timeStamp), + // 0x04 = "Octet String" ASN.1 tag + 0x04, ×tamp_token_size); + if (timestamp_token) { + timestamp_str = get_oid_data_from_asn1(timestamp_token, timestamp_token_size, NULL, 0, + // 0x18 = "Generalized Time" ASN.1 tag + 0x18, ×tamp_str_size); + if (timestamp_str) { + // As per RFC 3161 The syntax is: YYYYMMDDhhmmss[.s...]Z + if ((timestamp_str_size < 14) || (timestamp_str[timestamp_str_size - 1] != 'Z')) { + // Sanity checks + uprintf("PKI: Not an RFC 3161 timestamp"); + DumpBufferHex(timestamp_str, timestamp_str_size); + } else { + ts = strtoull(timestamp_str, NULL, 10); + } + } + } + safe_free(pCounterSignerInfo); + } + } + return ts; +} + +// Return the signature timestamp (as a YYYYMMDDHHMMSS value) or 0 on error +uint64_t GetSignatureTimeStamp(const char* path) +{ + char *mpath = NULL; + BOOL r; + HMODULE hm; + HCERTSTORE hStore = NULL; + HCRYPTMSG hMsg = NULL; + DWORD dwSize, dwEncoding, dwContentType, dwFormatType; + PCMSG_SIGNER_INFO pSignerInfo = NULL; + DWORD dwSignerInfo = 0; + wchar_t *szFileName; + uint64_t timestamp = 0ULL; + + // If the path is NULL, get the signature of the current runtime + if (path == NULL) { + szFileName = calloc(MAX_PATH, sizeof(wchar_t)); + if (szFileName == NULL) + goto out; + hm = GetModuleHandle(NULL); + if (hm == NULL) { + uprintf("PKI: Could not get current executable handle: %s", WinPKIErrorString()); + goto out; + } + dwSize = GetModuleFileNameW(hm, szFileName, MAX_PATH); + if ((dwSize == 0) || ((dwSize == MAX_PATH) && (GetLastError() == ERROR_INSUFFICIENT_BUFFER))) { + uprintf("PKI: Could not get module filename: %s", WinPKIErrorString()); + goto out; + } + mpath = wchar_to_utf8(szFileName); + } else { + szFileName = utf8_to_wchar(path); + } + + // Get message handle and store handle from the signed file. + r = CryptQueryObject(CERT_QUERY_OBJECT_FILE, szFileName, + CERT_QUERY_CONTENT_FLAG_PKCS7_SIGNED_EMBED, CERT_QUERY_FORMAT_FLAG_BINARY, + 0, &dwEncoding, &dwContentType, &dwFormatType, &hStore, &hMsg, NULL); + if (!r) { + uprintf("PKI: Failed to get signature for '%s': %s", (path == NULL) ? mpath : path, WinPKIErrorString()); + goto out; + } + + // Get signer information size. + r = CryptMsgGetParam(hMsg, CMSG_SIGNER_INFO_PARAM, 0, NULL, &dwSignerInfo); + if (!r) { + uprintf("PKI: Failed to get signer size: %s", WinPKIErrorString()); + goto out; + } + + // Allocate memory for signer information. + pSignerInfo = (PCMSG_SIGNER_INFO)calloc(dwSignerInfo, 1); + if (!pSignerInfo) { + uprintf("PKI: Could not allocate memory for signer information"); + goto out; + } + + // Get Signer Information. + r = CryptMsgGetParam(hMsg, CMSG_SIGNER_INFO_PARAM, 0, (PVOID)pSignerInfo, &dwSignerInfo); + if (!r) { + uprintf("PKI: Failed to get signer information: %s", WinPKIErrorString()); + goto out; + } + + // Get the RFC 3161 timestamp + timestamp = GetRFC3161TimeStamp(pSignerInfo); + +out: + safe_free(mpath); + safe_free(szFileName); + safe_free(pSignerInfo); + if (hStore != NULL) + CertCloseStore(hStore, 0); + if (hMsg != NULL) + CryptMsgClose(hMsg); + return timestamp; +} + // From https://msdn.microsoft.com/en-us/library/windows/desktop/aa382384.aspx LONG ValidateSignature(HWND hDlg, const char* path) { @@ -241,6 +389,7 @@ LONG ValidateSignature(HWND hDlg, const char* path) { 0xaac56b, 0xcd44, 0x11d0,{ 0x8c, 0xc2, 0x0, 0xc0, 0x4f, 0xc2, 0x95, 0xee } }; char *signature_name; size_t i, len; + uint64_t current_ts, update_ts; // 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 @@ -292,6 +441,21 @@ LONG ValidateSignature(HWND hDlg, const char* path) safe_free(trust_file.pcwszFilePath); switch (r) { case ERROR_SUCCESS: + // Verify that the timestamp of the downloaded update is in the future of our current one. + // This is done to prevent the use of an officially signed, but older binary, as potential attack vector. + current_ts = GetSignatureTimeStamp(NULL); + if (current_ts == 0ULL) { + uprintf("PKI: Cannot retreive the current binary's timestamp - Aborting update"); + r = TRUST_E_TIME_STAMP; + } else { + update_ts = GetSignatureTimeStamp(path); + if (update_ts < current_ts) { + uprintf("PKI: Update timestamp (%" PRIi64 ") is older than ours (%" PRIi64 ")! - Aborting update", update_ts, current_ts); + r = TRUST_E_TIME_STAMP; + } + } + if (r != ERROR_SUCCESS) + MessageBoxExU(hDlg, lmprintf(MSG_300), lmprintf(MSG_299), MB_OK | MB_ICONERROR | MB_IS_RTL, selected_langid); break; case TRUST_E_NOSIGNATURE: // Should already have been reported, but since we have a custom message for it... diff --git a/src/resource.h b/src/resource.h index cb9bcef1..e059eb71 100644 --- a/src/resource.h +++ b/src/resource.h @@ -481,7 +481,8 @@ #define MSG_297 3297 #define MSG_298 3298 #define MSG_299 3299 -#define MSG_MAX 3300 +#define MSG_300 3300 +#define MSG_MAX 3301 // Next default values for new objects // diff --git a/src/rufus.h b/src/rufus.h index 2edfb86d..5d3680f2 100644 --- a/src/rufus.h +++ b/src/rufus.h @@ -477,6 +477,8 @@ extern char* insert_section_data(const char* filename, const char* section, cons extern char* replace_in_token_data(const char* filename, const char* token, const char* src, const char* rep, BOOL dos2unix); extern char* replace_char(const char* src, const char c, const char* rep); extern void parse_update(char* buf, size_t len); +extern void* get_oid_data_from_asn1(const uint8_t* buf, size_t buf_len, const uint8_t* oid, size_t oid_len, + uint8_t asn1_type, size_t* data_len); extern uint8_t WimExtractCheck(void); extern BOOL WimExtractFile(const char* wim_image, int index, const char* src, const char* dst); extern BOOL WimExtractFile_API(const char* image, int index, const char* src, const char* dst); @@ -487,6 +489,7 @@ extern BOOL AppendVHDFooter(const char* vhd_path); extern int SetWinToGoIndex(void); extern int IsHDD(DWORD DriveIndex, uint16_t vid, uint16_t pid, const char* strid); extern char* GetSignatureName(const char* path); +extern uint64_t GetSignatureTimeStamp(const char* path); extern LONG ValidateSignature(HWND hDlg, const char* path); extern BOOL IsFontAvailable(const char* font_name); extern BOOL WriteFileWithRetry(HANDLE hFile, LPCVOID lpBuffer, DWORD nNumberOfBytesToWrite, diff --git a/src/rufus.rc b/src/rufus.rc index 0d0c2a73..31c9429e 100644 --- a/src/rufus.rc +++ b/src/rufus.rc @@ -33,7 +33,7 @@ LANGUAGE LANG_NEUTRAL, SUBLANG_NEUTRAL IDD_DIALOG DIALOGEX 12, 12, 242, 376 STYLE DS_SETFONT | DS_MODALFRAME | DS_CENTER | WS_MINIMIZEBOX | WS_POPUP | WS_CAPTION | WS_SYSMENU EXSTYLE WS_EX_ACCEPTFILES -CAPTION "Rufus 2.17.1187" +CAPTION "Rufus 2.17.1188" FONT 8, "Segoe UI Symbol", 400, 0, 0x0 BEGIN LTEXT "Device",IDS_DEVICE_TXT,9,6,200,8 @@ -366,8 +366,8 @@ END // VS_VERSION_INFO VERSIONINFO - FILEVERSION 2,17,1187,0 - PRODUCTVERSION 2,17,1187,0 + FILEVERSION 2,17,1188,0 + PRODUCTVERSION 2,17,1188,0 FILEFLAGSMASK 0x3fL #ifdef _DEBUG FILEFLAGS 0x1L @@ -384,13 +384,13 @@ BEGIN BEGIN VALUE "CompanyName", "Akeo Consulting (http://akeo.ie)" VALUE "FileDescription", "Rufus" - VALUE "FileVersion", "2.17.1187" + VALUE "FileVersion", "2.17.1188" VALUE "InternalName", "Rufus" VALUE "LegalCopyright", "© 2011-2017 Pete Batard (GPL v3)" VALUE "LegalTrademarks", "http://www.gnu.org/copyleft/gpl.html" VALUE "OriginalFilename", "rufus.exe" VALUE "ProductName", "Rufus" - VALUE "ProductVersion", "2.17.1187" + VALUE "ProductVersion", "2.17.1188" END END BLOCK "VarFileInfo"