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

Suport Docker TLS endpoint with client certificate authentication #370

Closed
prskr opened this issue Mar 5, 2021 · 16 comments
Closed

Suport Docker TLS endpoint with client certificate authentication #370

prskr opened this issue Mar 5, 2021 · 16 comments
Assignees
Labels
enhancement New feature or request help wanted Extra attention is needed
Milestone

Comments

@prskr
Copy link

prskr commented Mar 5, 2021

Is your feature request related to a problem? Please describe.

GitLab CI allows to use Docker-in-Docker (DinD) to execute various Docker related tasks.
The DinD configuration within the public GitLab CI enforces the usage of TLS to connect to the Docker daemon which is apparently not supported by dotnet-testconainers.

Describe the solution you'd like

It would be nice to either have the option to further configure the DockerClientConfiguration e.g. with a Lambda like it's quite common with ASP.NET Core to be able to set e.g. credentials or to expose a WithDockerEnvEndpoint method that takes the common environment variables:

  • DOCKER_HOST
  • DOCKER_TLS_VERIFY
  • DOCKER_TLS_CERTDIR/DOCKER_CERT_PATH

into account and configures the Docker client accordingly.

Describe alternatives you've considered

Setting the DockerEndpoint to something like https://docker:2376 allows to connect to the API via TLS already but I couldn't manage to use the client certs to authorize myself.
There are probably other options e.g. with a nginx proxy between the client and the API but I don't consider them actually very practical.
It would also be sufficient to be able to pass in a pre-configured Docker client or register a pre-configured client previously.

Additional context

The GitLab CI docs might also be interesting for further details, paths, etc.

Of course I'd be glad to create a PR to implement the required changes as soon as you confirmed that this request is valid and the design how to implement it is settled!

@HofmeisterAn
Copy link
Collaborator

👍 I agree, it makes a lot of sense to support client certificate authentication. The implementation is not that difficult, I guess. I suggest the following changes:

  1. Add an overloaded WithDockerEndpoint that gets the necessary parameters. Pass them along TestcontainersClient to DockerApiClient and create the DockerClient instance including the credentials.
  2. If any of the environment variables DOCKER_HOST, DOCKER_TLS_VERIFY, or DOCKER_TLS_CERTDIR exists, apply them as a default value in TestcontainersBuilder. This will make an explicit call of WithDockerEndpoint unnecessary in the future.

What do you think? Would you like to contribute and create a pull request? Otherwise, I can probably take a look into it at the end of next week, but I need at least someone to test it.

@HofmeisterAn HofmeisterAn added the enhancement New feature or request label Mar 6, 2021
@prskr
Copy link
Author

prskr commented Mar 7, 2021

Sounds reasonable!

I'd like to give it a try and create a PR following the suggestion you made.

I'm not sure how fast I can provide the complete PR but I think as I am the only who depends on it right now there's no reason to hurry 😅

I already had a look at your contribution guide, I'll try to stick to it as good as I can 😊

@prskr
Copy link
Author

prskr commented Mar 8, 2021

Sorry to bother you but I think there are some details worthwhile to discuss:

First I have to add a dependency to Docker.DotNet.X509 to be able to create the required certificate credentials inferred from the environment variables like you proposed it. Is that okay?

Second: reading PEM files in .NET is way more cumbersome than I expected. It's better in .NET 5 but for now I thought it might be okay to assume the certs are RSA based because it's the default for docker:dind. It would be possible to handle it 100% correct but it might blow the complexity even more (requires to parse the labels in the PEM files, like described here). I'm also wondering if you might want to ship public utility methods for users to load their keys easier when the logic is already present.
At first I thought it's best to stick to the abstract Credentials class coming from the Docker client library but as it turns out, it's also not that convenient.
Speaking of utility methods, I'm not sure where to put the cert loading stuff in the current project structure. Might also depend a bit on how you would like to have the whole thing implemented.

You might have seen the WIP PR, probably you would like to give me some early feedback what you think so that I'm able to clean up and align everything according to your structure?

It's not urgent I just thought this might be easier and less stressful than a huge review afterwards.

