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

ManagedIdentity and Workload Identity support #1864

Open
dersia opened this issue Jan 26, 2024 · 8 comments
Open

ManagedIdentity and Workload Identity support #1864

dersia opened this issue Jan 26, 2024 · 8 comments
Labels
area-integrations Issues pertaining to Aspire Integrations packages azure Issues associated specifically with scenarios tied to using Azure feature A single user-facing feature. Can be grouped under an epic.
Milestone

Comments

@dersia
Copy link

dersia commented Jan 26, 2024

I really like what Aspire is doing and how it simplifies the development and deployment cycle. as I was looking through the issue for preview 3 I came across some issue that have considerations around connectionstrings.

in general I don't think that it is a good approach to have "full" connectionstrings in files like the manifest, though I understand that there might be circumstances where this might be preferable, I think we should have a better way of handling secrets espacially during deployment.

I am a big fan of Managed- and Workflow Identities and think it makes sense to use them when ever possible. there are some services that might not support ManagedIdentities and for this I would prefer to have them in a Keyvault and them have the connection to that KV be secured using a ManagedIdentity.

I think it would be great for Aspire to set in the manifest to use managed Identities where possible or where it is not possible a additional KV resource is added to the manifest and that in the manifest it ptells the tooling to put the credentials/connectionstrings into this KV and then use the secret from KV when added as a reference in another service.

the tooling (azd up, aspir8) could than create the managed/workflow Identities, add the right role permissions and configure ACA or Aks to use that identity to connect to the services. this would make much safer production deployments and increase overall security.

I hope it is understandable what I am suggesting, sometimes things sound well formulated in my head, but they don't come out the way I'd like 😅
so please let me know if something is unclear.

@dotnet-issue-labeler dotnet-issue-labeler bot added the area-app-model Issues pertaining to the APIs in Aspire.Hosting, e.g. DistributedApplication label Jan 26, 2024
@davidfowl davidfowl added this to the Backlog milestone Jan 27, 2024
@davidfowl
Copy link
Member

davidfowl commented Jan 28, 2024

We have plans on how to model secrets that are going to be fleshed out in the next release. Once we're done with the refactoring, they'll never show up in the manifest verbatim (unless you do it manually as part of WithEnvironment).

Today when you spin up an aspire app with azd, it creates a user assigned managed identity behind the scenes and manages various role assignments for them. It's unclear if we'll provide any more control over you dropping to bicep in the initial release. That said, in the future we'll want to model this as well.

@davidfowl davidfowl added azure Issues associated specifically with scenarios tied to using Azure and removed untriaged labels Jan 28, 2024
@dersia
Copy link
Author

dersia commented Jan 30, 2024

It's great that this is happening as of today already, but when I understand that correctly this is not part of the manifest.json, right? So probably this won't happen for Aspir8?

If it would be part of the manifest, we could just implement a "lite" version of this:

AddManagedIdentityRole(string roleId). That would make it easier to add permissions the ManagedIdentity that is created by azd up to existing resources. Most of the time, when you start a new project you often want to connect to existing Azure Resources. We have a lot shared Cognitive Search Resources for example, or when we connect to Azure OpenAI.

@davidfowl
Copy link
Member

It's great that this is happening as of today already, but when I understand that correctly this is not part of the manifest.json, right? So probably this won't happen for Aspir8?

Aspir8 doesn't support azure at all. It's a pure kubernetes solution that has no dependency on azure resources.

If it would be part of the manifest, we could just implement a "lite" version of this:

We're pushing azure resources out of the manifest as first class concept. Instead we're moving to a single bicep resource to represent them. That'll open the door for modeling what you are trying to model.

@WolfspiritM
Copy link

WolfspiritM commented Feb 14, 2024

@davidfowl

Today when you spin up an aspire app with azd, it creates a user assigned managed identity behind the scenes and manages various role assignments for them

I gave that a try and exported everything to biceps via azd infra synth.

My code looks like this right now:

var sqlServer = builder.AddSqlServer("sqlserver");

var db = sqlServer.AddDatabase("aspire_sample");
var db2 = sqlServer.AddDatabase("aspire_sample2");

var apiservice = builder.AddProject<Projects.AspireSample_ApiService>("apiservice")
    .WithReference(db);

builder.AddProject<Projects.AspireSample_Web>("webfrontend")
    .WithReference(db2)
    .WithReference(apiservice);

