From bd439c909045de71f3ab6907ff3f2e74682e7f3a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20Mo=C5=84?= Date: Sat, 21 Sep 2019 20:43:18 +0200 Subject: [PATCH] Win32: Do not reload TLS keylog file on each packet On Windows, fstat() and stat() sets st_dev to different value depending on whether it was called with file handle or file path. If file handle was used, the st_dev is simply the file handle casted to unsigned. If file path was used, then st_dev corresponds to drive letter (A=0, B=1, C=2, ...). Compare the files using the file index information retrieved by GetFileInformationByHandle(). When compiled in configuration that supports FILE_ID_INFO, the code first tries to obtain 128-bit FILE_ID_INFO and if that fails, fallback to GetFileInformationByHandle(). Bug: 16059 Change-Id: I5f8d8d8127337891ef9907c291e550b1d17aabbb Reviewed-on: https://code.wireshark.org/review/34573 Reviewed-by: Peter Wu Petri-Dish: Peter Wu Tested-by: Petri Dish Buildbot Reviewed-by: Anders Broman --- debian/libwsutil0.symbols | 1 + epan/dissectors/packet-tls-utils.c | 20 --------- epan/dissectors/packet-wireguard.c | 22 +--------- wsutil/filesystem.c | 65 ++++++++++++++++++++++++++++++ wsutil/filesystem.h | 6 +++ 5 files changed, 73 insertions(+), 41 deletions(-) diff --git a/debian/libwsutil0.symbols b/debian/libwsutil0.symbols index 2e64ef3c18..ccbf06bd46 100644 --- a/debian/libwsutil0.symbols +++ b/debian/libwsutil0.symbols @@ -62,6 +62,7 @@ libwsutil.so.0 libwsutil0 #MINVER# delete_persconffile_profile@Base 1.12.0~rc1 deregister_codec@Base 3.1.0 file_exists@Base 1.12.0~rc1 + file_needs_reopen@Base 3.0.6 file_open_error_message@Base 1.12.0~rc1 file_write_error_message@Base 1.12.0~rc1 files_identical@Base 1.12.0~rc1 diff --git a/epan/dissectors/packet-tls-utils.c b/epan/dissectors/packet-tls-utils.c index 7253724e4e..c28f043940 100644 --- a/epan/dissectors/packet-tls-utils.c +++ b/epan/dissectors/packet-tls-utils.c @@ -5189,26 +5189,6 @@ ssl_compile_keyfile_regex(void) return regex; } -static gboolean -file_needs_reopen(FILE *fp, const char *filename) -{ - ws_statb64 open_stat, current_stat; - - /* consider a file deleted when stat fails for either file, - * or when the residing device / inode has changed. */ - if (0 != ws_fstat64(ws_fileno(fp), &open_stat)) - return TRUE; - if (0 != ws_stat64(filename, ¤t_stat)) - return TRUE; - - /* Note: on Windows, ino may be 0. Existing files cannot be deleted on - * Windows, but hopefully the size is a good indicator when a file got - * removed and recreated */ - return open_stat.st_dev != current_stat.st_dev || - open_stat.st_ino != current_stat.st_ino || - open_stat.st_size > current_stat.st_size; -} - typedef struct ssl_master_key_match_group { const char *re_group_name; GHashTable *master_key_ht; diff --git a/epan/dissectors/packet-wireguard.c b/epan/dissectors/packet-wireguard.c index bc236c2e9a..51e71b5ec3 100644 --- a/epan/dissectors/packet-wireguard.c +++ b/epan/dissectors/packet-wireguard.c @@ -26,6 +26,7 @@ #include #include #include +#include #include #include #include @@ -619,27 +620,6 @@ wg_psk_iter_next(wg_psk_iter_context *psk_iter, const wg_handshake_state_t *hs, /* PSK handling. }}} */ /* UAT and key configuration. {{{ */ -/* XXX this is copied verbatim from packet-tls-utils.c - create new common API - * for retrieval of runtime secrets? */ -static gboolean -file_needs_reopen(FILE *fp, const char *filename) -{ - ws_statb64 open_stat, current_stat; - - /* consider a file deleted when stat fails for either file, - * or when the residing device / inode has changed. */ - if (0 != ws_fstat64(ws_fileno(fp), &open_stat)) - return TRUE; - if (0 != ws_stat64(filename, ¤t_stat)) - return TRUE; - - /* Note: on Windows, ino may be 0. Existing files cannot be deleted on - * Windows, but hopefully the size is a good indicator when a file got - * removed and recreated */ - return open_stat.st_dev != current_stat.st_dev || - open_stat.st_ino != current_stat.st_ino || - open_stat.st_size > current_stat.st_size; -} static void wg_keylog_reset(void) diff --git a/wsutil/filesystem.c b/wsutil/filesystem.c index f56b4f0c48..8c60b41e8c 100644 --- a/wsutil/filesystem.c +++ b/wsutil/filesystem.c @@ -2123,6 +2123,71 @@ files_identical(const char *fname1, const char *fname2) #endif } +gboolean +file_needs_reopen(FILE* fp, const char* filename) +{ +#ifdef _WIN32 + /* Windows handles st_dev in a way unsuitable here: + * * _fstat() simply casts the file descriptor (ws_fileno(fp)) to unsigned + * and assigns this value to st_dev and st_rdev + * * _wstat() converts drive letter (eg. C) to number (A=0, B=1, C=2, ...) + * and assigns such number to st_dev and st_rdev + * + * The st_ino parameter is simply zero as there is no specific assignment + * to it in the Universal CRT source code. + * + * Thus instead of using fstat(), use Windows specific API. + */ + + HANDLE open_handle = (HANDLE)_get_osfhandle(ws_fileno(fp)); + HANDLE current_handle = CreateFile(utf_8to16(filename), FILE_READ_ATTRIBUTES, + FILE_SHARE_DELETE | FILE_SHARE_READ | FILE_SHARE_WRITE, + NULL, OPEN_EXISTING, 0, NULL); + BY_HANDLE_FILE_INFORMATION open_info, current_info; + + if (current_handle == INVALID_HANDLE_VALUE) { + return TRUE; + } + +#if (_WIN32_WINNT >= _WIN32_WINNT_WIN8) + FILE_ID_INFO open_id, current_id; + if (GetFileInformationByHandleEx(open_handle, FileIdInfo, &open_id, sizeof(open_id)) && + GetFileInformationByHandleEx(current_handle, FileIdInfo, ¤t_id, sizeof(current_id))) { + /* 128-bit identifier is available, use it */ + CloseHandle(current_handle); + return open_id.VolumeSerialNumber != current_id.VolumeSerialNumber || + memcmp(&open_id.FileId, ¤t_id.FileId, sizeof(open_id.FileId)) != 0; + } +#endif /* _WIN32_WINNT >= _WIN32_WINNT_WIN8 */ + if (GetFileInformationByHandle(open_handle, &open_info) && + GetFileInformationByHandle(current_handle, ¤t_info)) { + /* Fallback to 64-bit identifier */ + CloseHandle(current_handle); + guint64 open_size = (((guint64)open_info.nFileSizeHigh) << 32) | open_info.nFileSizeLow; + guint64 current_size = (((guint64)current_info.nFileSizeHigh) << 32) | current_info.nFileSizeLow; + return open_info.dwVolumeSerialNumber != current_info.dwVolumeSerialNumber || + open_info.nFileIndexHigh != current_info.nFileIndexHigh || + open_info.nFileIndexLow != current_info.nFileIndexLow || + open_size > current_size; + } + CloseHandle(current_handle); + return TRUE; +#else + ws_statb64 open_stat, current_stat; + + /* consider a file deleted when stat fails for either file, + * or when the residing device / inode has changed. */ + if (0 != ws_fstat64(ws_fileno(fp), &open_stat)) + return TRUE; + if (0 != ws_stat64(filename, ¤t_stat)) + return TRUE; + + return open_stat.st_dev != current_stat.st_dev || + open_stat.st_ino != current_stat.st_ino || + open_stat.st_size > current_stat.st_size; +#endif +} + /* * Copy a file in binary mode, for those operating systems that care about * such things. This should be OK for all files, even text files, as diff --git a/wsutil/filesystem.h b/wsutil/filesystem.h index 441586eab3..690c4a138a 100644 --- a/wsutil/filesystem.h +++ b/wsutil/filesystem.h @@ -11,6 +11,7 @@ #ifndef FILESYSTEM_H #define FILESYSTEM_H +#include #include "ws_symbol_export.h" #include "ws_attributes.h" @@ -302,6 +303,11 @@ WS_DLL_PUBLIC gboolean config_file_exists_with_entries(const char *fname, char c */ WS_DLL_PUBLIC gboolean files_identical(const char *fname1, const char *fname2); +/* + * Check if file has been recreated since it was opened. + */ +WS_DLL_PUBLIC gboolean file_needs_reopen(FILE* fp, const char* filename); + /* * Copy a file in binary mode, for those operating systems that care about * such things. This should be OK for all files, even text files, as