@HofmeisterAn
Copy link
Collaborator

I haven't seen your comment - sorry.

First I have to add a dependency to Docker.DotNet.X509 to be able to create the required certificate credentials inferred from the environment variables like you proposed it. Is that okay?

Ofc.

Second: reading PEM files in .NET is way more cumbersome than I expected.

The .NET Client for Docker Remote API suggest to use a .pfx file (PKCS12 format), see Example: HTTPS Authentication to Docker. This should make the pem file obsolete.

Speaking of utility methods, I'm not sure where to put the cert loading stuff in the current project structure. Might also depend a bit on how you would like to have the whole thing implemented.

Shouldn't be necessary with .pfx, right?

It's not urgent I just thought this might be easier and less stressful than a huge review afterwards.

Sure, I pushed some suggestions to your fork.

@prskr
Copy link
Author

prskr commented Mar 13, 2021

😄 sure they suggest to use .pfx files because they have very good support for .pfx but .NET Core just got support for .pem in .NET 5 but unfortunately Docker does not support .pfx directly you have to convert the .pfx to a pair of PEM files (see docs) to use them with the daemon and DinD does automatically create certs for the daemon but also only in PEM format and I'm not sure how many users are keen on converting the default certs in the CI first to a .pfx file e.g. with OpenSSL before they are able to use testcontainers for their tests 😅

I'm wondering if it's not easier to rather accept a X509Certificate2 for client auth and to allow optionally a RemoteCertificateValidationCallback for server cert validation and provide some examples on how to load the cert from different sources than supporting various certificate formats (just like the original Docker client API did it - most likely for the same reasons 😅 ).

This would even allow most users to use the included convenience wrappers from .NET 5 and to keep this library on .NET Standard 2.1?

@HofmeisterAn
Copy link
Collaborator

That's complicated, somehow 😅. I think I need to set up a small example to get a better understanding of which parts are supported and which not.

.NET Core just got support for .pem in .NET 5

It's not part of Standard 2.1?

I'm wondering if it's not easier to rather accept a X509Certificate2 for client auth and to allow optionally a RemoteCertificateValidationCallback for server cert validation and provide some examples on how to load the cert from different sources than supporting various certificate formats

If we keep the flow from ITestcontainersBuilder to DockerApiClient I'm ok with your suggestion. There is no reason to make it more complicated.

This would even allow most users to use the included convenience wrappers from .NET 5 and to keep this library on .NET Standard 2.1

Sounds good.

@prskr
Copy link
Author

prskr commented Mar 16, 2021

It's not part of Standard 2.1?

No they are not 😕

.NET has this nice convenience method:

var certPem = File.ReadAllText("cert.pem");
var eccPem = File.ReadAllText("ecc.pem");

var cert = X509Certificate2.CreateFromPem(certPem, eccPem);

but .NET Standard 2.1 does not. All I can do is either accept a X502Certificate2 for authentication or a directory that contains all required files.
I'll implement the original suggestion of the X509Certificate2 and see how it "feels" in the tests and we can discuss this as soon as it is done?

@HofmeisterAn
Copy link
Collaborator

but .NET Standard 2.1 does not.

Yep, I double checked it. Not available in Standard 2.1.

I'll implement the original suggestion of the X509Certificate2

Sounds good. Let me know if I can help.

@tyrel
Copy link

tyrel commented Nov 10, 2021

Hi! We have a new project using dotnet-testcontainers. I'm familiar with the Java Testcontainers from previous jobs and love it, so I was hoping to use it here as well. Our docker host is in AWS and secured by TLS client auth, so to use dotnet-testcontainers we need TLS working. Note that our project uses .NET Framework 4.8.

It doesn't look like there's been any activity here for a while. As far as pem/pfx is concerned, it's trivial to convert cert.pem and key.pem to a pfx file using the openssl command line, and it's even documented in the Docker.DotNet documentation. We are more than happy to use it this way.

If there hasn't been any progress on this, we may be able to take the time to implement it ourselves and submit a PR. But I wanted to check first to see if anyone else is close to finishing.

@prskr
Copy link
Author

prskr commented Nov 11, 2021