With that code I'd expect that aspire makes sure that only the ApiService has access to aspire_sample and only the Web project has access to "aspire_sample2".

The generated biceps code however generates one managed identity for all applications and assigns that as ADMIN to the sqlServer. I can't believe that this is best practice.

resource managedIdentity 'Microsoft.ManagedIdentity/userAssignedIdentities@2023-01-31' = {
  name: 'mi-${resourceToken}'
  ...
}
resource sqlserver 'Microsoft.Sql/servers@2022-05-01-preview' = {
  name: 'sqlserver-${resourceToken}'
  ...
  properties: {
    ...
    administrators: {
      administratorType: 'ActiveDirectory'
      azureADOnlyAuthentication: true
      login: managedIdentity.name
      principalType: 'User'
      sid: managedIdentity.properties.principalId
      tenantId: subscription().tenantId
    }
  }

In my opinion this generates multiple problems:

  1. How do I assign myself, or an administrator group as admin when deploying?
  2. Adding the managed identity as admin means if someone somehow manages to get access to the frontend projects and retrieve the managed identity, then they also have access to the whole sql server (and even to all other sqlservers in the same aspire project, too).

The correct way in my opinion is to create a managed identity for each service and assign it to the database. Even db_owner on the database would be better than having full admin access on every server. I know that assigning managed identities to databases isn't possible yet (without using some deploy script), which is a big issue for us for years now, which is why I hoped that with Aspire I don't have to think about managing SQL users anymore and still have a secure environment.

Not sure if this is only an Azure SQL issue but it feels like a general issue to me.

@davidfowl
Copy link
Member

davidfowl commented Feb 14, 2024

The generated biceps code however generates one managed identity for all applications and assigns that as ADMIN to the sqlServer. I can't believe that this is best practice.

It's not. Right now the generated bicep hasn't been reviewed for best practices and I expect us to make big changes to the default model once we get the end-to-end working.

The correct way in my opinion is to create a managed identity for each service and assign it to the database. Even db_owner on the database would be better than having full admin access on every server.

How do you tell the application which identity to use in this case? In an app uses N services, do you pass it N managed identity client ids somehow and manually flow that to the SDK? Or am I missing something?

Aspire I don't have to think about managing SQL users anymore and still have a secure environment.

No, I don't think aspire will help you forget about needing to understand things in azure to be secure. We will have defaults for different use cases that will be heavily documented and you can decide if that level of security works for your scenario.

It's a little fast and loose right now while we're in preview, things are still changing.

As an aside. I've been experimenting with a managed identity resource type based on the bicep resource primitive:

https://github.com/dotnet/aspire/tree/davidfowl/managed-identity

@WolfspiritM
Copy link

How do you tell the application which identity to use in this case? In an app uses N services, do you pass it N managed identity client ids somehow and manually flow that to the SDK? Or am I missing something?

Sorry. I think the word "service" was a bad choice. With service I meant that each Container App (project) for example should have one managed identity representing the container app.

For example with terraform we currently deploy a managed identity with each container app and then use some third party terraform module to assign the managed identity as db_owner to the database the app is allowed to access. Sadly currently no Azure API seems to support adding users or managed identities to databases. I hope this sometime gets fixed.

@davidfowl
Copy link
Member

Sorry. I think the word "service" was a bad choice. With service I meant that each Container App (project) for example should have one managed identity representing the container app.

This seems like a reasonable default, still, before we make any changes here we'll review the default security model with our security gurus and decide what the "default" should be if nothing is specified. Then there's be a mode where you get full control and can decide what to do (like keeping your terraform model).

@davidfowl
Copy link
Member

davidfowl commented Feb 17, 2024

Spike for managed identity here.

Managed identity discussion on azd here Azure/azure-dev#3384

@davidfowl davidfowl added area-integrations Issues pertaining to Aspire Integrations packages and removed area-app-model Issues pertaining to the APIs in Aspire.Hosting, e.g. DistributedApplication labels Sep 7, 2024
@davidfowl davidfowl added the feature A single user-facing feature. Can be grouped under an epic. label Sep 15, 2024
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 azure Issues associated specifically with scenarios tied to using Azure feature A single user-facing feature. Can be grouped under an epic.
Projects
None yet
Development

No branches or pull requests

3 participants