Azure / azure-libraries-for-net

Azure libraries for .Net
MIT License
379 stars 193 forks source link

Support subject + issuer validation in AzureCredentialsFactory.FromServicePrincipal #615

Closed m-mertz closed 4 years ago

m-mertz commented 5 years ago

In order to use a certificate that is whitelisted by subject + issuer instead of thumbprint, the whole public key needs to be sent when getting an access token. This is controlled by the sendx5c parameter in AuthenticationContext.AcquireTokenAsync. Currently it's not possible to set that flag through the AzureCredentialsFactory methods, and S+I validation doesn't work as a consequence.

A possible solution would be to add an optional sendx5c parameter to AzureCredentialsFactory.FromServicePrincipal, and pass it through to AcquireTokenAsync.

This may depend on the following issue, if that SDK is used internally for authentication rather than AuthenticationContext directly: https://github.com/Azure/azure-sdk-for-net/issues/5206

kurtzeborn commented 5 years ago

Thank you for opening this issue! We are routing it to the appropriate team for follow up.

ChenTanyi commented 4 years ago

@m-mertz The AzureCredentialsFactory depends on Microsoft.Rest.Azure.Authentication.ApplicationTokenProvider, rather than call AuthenticationContext directly. From https://github.com/Azure/azure-libraries-for-net/blob/02cc9fcde0c44d3488aa73b5c3a5cb6f379ba833/src/ResourceManagement/ResourceManager/Authentication/AzureCredentials.cs#L172 to https://github.com/Azure/azure-sdk-for-net/blob/0ec839f23c14c8109a6bdd59118bb74384a73818/sdk/mgmtcommon/Auth/Az.Auth/Az.Authentication/ApplicationTokenProvider.cs#L457

Thus, we are afraid that we cannot update it with sendx5c parameter until the Microsoft.Rest.Azure.Authentication provider a function with this parameter.

huajunzhao commented 4 years ago

@ChenTanyi We have the same requirement recently, is there any timeline to support this in Azure fluent sdk? Do you have any alternatives to support SN+Issuer in Azure fluent sdk?

weidongxu-microsoft commented 4 years ago

@ChenTanyi Actually Authentication have it, but it is !net452, so our lib cannot see/use it for our target framework is net452. https://github.com/Azure/azure-sdk-for-net/blob/0ec839f23c14c8109a6bdd59118bb74384a73818/sdk/mgmtcommon/Auth/Az.Auth/Az.Authentication/ApplicationTokenProvider.cs#L270 The IsCertificateRollOverEnabled parameter.

To support this, we might need to add additional target framework as net461.

cc @yungezz @manish1604 @yaohaizh @nickzhums

manish1604 commented 4 years ago

We are a heavy user of fluent. And this is required to unblock us for cert+issuer requirements.

Our target framework is net462. More implementation can be found at:

  1. https://msazure.visualstudio.com/One/_git/CRM-MidTier-Services-NRD-Fabric?path=%2Fsrc%2FMicrosoft.CDS.NonRelational.Provisioning.Azure
  2. https://msazure.visualstudio.com/CCI/_git/Foundation?path=%2Fsrc%2FProvisioning%2FMicrosoft.CCI.Platform.Azure

Thanks Manish

ChenTanyi commented 4 years ago

@manish1604 Is there any document I can test with? I saw the function with IsCertificateRollOverEnabled parameter call another function outside !net452, but I'm not sure if it is same parameter as sendx5c, since it doesn't call AcquireTokenAsync finally.

weidongxu-microsoft commented 4 years ago

@ChenTanyi The actual call happens here https://github.com/Azure/azure-sdk-for-net/blob/0ec839f23c14c8109a6bdd59118bb74384a73818/sdk/mgmtcommon/Auth/Az.Auth/Az.Authentication/CertificateAuthenticationProvider.cs#L87

So I guess net452 will not do...

ChenTanyi commented 4 years ago

@weidongxu-microsoft Does this only need to add a target framework at .csproj?

weidongxu-microsoft commented 4 years ago

https://github.com/Azure/azure-sdk-for-net/pull/10696

nickzhums commented 4 years ago

@manish1604 Thanks for letting us know about this issue. Just for reference, we are currently working on the next version of .NET SDK based on a better authentication mechanism and API design and it will be released around June. The .NET Fluent SDK will be slowly phased out, it's recommended that you guys slowly move away from Fluent SDK when the new SDK comes out. Let me know if you have further questions.

manish1604 commented 4 years ago

Thanks @nickzhums. We will keep this task in our roadmap. For now, we need the support for sendX5c flag to enable auto rotation of the certificate.

weidongxu-microsoft commented 4 years ago

@ChenTanyi

Update from @manish1604, client calls

AzureCredentials azureCredentials = new AzureCredentialsFactory().FromServicePrincipal(
    clientId: clientId,
    certificate: certificate,
    tenantId: tenantId,
    environment: azureEnvironment);

We can add one more param e.g. isCertRollOverEnabled = false (and finally in ServicePrincipalLoginInformation).

We likely need to handle the case of X509Certificate2 as well.

The interface we might use in CertificateAuthenticationProvider is likely

public CertificateAuthenticationProvider(byte[] rawCertificate, bool IsCertRollOverEnabled)
public CertificateAuthenticationProvider(ClientAssertionCertificate certAssertion, bool IsCertRollOverEnabled)

Former for raw, latter for X509Certificate2.

weidongxu-microsoft commented 4 years ago

Internal doc

https://aadwiki.windows-int.net/index.php?title=Subject_Name_and_Issuer_Authentication

huajunzhao commented 4 years ago

FYI

I was using the below workaround to get AzureCrendetails which supports SN+Issuer auth.

image

sngupt commented 4 years ago

@ChenTanyi, trying to implement this in .net462, with the isCertRolloverEnabled set to true. Get error, it still looks for the thumbprint of the certificate instead of looking up by subject name. (details here: https://stackoverflow.microsoft.com/questions/205108 )

ActiveDirectoryServiceSettings graphActiveDirectoryServiceSettings = new ActiveDirectoryServiceSettings() { AuthenticationEndpoint = new Uri(@"https://login.microsoftonline.com/" + domain),TokenAudience = new Uri(@"https://graph.windows.net/") }; var clientAssertionCertificate = new Microsoft.Rest.Azure.Authentication.ClientAssertionCertificate(webApp_clientId, clientCert); CertificateAuthenticationProvider certificateAuthenticationProvider = new CertificateAuthenticationProvider(clientAssertionCertificate, true); var xCredentials = await ApplicationTokenProvider.LoginSilentAsync(domain, AppId, certificateAuthenticationProvider,graphActiveDirectoryServiceSettings, TokenCache.DefaultShared);

Error: Get error - AADSTS700027: Client assertion contains an invalid signature. [Reason - The key was not found., Thumbprint of key used by client: 'XXX', Please visit 'https://developer.microsoft.com/en-us/graph/graph-explorer' and query for 'https://graph.microsoft.com/beta/applications/17376230-408c-4b62-b3f3-e2d3b6c74619' to see configured keys]

ChenTanyi commented 4 years ago

@sngupt It may be you setting or certificate error. Could you use the AuthenticationContext.AcquireTokenAsync to test your authentication? The implementation is using this function to acquire token.

sngupt commented 4 years ago

@sngupt It may be you setting or certificate error. Could you use the AuthenticationContext.AcquireTokenAsync to test your authentication? The implementation is using this function to acquire token.

@ChenTanyi , I am able to get token using MSAL API but not with ADAL API, i.e good with -

`IConfidentialClientApplication app = ConfidentialClientApplicationBuilder.Create("") .WithAuthority("https://login.microsoftonline.com/") .WithCertificate(certificate) .BuildConcrete();

        AuthenticationResult result = await app.AcquireTokenForClient(new[] {"https://graph.windows.net/.default"})
                                         .WithSendX5C(true) // WithSendX5c is required here
                                         .ExecuteAsync(CancellationToken.None);`

but not with

AuthenticationContext context = new AuthenticationContext("https://login.microsoftonline.com/<tenantid>"); AuthenticationResult result = await context.AcquireTokenAsync("https://graph.windows.net", new ClientAssertionCertificate("<applicationid>", certificate), sendX5c: true); // sendX5c is required here and is only available in 3.19.4 and after.

ChenTanyi commented 4 years ago

@sngupt if you are implementing authentication by your own, I think the MSAL is OK. These two tokens are the same. I don't know the API logic either. If you have question about ADAL API, maybe you had better open a issue in https://github.com/AzureAD/azure-activedirectory-library-for-dotnet