Hi @tyrel I'm sorry to say that I had to postpone my project where I intended to use this PR and therefore stopped working on it.
The original PR #372 got closed and @HofmeisterAn proposed to - if working on this again - probably smaller increments would be better to keep the scope of the PRs cleaner.

Feel free to 'recycle' as much of the original code as you like but I don't think I'll have much time to work on this in the near future. At least not to provide the complete implementation. I'd happy to help with feedback or probably smaller PRs though.

@HofmeisterAn
Copy link
Collaborator

@tyrel as @baez90 mentioned PR #372 covers already a lot of the requirement, but it is not finished yet. I have not the infrastructure or use case to develop and test it further. Anyway - I think it is an important feature and I would be happy too, to support as much as I can.

I'm pretty sure we can use a lot of @baez90 work. Personally, I prefer smaller increments to keep the scope of the PR cleaner. It makes the review much easier.

Maybe we can collect necessary tasks and organize the work a bit?

  1. An interface to pass the credentials from TestcontainersBuilder to DockerApiClient
  2. Support global and builder configurations.
  3. Maybe we can refactore IAuthenticationConfiguration too (Docker Registry authentication)
  4. Add credential providers (basic, certificate, etc.)
  5. Manage certificates (load, validation, etc.)

What do you think?

@prskr
Copy link
Author

prskr commented Nov 11, 2021

One thing that comes to my mind looking at the old PR is that it was actually quite hard to keep track of:

  • TLS validation does not necessarily come with TLS auth hence you've to distinguish between both
  • precedence of the different configuration mechanisms: environment vs. explicit builder. Thinking again about especially this aspect I'd rather skip the whole TLS environment based configuration and would make it explicit. If someone wants to use the environment variables it can always be done e.g. from the test cases...

I think it might be rather import to conform about which and how these TLS related settings should be done before starting to extend the API. As soon this is settled the tasks @HofmeisterAn already mentioned are for sure accurate.

Regarding testing: I did the integration testing on GitLab with DinD but shouldn't be that hard to integrate DinD with TLS enabled also e.g. in Github actions?

@tyrel
Copy link

tyrel commented Nov 11, 2021

Thanks for your responses @baez90 and @HofmeisterAn! I will discuss this with my team and work out how much of this we have bandwidth for. At a minimum we just need to specify a PFX file that can be used for TLS client authentication. We'll look at our options for the server certificate validation as well.

@HofmeisterAn
Copy link
Collaborator

@baez90 DinD is a nice idea. I'll try to setup an example for Azure Pipelines.

@HofmeisterAn
Copy link
Collaborator

HofmeisterAn commented Nov 20, 2021

We can use docker run --rm --privileged --detach -v /tmp:/certs/ca -v /tmp:/certs/client -e DOCKER_TLS_CERTDIR=/certs docker:dind to setup a container that exposes a secure Docker daemon. The client can pick up the certificate files from the mounted volume.

This (setup the test environment) can be done straight in .NET Testcontainers using xUnit fixtures. I can prepare it in the next days. Shouldn't be much code.

@HofmeisterAn HofmeisterAn added the help wanted Extra attention is needed label Jun 4, 2022
HofmeisterAn added a commit that referenced this issue Jun 5, 2022
…ckerEndpointAuthenticationConfiguration'

{Add Docker endpoint auth config.}
HofmeisterAn added a commit that referenced this issue Jun 5, 2022
…ckerEndpointAuthenticationConfiguration'

{Add Docker endpoint auth config.}
@HofmeisterAn
Copy link
Collaborator

HofmeisterAn commented Jun 5, 2022

aed5679 (1.6.0-beta.2251) extends all builder methods WithDockerEndpoint. It adds an overloaded method that gets an implementation of IDockerEndpointAuthenticationConfiguration, e.g.: https://github.com/HofmeisterAn/dotnet-testcontainers/blob/aed56793f5edfb00791a4085ae6b7e7b0f521f93/tests/DotNet.Testcontainers.Tests/Unit/Containers/Unix/TestcontainersContainerTest.cs#L253

You can use this method to do the TLS or any other kind of authentication.


