Skip to content

feat: Add support for RSA private key (RsaPrivateCrtKeyParameters) TLS authentication with protected Docker daemon sockets#978

Merged
HofmeisterAn merged 5 commits into
testcontainers:developfrom
zuntio:feature/support-rsaprivatecrtkeyparameters-from-pem
Aug 28, 2023
Merged

feat: Add support for RSA private key (RsaPrivateCrtKeyParameters) TLS authentication with protected Docker daemon sockets#978
HofmeisterAn merged 5 commits into
testcontainers:developfrom
zuntio:feature/support-rsaprivatecrtkeyparameters-from-pem

Conversation

@zuntio

@zuntio zuntio commented Aug 18, 2023

Copy link
Copy Markdown
Contributor

What does this PR do?

Adds switch case to determine if read object is actually AsymmetricCipherKeyPair or RsaPrivateCrtKeyParameters instead of hard casting it to AsymmetricCipherKeyPair.

Why is it important?

Tutorial of protecting socket results in pem key type which resolves as RsaPrivateCrtKeyParameters object type. Same outputs come from many other tutorials too so it is reasonable to support it.

Related issues

How to test this PR

Configure TLS protection to docker daemon according to tutorial and write simple hello-world usage.

@netlify

netlify Bot commented Aug 18, 2023

Copy link
Copy Markdown

Deploy Preview for testcontainers-dotnet ready!

Name Link
🔨 Latest commit 9491afd
🔍 Latest deploy log https://app.netlify.com/sites/testcontainers-dotnet/deploys/64ecc5db985a67000851a8b6
😎 Deploy Preview https://deploy-preview-978--testcontainers-dotnet.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@HofmeisterAn HofmeisterAn left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the pull request 🙏. Do you think we can cover the changes with a test?

@zuntio

zuntio commented Aug 19, 2023

Copy link
Copy Markdown
Contributor Author

Thanks for the pull request 🙏. Do you think we can cover the changes with a test?

Seems that if I replace DockerVersion with newest 24.0.5, object is resolved as RsaPrivateCrtKeyParameters.

If you give me a green light I'll do some refactoring to ProtectDaemonSocket fixture to make version overrideable and add duplicate test with other version including some verification that different type of object was actually resolved?

Edit: verifying type seems a bit nasty since contents of X509Certificate2 are pretty similar between runs with different versions. Only length of raw data byte array differs and not propably goot property to match :)

@HofmeisterAn

Copy link
Copy Markdown
Collaborator

Seems that if I replace DockerVersion with newest 24.0.5, object is resolved as RsaPrivateCrtKeyParameters.

Ah, that is fortunate.

If you give me a green light I'll do some refactoring to ProtectDaemonSocket fixture to make version overrideable and add duplicate test with other version including some verification that different type of object was actually resolved?

Of course, certainly 👍. Couldn't we just verify if the generated key (inside _hostCertsDirectoryPath) matches the type we expect? This won't validate our internal resolution, but I think it's fine as long as we confirm that both types are working and not throwing an exception.

@zuntio

zuntio commented Aug 19, 2023

Copy link
Copy Markdown
Contributor Author

Couldn't we just verify if the generated key (inside _hostCertsDirectoryPath) matches the type we expect? This won't validate our internal resolution, but I think it's fine as long as we confirm that both types are working and not throwing an exception.

Sounds good. I'll write and commit it tomorrow along with fixture refac.

@HofmeisterAn

Copy link
Copy Markdown
Collaborator

Great, take your time. There's no rush at all. Enjoy the rest of the weekend 🥳.

@zuntio

zuntio commented Aug 21, 2023

Copy link
Copy Markdown
Contributor Author

Here you go. Lot of patterns to go with but decided to use IClassFixture pattern. Naming is bit off after realising this is all from IETF TLS deprecation and therefore OpenSSL has been updated at some point. Of course open for suggestions.

@zuntio zuntio force-pushed the feature/support-rsaprivatecrtkeyparameters-from-pem branch from 468f516 to c0a568e Compare August 21, 2023 19:21
@HofmeisterAn

Copy link
Copy Markdown
Collaborator

Thank you for making the changes. For your information: I will be able to do the review earliest by the end of this week (possibly at the beginning of next week).

@HofmeisterAn HofmeisterAn changed the title feat: support RsaPrivateCrtKeyParameters being resolved from key pem feat: Add support for RSA private key (RsaPrivateCrtKeyParameters) TLS authentication with protected Docker daemon sockets Aug 28, 2023
@HofmeisterAn HofmeisterAn merged commit b121dde into testcontainers:develop Aug 28, 2023
@HofmeisterAn

Copy link
Copy Markdown
Collaborator

Thanks again. PR looks good.

@HofmeisterAn HofmeisterAn added the enhancement New feature or request label Aug 28, 2023
@safalin1

Copy link
Copy Markdown

Just tested it, working great now. Thanks guys!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: InvalidCastException when running with docker with TLS

3 participants