From 61d7c70aaf511967552702f22c27a41649c5ffe8 Mon Sep 17 00:00:00 2001 From: Juan Sebastian Hoyos Ayala Date: Mon, 12 Jun 2023 17:47:46 +0000 Subject: [PATCH 1/3] [release/6.0] Change NamedPipe creation flags --- src/native/eventpipe/ds-ipc-pal-namedpipe.c | 117 ++++++++++++++++++-- src/native/eventpipe/ds-ipc-pal-socket.c | 5 +- src/native/eventpipe/ds-ipc-pal.h | 2 +- src/native/eventpipe/ds-server.c | 2 +- 4 files changed, 115 insertions(+), 11 deletions(-) diff --git a/src/native/eventpipe/ds-ipc-pal-namedpipe.c b/src/native/eventpipe/ds-ipc-pal-namedpipe.c index 81a0499e56f948..1bd3a202277204 100644 --- a/src/native/eventpipe/ds-ipc-pal-namedpipe.c +++ b/src/native/eventpipe/ds-ipc-pal-namedpipe.c @@ -57,6 +57,8 @@ ep_rt_object_free (void *ptr) } #endif /* !FEATURE_PERFTRACING_STANDALONE_PAL */ +static HANDLE _ipc_listen_ownership_handle = INVALID_HANDLE_VALUE; + /* * Forward declares of all static functions. */ @@ -91,6 +93,18 @@ static bool ipc_stream_close_func (void *object); +static +void +ipc_close_ownership_handle ( + ds_ipc_error_callback_func callback); + +static +bool +ipc_createpipe_helper ( + DiagnosticsIpc *ipc, + bool ensure_pipe_creation, + ds_ipc_error_callback_func callback); + static DiagnosticsIpcStream * ipc_stream_alloc ( @@ -108,8 +122,9 @@ ds_ipc_pal_init (void) } bool -ds_ipc_pal_shutdown (void) +ds_ipc_pal_shutdown (ds_ipc_error_callback_func callback) { + ipc_close_ownership_handle(callback); return true; } @@ -330,9 +345,11 @@ ds_ipc_poll ( ep_exit_error_handler (); } +static bool -ds_ipc_listen ( +ipc_createpipe_helper ( DiagnosticsIpc *ipc, + bool ensure_pipe_creation, ds_ipc_error_callback_func callback) { bool result = false; @@ -348,16 +365,41 @@ ds_ipc_listen ( if (ipc->is_listening) return true; - EP_ASSERT (ipc->pipe == INVALID_HANDLE_VALUE); + if (!ensure_pipe_creation && _ipc_listen_ownership_handle == INVALID_HANDLE_VALUE) + { + if (callback) + callback ("Can't ensure we have ownership of the pipe. Disallowing creation.", -1); + return false; + } + + if (ensure_pipe_creation && _ipc_listen_ownership_handle != INVALID_HANDLE_VALUE) + { + if (callback) + callback ("Inconsistent state - pipe sentinel already in use for listen.", -1); + return false; + } + + EP_ASSERT (ipc->pipe == INVALID_HANDLE_VALUE); const uint32_t in_buffer_size = 16 * 1024; const uint32_t out_buffer_size = 16 * 1024; + DWORD creationFlags = PIPE_ACCESS_DUPLEX // read/write access + | FILE_FLAG_OVERLAPPED; // async listening. + + if (ensure_pipe_creation) + { + // Fail if we can't own pipe. This is the only way to ensure + // ownership of the pipe, and by extension the default DACL. + // Otherwise, Windows treats this as a FIFO queue get-or-create + // request and we might end up with DACLs set by other creators. + creationFlags |= FILE_FLAG_FIRST_PIPE_INSTANCE; + } + DS_ENTER_BLOCKING_PAL_SECTION; ipc->pipe = CreateNamedPipeA ( ipc->pipe_name, // pipe name - PIPE_ACCESS_DUPLEX | // read/write access - FILE_FLAG_OVERLAPPED, // async listening + creationFlags, PIPE_TYPE_BYTE | PIPE_WAIT | PIPE_REJECT_REMOTE_CLIENTS, // message type pipe, message-read and blocking mode PIPE_UNLIMITED_INSTANCES, // max. instances out_buffer_size, // output buffer size @@ -372,6 +414,28 @@ ds_ipc_listen ( ep_raise_error (); } + if (ensure_pipe_creation) + { + EP_ASSERT (_ipc_listen_ownership_handle == INVALID_HANDLE_VALUE); + + // The dupe and leak of the handle to ensure listen EP ownership for process duration. + bool createdSentinel = DuplicateHandle( + GetCurrentProcess(), + ipc->pipe, + GetCurrentProcess(), + &_ipc_listen_ownership_handle, + 0, + FALSE, + DUPLICATE_SAME_ACCESS); + + if (!createdSentinel) + { + if (callback) + callback ("Failed to ownership sentinel.", GetLastError()); + ep_raise_error(); + } + } + EP_ASSERT (ipc->overlap.hEvent == INVALID_HANDLE_VALUE); ipc->overlap.hEvent = CreateEventW (NULL, true, false, NULL); @@ -407,10 +471,23 @@ ds_ipc_listen ( ep_on_error: ds_ipc_close (ipc, false, callback); + if (ensure_pipe_creation) + ipc_close_ownership_handle(callback); result = false; ep_exit_error_handler (); } +bool +ds_ipc_listen ( + DiagnosticsIpc *ipc, + ds_ipc_error_callback_func callback) +{ + // This is the first time that this listening channel is created + // from the perspective of the runtime. Request we ensure that we create + // the pipe. + return ipc_createpipe_helper(ipc, true, callback); +} + DiagnosticsIpcStream * ds_ipc_accept ( DiagnosticsIpc *ipc, @@ -459,7 +536,10 @@ ds_ipc_accept ( memset(&ipc->overlap, 0, sizeof(OVERLAPPED)); // clear the overlapped objects state ipc->overlap.hEvent = INVALID_HANDLE_VALUE; - ep_raise_error_if_nok (ds_ipc_listen (ipc, callback)); + // At this point we have at least one open connection with this pipe, + // so this listen pipe won't recreate the named pipe and thus inherit + // all the necessary DACLs from the original listen call. + ep_raise_error_if_nok (ipc_createpipe_helper (ipc, false, callback)); ep_on_exit: return stream; @@ -526,6 +606,27 @@ ds_ipc_connect ( ep_exit_error_handler (); } +void +ipc_close_ownership_handle ( + ds_ipc_error_callback_func callback) +{ + if (_ipc_listen_ownership_handle == INVALID_HANDLE_VALUE) + return; + + const BOOL success_close_pipe = CloseHandle(_ipc_listen_ownership_handle); + if (success_close_pipe != TRUE) + { + if (callback) + callback ("Failed to IPC ownership sentinel handle", GetLastError()); + // Explicitly don't reset it. Failing to close and setting it to an invalid handle + // leaks the handle in a way we can't diagnose anything. Leaving it rooted helps us + // assert state consistency. + return; + } + + _ipc_listen_ownership_handle = INVALID_HANDLE_VALUE; +} + void ds_ipc_close ( DiagnosticsIpc *ipc, @@ -535,7 +636,9 @@ ds_ipc_close ( EP_ASSERT (ipc != NULL); // don't attempt cleanup on shutdown and let the OS handle it - if (is_shutdown) { + // except in the case of listen pipes - if they leak the process + // will fail to reinitialize the pipe for embedding scenarios. + if (is_shutdown && ipc->mode != DS_IPC_CONNECTION_MODE_LISTEN) { if (callback) callback ("Closing without cleaning underlying handles", 100); return; diff --git a/src/native/eventpipe/ds-ipc-pal-socket.c b/src/native/eventpipe/ds-ipc-pal-socket.c index 04b06451effffd..742de3f90a7aa2 100644 --- a/src/native/eventpipe/ds-ipc-pal-socket.c +++ b/src/native/eventpipe/ds-ipc-pal-socket.c @@ -1009,11 +1009,12 @@ ds_ipc_pal_init (void) } bool -ds_ipc_pal_shutdown (void) +ds_ipc_pal_shutdown (ds_ipc_error_callback_func callback) { #ifdef HOST_WIN32 if (_ipc_pal_socket_init) - WSACleanup (); + if (WSACleanup() == SOCKET_ERROR && callback) + callback ("Failed to cleanup Winsock", WSAGetLastError()); #endif _ipc_pal_socket_init = false; return true; diff --git a/src/native/eventpipe/ds-ipc-pal.h b/src/native/eventpipe/ds-ipc-pal.h index 98e0fba180e69b..8ffd202b6b5185 100644 --- a/src/native/eventpipe/ds-ipc-pal.h +++ b/src/native/eventpipe/ds-ipc-pal.h @@ -21,7 +21,7 @@ bool ds_ipc_pal_init (void); bool -ds_ipc_pal_shutdown (void); +ds_ipc_pal_shutdown (ds_ipc_error_callback_func callback); int32_t ds_ipc_get_handle_int32_t (DiagnosticsIpc *ipc); diff --git a/src/native/eventpipe/ds-server.c b/src/native/eventpipe/ds-server.c index 710b18465d7eb0..44714af7a9d7ab 100644 --- a/src/native/eventpipe/ds-server.c +++ b/src/native/eventpipe/ds-server.c @@ -247,7 +247,7 @@ ds_server_shutdown (void) ds_ipc_stream_factory_shutdown (server_error_callback_close); ds_ipc_stream_factory_fini (); - ds_ipc_pal_shutdown (); + ds_ipc_pal_shutdown (server_error_callback_close); return true; } From 07516cc6579e217675c7c873bd222897e903fee9 Mon Sep 17 00:00:00 2001 From: Jeff Handley Date: Tue, 20 Jun 2023 18:28:50 +0000 Subject: [PATCH 2/3] Merged PR 32087: Merged PR 32016: [6.0] Fix regression loading null-password encrypted PFX certificates When decrypting the payload with empty string and null passwords, try reading the payload with the Asn reader to ensure the header matches the expected format. If that succeeds, then proceed with the iteration counting. This guards against a false-positive match that previously caused our iteration count work to throw/abort, thus preventing some null-password encrypted payloads from being loaded. --- .../X509Certificates/Asn1/PfxAsn.manual.cs | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/src/libraries/System.Security.Cryptography.X509Certificates/src/System/Security/Cryptography/X509Certificates/Asn1/PfxAsn.manual.cs b/src/libraries/System.Security.Cryptography.X509Certificates/src/System/Security/Cryptography/X509Certificates/Asn1/PfxAsn.manual.cs index aba715f7e3e082..66173ae838cc70 100644 --- a/src/libraries/System.Security.Cryptography.X509Certificates/src/System/Security/Cryptography/X509Certificates/Asn1/PfxAsn.manual.cs +++ b/src/libraries/System.Security.Cryptography.X509Certificates/src/System/Security/Cryptography/X509Certificates/Asn1/PfxAsn.manual.cs @@ -168,6 +168,12 @@ private static ArraySegment DecryptContentInfo(ContentInfoAsn contentInfo, default, encryptedData.EncryptedContentInfo.EncryptedContent.Value.Span, destination); + + // When padding happens to be as expected (false-positive), we can detect gibberish and prevent unexpected failures later + // This extra check makes it so it's very unlikely we'll end up with false positive. + AsnValueReader outerSafeBag = new AsnValueReader(destination.AsSpan(0, written), AsnEncodingRules.BER); + AsnValueReader safeBagReader = outerSafeBag.ReadSequence(); + outerSafeBag.ThrowIfNotEmpty(); } catch { @@ -178,6 +184,12 @@ private static ArraySegment DecryptContentInfo(ContentInfoAsn contentInfo, default, encryptedData.EncryptedContentInfo.EncryptedContent.Value.Span, destination); + + // When padding happens to be as expected (false-positive), we can detect gibberish and prevent unexpected failures later + // This extra check makes it so it's very unlikely we'll end up with false positive. + AsnValueReader outerSafeBag = new AsnValueReader(destination.AsSpan(0, written), AsnEncodingRules.BER); + AsnValueReader safeBagReader = outerSafeBag.ReadSequence(); + outerSafeBag.ThrowIfNotEmpty(); } } finally From b870d77c19b176e5d015b829665f2f4aa329d6a0 Mon Sep 17 00:00:00 2001 From: "dotnet-maestro[bot]" <42748379+dotnet-maestro[bot]@users.noreply.github.com> Date: Mon, 10 Jul 2023 17:49:34 -0500 Subject: [PATCH 3/3] Update dependencies from https://github.com/dotnet/emsdk build 20230707.3 (#88535) Microsoft.NET.Workload.Emscripten.Manifest-6.0.100 , Microsoft.NET.Workload.Emscripten.Manifest-6.0.300 , Microsoft.NET.Workload.Emscripten.Manifest-6.0.400 From Version 6.0.20 -> To Version 6.0.21 Co-authored-by: dotnet-maestro[bot] --- NuGet.config | 2 +- eng/Version.Details.xml | 12 ++++++------ eng/Versions.props | 6 +++--- 3 files changed, 10 insertions(+), 10 deletions(-) diff --git a/NuGet.config b/NuGet.config index 90e21e18c5bffd..6544a77805caf6 100644 --- a/NuGet.config +++ b/NuGet.config @@ -9,7 +9,7 @@ - + diff --git a/eng/Version.Details.xml b/eng/Version.Details.xml index b64486cec2ab43..0575cae370e525 100644 --- a/eng/Version.Details.xml +++ b/eng/Version.Details.xml @@ -8,17 +8,17 @@ https://github.com/dotnet/msquic 7312355e44fd230b7aa26c7190f3870391751476 - + https://github.com/dotnet/emsdk - 6c7f38c614304c93cd392907df7d5e770d5cf7f1 + 864fae9e00da58498da19b40d46be706a9bf5f47 - + https://github.com/dotnet/emsdk - 6c7f38c614304c93cd392907df7d5e770d5cf7f1 + 864fae9e00da58498da19b40d46be706a9bf5f47 - + https://github.com/dotnet/emsdk - 6c7f38c614304c93cd392907df7d5e770d5cf7f1 + 864fae9e00da58498da19b40d46be706a9bf5f47 https://github.com/dotnet/wcf diff --git a/eng/Versions.props b/eng/Versions.props index 851a89e62004da..bf34af3e0dc50c 100644 --- a/eng/Versions.props +++ b/eng/Versions.props @@ -175,9 +175,9 @@ 11.1.0-alpha.1.21416.1 11.1.0-alpha.1.21416.1 - 6.0.20 - 6.0.20 - 6.0.20 + 6.0.21 + 6.0.21 + 6.0.21 $(MicrosoftNETWorkloadEmscriptenManifest60100Version) 1.1.87-gba258badda