Upcoming tasks:

  • Consider the IDockerEndpointAuthenticationConfiguration in the resource reaper
  • Detect the system configuration (env variables) automatic and set the right default authentication (TestcontainersSettings.OS.DockerEndpointAuthConfig)

HofmeisterAn added a commit that referenced this issue Jun 6, 2022
…ntainersSettings'

{Add the public API attribute to the configuration namespace.}
HofmeisterAn added a commit that referenced this issue Jun 6, 2022
…nticationProvider'

{Prepare the authentication provider for different configurations.}
HofmeisterAn added a commit that referenced this issue Jun 6, 2022
…kerEndpointAuthenticationProvider'

{Add Docker endpoint auth provider.}
HofmeisterAn added a commit that referenced this issue Jun 7, 2022
…ceReaper'

{Use Docker endpoint authentication configuration.}
HofmeisterAn added a commit that referenced this issue Jun 7, 2022
…ceReaper'

{Use Docker endpoint authentication configuration.}
@HofmeisterAn HofmeisterAn added this to the 1.7.0 milestone Jun 7, 2022
@HofmeisterAn HofmeisterAn removed this from the 2.0.0 milestone Jun 21, 2022
vlaskal added a commit to vlaskal/dotnet-testcontainers that referenced this issue Sep 3, 2022
vlaskal added a commit to vlaskal/dotnet-testcontainers that referenced this issue Sep 3, 2022
vlaskal added a commit to vlaskal/dotnet-testcontainers that referenced this issue Sep 4, 2022
vlaskal added a commit to vlaskal/dotnet-testcontainers that referenced this issue Sep 4, 2022
vlaskal added a commit to vlaskal/dotnet-testcontainers that referenced this issue Sep 6, 2022
vlaskal added a commit to vlaskal/dotnet-testcontainers that referenced this issue Sep 6, 2022
vlaskal added a commit to vlaskal/dotnet-testcontainers that referenced this issue Oct 2, 2022
vlaskal added a commit to vlaskal/dotnet-testcontainers that referenced this issue Oct 9, 2022
vlaskal added a commit to vlaskal/dotnet-testcontainers that referenced this issue Oct 9, 2022
vlaskal added a commit to vlaskal/dotnet-testcontainers that referenced this issue Oct 9, 2022
vlaskal added a commit to vlaskal/dotnet-testcontainers that referenced this issue Oct 9, 2022
vlaskal added a commit to vlaskal/dotnet-testcontainers that referenced this issue Oct 9, 2022
@HofmeisterAn HofmeisterAn self-assigned this Oct 10, 2022
vlaskal added a commit to vlaskal/dotnet-testcontainers that referenced this issue Oct 10, 2022
vlaskal added a commit to vlaskal/dotnet-testcontainers that referenced this issue Oct 10, 2022
vlaskal added a commit to vlaskal/dotnet-testcontainers that referenced this issue Oct 10, 2022
vlaskal added a commit to vlaskal/dotnet-testcontainers that referenced this issue Oct 10, 2022
vlaskal added a commit to vlaskal/dotnet-testcontainers that referenced this issue Oct 11, 2022
HofmeisterAn pushed a commit to vlaskal/dotnet-testcontainers that referenced this issue Oct 12, 2022
HofmeisterAn pushed a commit to vlaskal/dotnet-testcontainers that referenced this issue Oct 12, 2022
HofmeisterAn pushed a commit to vlaskal/dotnet-testcontainers that referenced this issue Oct 12, 2022
vlaskal added a commit to vlaskal/dotnet-testcontainers that referenced this issue Oct 13, 2022
vlaskal added a commit to vlaskal/dotnet-testcontainers that referenced this issue Oct 13, 2022
vlaskal added a commit to vlaskal/dotnet-testcontainers that referenced this issue Oct 13, 2022
HofmeisterAn added a commit to vlaskal/dotnet-testcontainers that referenced this issue Oct 20, 2022
HofmeisterAn added a commit to vlaskal/dotnet-testcontainers that referenced this issue Oct 20, 2022
HofmeisterAn added a commit that referenced this issue Oct 20, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

3 participants