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

Ensure all containers are password protected #6155

Open
2 of 7 tasks
eerhardt opened this issue Oct 7, 2024 · 6 comments
Open
2 of 7 tasks

Ensure all containers are password protected #6155

eerhardt opened this issue Oct 7, 2024 · 6 comments
Labels
area-integrations Issues pertaining to Aspire Integrations packages security 🔐
Milestone

Comments

@eerhardt
Copy link
Member

eerhardt commented Oct 7, 2024

Where possible, we should be enabling passwords on our containers.

The goal here is to provide a layer of redundancy. If ACA firewalling (or other system to from external access) fails, an external client will still not be able to access the container without knowledge of the authentication password.

The following containers don't have password protection:

@eerhardt eerhardt added this to the Backlog milestone Oct 7, 2024
@davidfowl davidfowl added the area-integrations Issues pertaining to Aspire Integrations packages label Oct 7, 2024
@Mrxx99
Copy link
Contributor

Mrxx99 commented Oct 10, 2024

For NATS I would like to contribute the password implementation, I already have explored and started work on this

@eerhardt
Copy link
Member Author

For NATS I would like to contribute the password implementation, I already have explored and started work on this

Sounds great! Looking forward to the PR.

@Mrxx99
Copy link
Contributor

Mrxx99 commented Oct 19, 2024

For NATS I would like to contribute the password implementation, I already have explored and started work on this

Sounds great! Looking forward to the PR.

@eerhardt The PR #6259 is ready for review.

eerhardt added a commit that referenced this issue Nov 6, 2024
This adds parameter support to NATS. The postgres implementation was used as inspiration for the implementation.
It uses the user info of the connection string URL to add user name and password, so the connections string looks like
nats://nats:MGBKxjTAjX2stg2wzzjtVW@localhost:52166.
Some NATS clients support this form natively but the .NET one unfortunately does not. It would be nice if this support would be added directly to NATS.Net then that code could be removed from here again.

Contributes to: #6155

* Add password protection to NATS

* fix redacting of password

* restore name only NatsServerResource constructor to remove breaking change + added more tests

* fix duplicated namespace

* authentication tests use same wait mechanic as other tests

* restore xml comment for resource name

* Update src/Aspire.Hosting.Nats/NatsServerResource.cs

Co-authored-by: Eric Erhardt <[email protected]>

* Update src/Aspire.Hosting.Nats/NatsServerResource.cs

Co-authored-by: Eric Erhardt <[email protected]>

* Update tests/Aspire.Hosting.Nats.Tests/NatsFunctionalTests.cs

Co-authored-by: Eric Erhardt <[email protected]>

* Update tests/Aspire.Hosting.Nats.Tests/AddNatsTests.cs

Co-authored-by: Eric Erhardt <[email protected]>

* addressed some review comments

* updated to Nats.NET 2.5.3, use authentication in connection string that is now built in

* re-add original AddNats overload (without optional parameter)

* Trigger build

---------

Co-authored-by: Eric Erhardt <[email protected]>
@eerhardt
Copy link
Member Author

eerhardt commented Dec 2, 2024

@liammclennan @nblumhardt - any interest in adding password support for Seq?

@nblumhardt
Copy link

nblumhardt commented Dec 3, 2024

@eerhardt sounds good!

The Seq container supports a couple of environment variables that would make this pretty easy.

One possible wrinkle; the variables are:

  • SEQ_FIRSTRUN_ADMINUSERNAME - username for the automatically-created admin user account; default is admin
  • SEQ_FIRSTRUN_ADMINPASSWORD - plain text password for same
  • SEQ_FIRSTRUN_ADMINPASSWORDHASH - alternative to the above, provided as a salted cryptographic hash, for marginally better security when passing through configuration systems
  • SEQ_FIRSTRUN_REQUIREAUTHENTICATIONFORHTTPINGESTION - sets the "require authentication for ingestion" flag

But, these only apply when the container's storage is first created, so modifying them once the Seq container has been initialized/has persistent data will have no effect. Would this be surprising/break expectations compared with the way other containers are doing it?

@eerhardt
Copy link
Member Author

eerhardt commented Dec 3, 2024

But, these only apply when the container's storage is first created, so modifying them once the Seq container has been initialized/has persistent data will have no effect. Would this be surprising/break expectations compared with the way other containers are doing it?

I don't think it would be surprising since it is consistent with other containers. Other containers (postgres, mongodb, etc) use the password in the stored data as well. Changing the password after the data was created basically invalidates the existing data.

See the Warning at: https://learn.microsoft.com/en-us/dotnet/aspire/database/sql-server-integration?tabs=dotnet-cli%2Cssms#add-sql-server-resource-with-data-volume

Warning

The password is stored in the data volume. When using a data volume and if the password changes, it will not work until you delete the volume.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-integrations Issues pertaining to Aspire Integrations packages security 🔐
Projects
None yet
Development

No branches or pull requests

4 participants