Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix Windows construction of identity cert PKCS#12 blob to not leak keys to disk. #4854

Merged
merged 2 commits into from
Apr 20, 2021

Conversation

arsing
Copy link
Member

@arsing arsing commented Apr 15, 2021

schannel and native-tls require a PKCS#12 blob of the device ID certificate
to use it as a TLS client cert. Before this change, the blob was created
using openssl even on Windows. However, with such a blob, schannel creates
the private key on disk under the user's persisted keys directory.
Since iotedged runs as LocalSystem, this was
C:\ProgramData\Microsoft\Crypto\RSA\S-1-5-18

It's empirically known that schannel doesn't create the disk file if
the private key is marked as "exportable". Since "exportable" is
a Windows-specific attribute, openssl doesn't have any way to set it.
So this commit uses winapi to create the PKCS#12 blob instead.

Other changes:

  • Revert two uses of single-threaded tokio runtime back to multi-threaded.
    deb2753 accidentally changed these while
    changing the tests.

…ys to disk.

schannel and native-tls require a PKCS#12 blob of the device ID certificate
to use it as a TLS client cert. Before this change, the blob was created
using openssl even on Windows. However, with such a blob, schannel creates
the private key on disk under the user's persisted keys directory.
Since iotedged runs as LocalSystem, this was
C:\ProgramData\Microsoft\Crypto\RSA\S-1-5-18

It's empirically known that schannel doesn't create the disk file if
the private key is marked as "exportable". Since "exportable" is
a Windows-specific attribute, openssl doesn't have any way to set it.
So this commit uses winapi to create the PKCS#12 blob instead.

Other changes:

- Revert two uses of single-threaded tokio runtime back to multi-threaded.
  deb2753 accidentally changed these while
  changing the tests.
@arsing arsing force-pushed the windows-pkcs12 branch 5 times, most recently from 48ce437 to 80fa568 Compare April 20, 2021 09:03
@arsing arsing marked this pull request as ready for review April 20, 2021 09:04
gordonwang0
gordonwang0 previously approved these changes Apr 20, 2021
@kodiakhq kodiakhq bot merged commit a46838f into Azure:release/1.1 Apr 20, 2021
@arsing arsing deleted the windows-pkcs12 branch April 21, 2021 07:15
darobs pushed a commit to darobs/iotedge that referenced this pull request Apr 21, 2021
…eys to disk. (Azure#4854)

schannel and native-tls require a PKCS#12 blob of the device ID certificate
to use it as a TLS client cert. Before this change, the blob was created
using openssl even on Windows. However, with such a blob, schannel creates
the private key on disk under the user's persisted keys directory.
Since iotedged runs as LocalSystem, this was
C:\ProgramData\Microsoft\Crypto\RSA\S-1-5-18

It's empirically known that schannel doesn't create the disk file if
the private key is marked as "exportable". Since "exportable" is
a Windows-specific attribute, openssl doesn't have any way to set it.
So this commit uses winapi to create the PKCS#12 blob instead.

Other changes:

- Revert two uses of single-threaded tokio runtime back to multi-threaded.
  deb2753 accidentally changed these while
  changing the tests.
kodiakhq bot pushed a commit that referenced this pull request Apr 30, 2021
Includes 1.1.2 and all commits as of the time of this PR:

Add EFLOW Job to Long Haul Test Pipeline (#4571)
Test Dashboard: Allow longhaul / connectivity pipelines to show with new dashboard queries (#4695)
Update ARM Base Image Security Patch (#4808)
Prepare for Release 1.1.2
Longhaul: Update snitcher linux images (#4843)
Fix ValidateMetrics on ARM32 (#4864)
Update C SDK submodules to 2020-12-09. (#4867)
Fix Windows construction of identity cert PKCS#12 blob to not leak keys to disk. (#4854)
Delete all containers after reprovision (#4833)
Remove Debian 8 build/package logic (#4876)
Bump serde-yaml version to 0.8 (#4860)
Allow release image pipeline to use a custom CR (#3507) (#4882)
Migrate Azure Monitor project to our repository. (#4877)
Metrics transform option (#4893)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants