{"action":"created","comment":{"url":"https://api.github.com/repos/dotnet/corefx/pulls/comments/83229174","pull_request_review_id":4085420,"id":83229174,"diff_hunk":"@@ -98,46 +99,81 @@ public ECDsa GetECDsaPrivateKey()\n             }\n         }\n \n-        private static SafeNCryptKeyHandle TryAcquireCngPrivateKey(SafeCertContextHandle certificateContext)\n+        private static SafeNCryptKeyHandle TryAcquireCngPrivateKey(\n+            SafeCertContextHandle certificateContext,\n+            out CngKeyHandleOpenOptions handleOptions)\n         {\n             Debug.Assert(certificateContext != null, \"certificateContext != null\");\n             Debug.Assert(!certificateContext.IsClosed && !certificateContext.IsInvalid,\n                          \"!certificateContext.IsClosed && !certificateContext.IsInvalid\");\n \n+            IntPtr privateKeyPtr;\n+\n+            // If the certificate has a key handle instead of a key prov info, return the\n+            // ephemeral key\n+            {\n+                int cbData = IntPtr.Size;\n+\n+                if (Interop.crypt32.CertGetCertificateContextProperty(\n+                    certificateContext,\n+                    CertContextPropId.CERT_NCRYPT_KEY_HANDLE_PROP_ID,\n+                    out privateKeyPtr,\n+                    ref cbData))\n+                {\n+                    handleOptions = CngKeyHandleOpenOptions.EphemeralKey;\n+                    return new SafeNCryptKeyHandle(privateKeyPtr, certificateContext);\n+                }\n+            }\n+\n             bool freeKey = true;\n             SafeNCryptKeyHandle privateKey = null;\n+            handleOptions = CngKeyHandleOpenOptions.None;\n             try\n             {\n                 int keySpec = 0;\n                 if (!Interop.crypt32.CryptAcquireCertificatePrivateKey(\n-                        certificateContext,\n-                        CryptAcquireFlags.CRYPT_ACQUIRE_ONLY_NCRYPT_KEY_FLAG,\n-                        IntPtr.Zero,\n-                        out privateKey,\n-                        out keySpec,\n-                        out freeKey))\n+                    certificateContext,\n+                    CryptAcquireFlags.CRYPT_ACQUIRE_ONLY_NCRYPT_KEY_FLAG,\n+                    IntPtr.Zero,\n+                    out privateKey,\n+                    out keySpec,\n+                    out freeKey))\n                 {\n                     int dwErrorCode = Marshal.GetLastWin32Error();\n+\n+                    // The documentation for CryptAcquireCertificatePrivateKey says that freeKey\n+                    // should already be false if \"key acquisition fails\", and it can be presumed\n+                    // that privateKey was set to 0.  But, just in case:\n+                    freeKey = false;\n+                    privateKey?.SetHandleAsInvalid();\n                     return null;\n                 }\n \n+                // It is very unlikely that Windows will tell us !freeKey other than when reporting failure,\n+                // because we set neither CRYPT_ACQUIRE_CACHE_FLAG nor CRYPT_ACQUIRE_USE_PROV_INFO_FLAG, which are\n+                // currently the only two success situations documented. However, any !freeKey response means the\n+                // key's lifetime is tied to that of the certificate, so re-register the handle as a child handle\n+                // of the certificate.\n+                if (!freeKey && privateKey != null && !privateKey.IsInvalid)\n+                {\n+                    var newKeyHandle = new SafeNCryptKeyHandle(privateKey.DangerousGetHandle(), certificateContext);\n+                    privateKey.SetHandleAsInvalid();\n+                    privateKey = newKeyHandle;\n+                    freeKey = true;\n+                }\n+\n                 return privateKey;\n             }\n-            finally\n+            catch","path":"src/System.Security.Cryptography.X509Certificates/src/Internal/Cryptography/Pal.Windows/CertificatePal.PrivateKey.cs","position":91,"original_position":91,"commit_id":"dcd5d84d945fbf066834a275cf7a9b854d548a73","original_commit_id":"dcd5d84d945fbf066834a275cf7a9b854d548a73","user":{"login":"steveharter","id":6986755,"avatar_url":"https://avatars.githubusercontent.com/u/6986755?v=3","gravatar_id":"","url":"https://api.github.com/users/steveharter","html_url":"https://github.com/steveharter","followers_url":"https://api.github.com/users/steveharter/followers","following_url":"https://api.github.com/users/steveharter/following{/other_user}","gists_url":"https://api.github.com/users/steveharter/gists{/gist_id}","starred_url":"https://api.github.com/users/steveharter/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/steveharter/subscriptions","organizations_url":"https://api.github.com/users/steveharter/orgs","repos_url":"https://api.github.com/users/steveharter/repos","events_url":"https://api.github.com/users/steveharter/events{/privacy}","received_events_url":"https://api.github.com/users/steveharter/received_events","type":"User","site_admin":false},"body":"When adding the copy ctor I noticed this finally block and thought it could be a catch. However, I found in certain cases (IIRC when the context was invalid\\freed) the above could fail with error code (no exception) but still fill in the privateKey causing the finally block to bump up the reference count. I assumed this was by design (same in desktop)","created_at":"2016-10-13T14:29:54Z","updated_at":"2016-10-13T14:29:54Z","html_url":"https://github.com/dotnet/corefx/pull/12610#discussion_r83229174","pull_request_url":"https://api.github.com/repos/dotnet/corefx/pulls/12610","_links":{"self":{"href":"https://api.github.com/repos/dotnet/corefx/pulls/comments/83229174"},"html":{"href":"https://github.com/dotnet/corefx/pull/12610#discussion_r83229174"},"pull_request":{"href":"https://api.github.com/repos/dotnet/corefx/pulls/12610"}}},"pull_request":{"url":"https://api.github.com/repos/dotnet/corefx/pulls/12610","id":89131950,"html_url":"https://github.com/dotnet/corefx/pull/12610","diff_url":"https://github.com/dotnet/corefx/pull/12610.diff","patch_url":"https://github.com/dotnet/corefx/pull/12610.patch","issue_url":"https://api.github.com/repos/dotnet/corefx/issues/12610","number":12610,"state":"open","locked":false,"title":"Enable ephemeral import of private keys from a PFX on Windows","user":{"login":"bartonjs","id":10642668,"avatar_url":"https://avatars.githubusercontent.com/u/10642668?v=3","gravatar_id":"","url":"https://api.github.com/users/bartonjs","html_url":"https://github.com/bartonjs","followers_url":"https://api.github.com/users/bartonjs/followers","following_url":"https://api.github.com/users/bartonjs/following{/other_user}","gists_url":"https://api.github.com/users/bartonjs/gists{/gist_id}","starred_url":"https://api.github.com/users/bartonjs/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/bartonjs/subscriptions","organizations_url":"https://api.github.com/users/bartonjs/orgs","repos_url":"https://api.github.com/users/bartonjs/repos","events_url":"https://api.github.com/users/bartonjs/events{/privacy}","received_events_url":"https://api.github.com/users/bartonjs/received_events","type":"User","site_admin":false},"body":"The classic behavior on Windows is when importing a PFX the private key\r\nmaterial gets written to a file on disk (as an implementation detail of the\r\nsoftware key provider), and the .NET behavior is to delete that private key\r\nwhen it is detected as no longer being needed (unless the import was\r\nperformed with a request to persist the key material, such as when loading\r\nthe certificate into a persisted store).\r\n\r\nThis change enables callers to request an import with the new\r\nEphemeralKeySet flag, which uses the PKCS12_NO_PERSIST_KEY flag to\r\nPFXImportCertStore.  For callers who do not do any interop with Win32\r\ndirectly everything should be invisible and work. Libraries which pass\r\nPCERT_CONTEXT values to Win32, or replicate the work of\r\nGetRSAPrivateKey, may not function as expected when passed an\r\nX509Certificate(2) object with an ephemeral key.  This situation is already\r\npossible if someone were to have done a P/Invoke into PFXImportCertStore\r\nmanually and constructed an X509Certificate(2) object over the pointer,\r\nthe addition of this functionality just makes it easier to get into that state.\r\nSince ephemeral is not the default behavior the onus of compatibility rests\r\nwith the code which specified the flag on the call to load the PFX.\r\n\r\nAt this point the Windows implementation of EnvelopedCms is known to\r\nnot work with ephemeral private keys, and that will be addressed in a later\r\nchange.\r\n\r\nThis feature has no impact on Linux builds, since the private keys there\r\nare always ephemeral.\r\n\r\nBefore this change callers to X509Certificate2Collection.Import could\r\nspecify unknown flags without an error, but due to Ephemeral and\r\nPersisted being incompatible flags the Import method has been aligned\r\nwith the X509Certificate(2) constructors in terms of keyStorageFlag\r\nvalidation.\r\n\r\nChange 2/3 for #8186.\r\ncc: @stephentoub @steveharter @AtsushiKan \r\ncc for packaging/contracts: @ericstj ","created_at":"2016-10-13T03:39:15Z","updated_at":"2016-10-13T14:29:54Z","closed_at":null,"merged_at":null,"merge_commit_sha":"8156935d33f5112f61f7cce7715fdb4c97ec4fbf","assignee":null,"assignees":[],"milestone":null,"commits_url":"https://api.github.com/repos/dotnet/corefx/pulls/12610/commits","review_comments_url":"https://api.github.com/repos/dotnet/corefx/pulls/12610/comments","review_comment_url":"https://api.github.com/repos/dotnet/corefx/pulls/comments{/number}","comments_url":"https://api.github.com/repos/dotnet/corefx/issues/12610/comments","statuses_url":"https://api.github.com/repos/dotnet/corefx/statuses/dcd5d84d945fbf066834a275cf7a9b854d548a73","head":{"label":"bartonjs:ephemeral_certs_2","ref":"ephemeral_certs_2","sha":"dcd5d84d945fbf066834a275cf7a9b854d548a73","user":{"login":"bartonjs","id":10642668,"avatar_url":"https://avatars.githubusercontent.com/u/10642668?v=3","gravatar_id":"","url":"https://api.github.com/users/bartonjs","html_url":"https://github.com/bartonjs","followers_url":"https://api.github.com/users/bartonjs/followers","following_url":"https://api.github.com/users/bartonjs/following{/other_user}","gists_url":"https://api.github.com/users/bartonjs/gists{/gist_id}","starred_url":"https://api.github.com/users/bartonjs/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/bartonjs/subscriptions","organizations_url":"https://api.github.com/users/bartonjs/orgs","repos_url":"https://api.github.com/users/bartonjs/repos","events_url":"https://api.github.com/users/bartonjs/events{/privacy}","received_events_url":"https://api.github.com/users/bartonjs/received_events","type":"User","site_admin":false},"repo":{"id":29888389,"name":"corefx","full_name":"bartonjs/corefx","owner":{"login":"bartonjs","id":10642668,"avatar_url":"https://avatars.githubusercontent.com/u/10642668?v=3","gravatar_id":"","url":"https://api.github.com/users/bartonjs","html_url":"https://github.com/bartonjs","followers_url":"https://api.github.com/users/bartonjs/followers","following_url":"https://api.github.com/users/bartonjs/following{/other_user}","gists_url":"https://api.github.com/users/bartonjs/gists{/gist_id}","starred_url":"https://api.github.com/users/bartonjs/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/bartonjs/subscriptions","organizations_url":"https://api.github.com/users/bartonjs/orgs","repos_url":"https://api.github.com/users/bartonjs/repos","events_url":"https://api.github.com/users/bartonjs/events{/privacy}","received_events_url":"https://api.github.com/users/bartonjs/received_events","type":"User","site_admin":false},"private":false,"html_url":"https://github.com/bartonjs/corefx","description":"This repository contains the foundational libraries that make up the .NET Core development stack.","fork":true,"url":"https://api.github.com/repos/bartonjs/corefx","forks_url":"https://api.github.com/repos/bartonjs/corefx/forks","keys_url":"https://api.github.com/repos/bartonjs/corefx/keys{/key_id}","collaborators_url":"https://api.github.com/repos/bartonjs/corefx/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/bartonjs/corefx/teams","hooks_url":"https://api.github.com/repos/bartonjs/corefx/hooks","issue_events_url":"https://api.github.com/repos/bartonjs/corefx/issues/events{/number}","events_url":"https://api.github.com/repos/bartonjs/corefx/events","assignees_url":"https://api.github.com/repos/bartonjs/corefx/assignees{/user}","branches_url":"https://api.github.com/repos/bartonjs/corefx/branches{/branch}","tags_url":"https://api.github.com/repos/bartonjs/corefx/tags","blobs_url":"https://api.github.com/repos/bartonjs/corefx/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/bartonjs/corefx/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/bartonjs/corefx/git/refs{/sha}","trees_url":"https://api.github.com/repos/bartonjs/corefx/git/trees{/sha}","statuses_url":"https://api.github.com/repos/bartonjs/corefx/statuses/{sha}","languages_url":"https://api.github.com/repos/bartonjs/corefx/languages","stargazers_url":"https://api.github.com/repos/bartonjs/corefx/stargazers","contributors_url":"https://api.github.com/repos/bartonjs/corefx/contributors","subscribers_url":"https://api.github.com/repos/bartonjs/corefx/subscribers","subscription_url":"https://api.github.com/repos/bartonjs/corefx/subscription","commits_url":"https://api.github.com/repos/bartonjs/corefx/commits{/sha}","git_commits_url":"https://api.github.com/repos/bartonjs/corefx/git/commits{/sha}","comments_url":"https://api.github.com/repos/bartonjs/corefx/comments{/number}","issue_comment_url":"https://api.github.com/repos/bartonjs/corefx/issues/comments{/number}","contents_url":"https://api.github.com/repos/bartonjs/corefx/contents/{+path}","compare_url":"https://api.github.com/repos/bartonjs/corefx/compare/{base}...{head}","merges_url":"https://api.github.com/repos/bartonjs/corefx/merges","archive_url":"https://api.github.com/repos/bartonjs/corefx/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/bartonjs/corefx/downloads","issues_url":"https://api.github.com/repos/bartonjs/corefx/issues{/number}","pulls_url":"https://api.github.com/repos/bartonjs/corefx/pulls{/number}","milestones_url":"https://api.github.com/repos/bartonjs/corefx/milestones{/number}","notifications_url":"https://api.github.com/repos/bartonjs/corefx/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/bartonjs/corefx/labels{/name}","releases_url":"https://api.github.com/repos/bartonjs/corefx/releases{/id}","deployments_url":"https://api.github.com/repos/bartonjs/corefx/deployments","created_at":"2015-01-26T23:55:53Z","updated_at":"2016-01-18T16:14:34Z","pushed_at":"2016-10-13T03:35:40Z","git_url":"git://github.com/bartonjs/corefx.git","ssh_url":"git@github.com:bartonjs/corefx.git","clone_url":"https://github.com/bartonjs/corefx.git","svn_url":"https://github.com/bartonjs/corefx","homepage":"","size":105588,"stargazers_count":0,"watchers_count":0,"language":"C#","has_issues":false,"has_downloads":true,"has_wiki":true,"has_pages":false,"forks_count":0,"mirror_url":null,"open_issues_count":0,"forks":0,"open_issues":0,"watchers":0,"default_branch":"master"}},"base":{"label":"dotnet:master","ref":"master","sha":"de8eee4b26f5154698fddcd150a4c7487a69da64","user":{"login":"dotnet","id":9141961,"avatar_url":"https://avatars.githubusercontent.com/u/9141961?v=3","gravatar_id":"","url":"https://api.github.com/users/dotnet","html_url":"https://github.com/dotnet","followers_url":"https://api.github.com/users/dotnet/followers","following_url":"https://api.github.com/users/dotnet/following{/other_user}","gists_url":"https://api.github.com/users/dotnet/gists{/gist_id}","starred_url":"https://api.github.com/users/dotnet/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/dotnet/subscriptions","organizations_url":"https://api.github.com/users/dotnet/orgs","repos_url":"https://api.github.com/users/dotnet/repos","events_url":"https://api.github.com/users/dotnet/events{/privacy}","received_events_url":"https://api.github.com/users/dotnet/received_events","type":"Organization","site_admin":false},"repo":{"id":26295345,"name":"corefx","full_name":"dotnet/corefx","owner":{"login":"dotnet","id":9141961,"avatar_url":"https://avatars.githubusercontent.com/u/9141961?v=3","gravatar_id":"","url":"https://api.github.com/users/dotnet","html_url":"https://github.com/dotnet","followers_url":"https://api.github.com/users/dotnet/followers","following_url":"https://api.github.com/users/dotnet/following{/other_user}","gists_url":"https://api.github.com/users/dotnet/gists{/gist_id}","starred_url":"https://api.github.com/users/dotnet/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/dotnet/subscriptions","organizations_url":"https://api.github.com/users/dotnet/orgs","repos_url":"https://api.github.com/users/dotnet/repos","events_url":"https://api.github.com/users/dotnet/events{/privacy}","received_events_url":"https://api.github.com/users/dotnet/received_events","type":"Organization","site_admin":false},"private":false,"html_url":"https://github.com/dotnet/corefx","description":"This repo contains the .NET Core foundational libraries, called CoreFX. It includes classes for collections, file systems, console, XML, async and many others.","fork":false,"url":"https://api.github.com/repos/dotnet/corefx","forks_url":"https://api.github.com/repos/dotnet/corefx/forks","keys_url":"https://api.github.com/repos/dotnet/corefx/keys{/key_id}","collaborators_url":"https://api.github.com/repos/dotnet/corefx/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/dotnet/corefx/teams","hooks_url":"https://api.github.com/repos/dotnet/corefx/hooks","issue_events_url":"https://api.github.com/repos/dotnet/corefx/issues/events{/number}","events_url":"https://api.github.com/repos/dotnet/corefx/events","assignees_url":"https://api.github.com/repos/dotnet/corefx/assignees{/user}","branches_url":"https://api.github.com/repos/dotnet/corefx/branches{/branch}","tags_url":"https://api.github.com/repos/dotnet/corefx/tags","blobs_url":"https://api.github.com/repos/dotnet/corefx/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/dotnet/corefx/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/dotnet/corefx/git/refs{/sha}","trees_url":"https://api.github.com/repos/dotnet/corefx/git/trees{/sha}","statuses_url":"https://api.github.com/repos/dotnet/corefx/statuses/{sha}","languages_url":"https://api.github.com/repos/dotnet/corefx/languages","stargazers_url":"https://api.github.com/repos/dotnet/corefx/stargazers","contributors_url":"https://api.github.com/repos/dotnet/corefx/contributors","subscribers_url":"https://api.github.com/repos/dotnet/corefx/subscribers","subscription_url":"https://api.github.com/repos/dotnet/corefx/subscription","commits_url":"https://api.github.com/repos/dotnet/corefx/commits{/sha}","git_commits_url":"https://api.github.com/repos/dotnet/corefx/git/commits{/sha}","comments_url":"https://api.github.com/repos/dotnet/corefx/comments{/number}","issue_comment_url":"https://api.github.com/repos/dotnet/corefx/issues/comments{/number}","contents_url":"https://api.github.com/repos/dotnet/corefx/contents/{+path}","compare_url":"https://api.github.com/repos/dotnet/corefx/compare/{base}...{head}","merges_url":"https://api.github.com/repos/dotnet/corefx/merges","archive_url":"https://api.github.com/repos/dotnet/corefx/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/dotnet/corefx/downloads","issues_url":"https://api.github.com/repos/dotnet/corefx/issues{/number}","pulls_url":"https://api.github.com/repos/dotnet/corefx/pulls{/number}","milestones_url":"https://api.github.com/repos/dotnet/corefx/milestones{/number}","notifications_url":"https://api.github.com/repos/dotnet/corefx/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/dotnet/corefx/labels{/name}","releases_url":"https://api.github.com/repos/dotnet/corefx/releases{/id}","deployments_url":"https://api.github.com/repos/dotnet/corefx/deployments","created_at":"2014-11-06T23:42:48Z","updated_at":"2016-10-13T14:23:59Z","pushed_at":"2016-10-13T13:44:05Z","git_url":"git://github.com/dotnet/corefx.git","ssh_url":"git@github.com:dotnet/corefx.git","clone_url":"https://github.com/dotnet/corefx.git","svn_url":"https://github.com/dotnet/corefx","homepage":"https://docs.microsoft.com/dotnet/","size":150913,"stargazers_count":10644,"watchers_count":10644,"language":"C#","has_issues":true,"has_downloads":true,"has_wiki":true,"has_pages":false,"forks_count":2674,"mirror_url":null,"open_issues_count":1487,"forks":2674,"open_issues":1487,"watchers":10644,"default_branch":"master"}},"_links":{"self":{"href":"https://api.github.com/repos/dotnet/corefx/pulls/12610"},"html":{"href":"https://github.com/dotnet/corefx/pull/12610"},"issue":{"href":"https://api.github.com/repos/dotnet/corefx/issues/12610"},"comments":{"href":"https://api.github.com/repos/dotnet/corefx/issues/12610/comments"},"review_comments":{"href":"https://api.github.com/repos/dotnet/corefx/pulls/12610/comments"},"review_comment":{"href":"https://api.github.com/repos/dotnet/corefx/pulls/comments{/number}"},"commits":{"href":"https://api.github.com/repos/dotnet/corefx/pulls/12610/commits"},"statuses":{"href":"https://api.github.com/repos/dotnet/corefx/statuses/dcd5d84d945fbf066834a275cf7a9b854d548a73"}}}}
{"actor":{"display_login":"steveharter"}}