Skip to content

Commit c9c0617

Browse files
Az.StorageSync | Added TenantId of ARC Server and checked with StorageSyncService tenant to prevent unsupported configuration (#28355)
Co-authored-by: Copilot <[email protected]>
1 parent 1355f5e commit c9c0617

18 files changed

+574
-5367
lines changed

src/StorageSync/StorageSync.Test/Common/MockServerManagedIdentityProvider.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,9 +24,9 @@ public MockServerManagedIdentityProvider(string testName)
2424

2525
public bool EnableMIChecking { get; set; }
2626

27-
public Guid GetServerApplicationId(LocalServerType serverType, bool throwIfNotFound = true, bool validateSystemAssignedManagedIdentity = true)
27+
public Task<ServerApplicationIdentity> GetServerApplicationIdentityAsync(LocalServerType serverType, bool throwIfNotFound = true, bool validateSystemAssignedManagedIdentity = true)
2828
{
29-
return Guid.Empty;
29+
return Task.FromResult(new ServerApplicationIdentity(Guid.Empty, Guid.Empty));
3030
}
3131

3232
public LocalServerType GetServerType(IEcsManagement ecsManagement)

src/StorageSync/StorageSync.Test/Common/MockSyncServerRegistrationClient.cs

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
using Microsoft.Azure.Commands.StorageSync.Common;
1919
using Microsoft.Azure.Commands.StorageSync.Common.Extensions;
2020
using Microsoft.Azure.Commands.StorageSync.InternalObjects;
21+
using Microsoft.Azure.Commands.StorageSync.Interop.ManagedIdentity;
2122
using Microsoft.Azure.Management.StorageSync.Models;
2223
using Newtonsoft.Json;
2324
using System;
@@ -372,17 +373,18 @@ private bool TryCreateDirectory(string monitoringDataPath, out DirectoryInfo dir
372373
return false;
373374
}
374375

375-
public override Guid? GetApplicationIdOrNull()
376+
public override ServerApplicationIdentity GetServerApplicationIdentityOrNull()
376377
{
377-
if(TestName == "TestNewRegisteredServerWithIdentityError")
378+
var testTenantGuid = new Guid("0483643a-cb2f-462a-bc27-1a270e5bdc0a");
379+
if (TestName == "TestNewRegisteredServerWithIdentityError")
378380
{
379381
return null;
380382
}
381383
if(TestName == "TestPatchRegisteredServer")
382384
{
383-
return new Guid("3b990c8b-9607-4c2a-8b04-1d41985facca");
385+
return new ServerApplicationIdentity(new Guid("3b990c8b-9607-4c2a-8b04-1d41985facca"), testTenantGuid);
384386
}
385-
return Guid.NewGuid();
387+
return new ServerApplicationIdentity(Guid.NewGuid(), testTenantGuid);
386388
}
387389
}
388390
}

src/StorageSync/StorageSync.Test/Common/MockSyncServerRegistrationClientBase.cs

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -61,8 +61,8 @@ public MockSyncServerRegistrationClientBase(IEcsManagement ecsManagementInteropC
6161
/// <summary>
6262
/// This function will return the application id of the server if it is available.
6363
/// </summary>
64-
/// <returns>Application Id or null</returns>
65-
public abstract Guid? GetApplicationIdOrNull();
64+
/// <returns>ServerApplicationIdentity or null</returns>
65+
public abstract ServerApplicationIdentity GetServerApplicationIdentityOrNull();
6666

6767
/// <summary>
6868
/// Validate sync server registration.
@@ -146,6 +146,7 @@ public void Dispose()
146146
/// 4. Get ClusterInfo
147147
/// 5. Populate RegistrationServerResource
148148
/// </summary>
149+
/// <param name="storageSyncServiceTenantId">Storage Sync Service Tenant Id</param>
149150
/// <param name="managementEndpointUri">Management endpoint Uri</param>
150151
/// <param name="subscriptionId">Subscription Id</param>
151152
/// <param name="storageSyncServiceName">Storage Sync Service Name</param>
@@ -162,6 +163,7 @@ public void Dispose()
162163
/// </exception>
163164
/// <exception cref="ServerRegistrationException"></exception>
164165
public RegisteredServer Register(
166+
string storageSyncServiceTenantId,
165167
Uri managementEndpointUri,
166168
Guid subscriptionId,
167169
string storageSyncServiceName,
@@ -176,7 +178,19 @@ public RegisteredServer Register(
176178
bool assignIdentity)
177179
{
178180
// Get ApplicationId
179-
Guid? applicationId = assignIdentity ? GetApplicationIdOrNull() : null;
181+
ServerApplicationIdentity serverApplicationIdentity = assignIdentity ? GetServerApplicationIdentityOrNull() : null;
182+
// Discover the server type , Get the application id,
183+
Guid? applicationId = serverApplicationIdentity?.ApplicationId;
184+
185+
if (serverApplicationIdentity != null && serverApplicationIdentity.TenantId != Guid.Empty)
186+
{
187+
// Check that tenants match
188+
if (!string.Equals(storageSyncServiceTenantId, serverApplicationIdentity.TenantId.ToString(), StringComparison.OrdinalIgnoreCase))
189+
{
190+
throw new ServerRegistrationException(
191+
$"Cross-tenant registration is not allowed. The server belongs to tenant '{serverApplicationIdentity.TenantId}' but the Storage Sync Service is in tenant '{storageSyncServiceTenantId}'.");
192+
}
193+
}
180194

181195
#pragma warning disable CA1416 // Validate platform compatibility
182196
//RegistryUtility.WriteValue(StorageSyncConstants.ServerAuthRegistryKeyName,

src/StorageSync/StorageSync.Test/SessionRecords/StorageSyncTests.RegisteredServerTests/TestNewRegisteredServerWithIdentity.json

Lines changed: 444 additions & 5331 deletions
Large diffs are not rendered by default.

src/StorageSync/StorageSync/ChangeLog.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@
1818
- Additional information about change #1
1919
-->
2020
## Upcoming Release
21-
21+
* Fixed security bug in checking tenant id for MI server registration
2222
## Version 2.5.1
2323
* Fixed security bug in token acquisition for MI server registration
2424

src/StorageSync/StorageSync/Interop/Clients/SyncServerRegistrationClient.cs

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@
2929
using System.Management;
3030
using System.Management.Automation;
3131
using System.Text.RegularExpressions;
32+
using System.Threading.Tasks;
3233

3334
namespace Commands.StorageSync.Interop.Clients
3435
{
@@ -406,13 +407,13 @@ private bool TryCreateDirectory(string monitoringDataPath, out DirectoryInfo dir
406407
/// This function will get the application id of the server if identity is available.
407408
/// </summary>
408409
/// <returns>Application id or null.</returns>
409-
public override Guid? GetApplicationIdOrNull()
410+
public async override Task<ServerApplicationIdentity> GetServerApplicationIdentityOrNull()
410411
{
411412
LocalServerType localServerType = this.ServerManagedIdentityProvider.GetServerType(this.EcsManagementInteropClient);
412413

413-
if(localServerType != LocalServerType.HybridServer)
414+
if (localServerType != LocalServerType.HybridServer)
414415
{
415-
return this.ServerManagedIdentityProvider.GetServerApplicationId(localServerType, throwIfNotFound: true, validateSystemAssignedManagedIdentity: true);
416+
return await this.ServerManagedIdentityProvider.GetServerApplicationIdentityAsync(localServerType, throwIfNotFound: true, validateSystemAssignedManagedIdentity: true);
416417
}
417418
return null;
418419
}

src/StorageSync/StorageSync/Interop/Clients/SyncServerRegistrationClientBase.cs

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -120,7 +120,7 @@ public abstract ServerRegistrationData Setup(
120120
/// This function will return the application id of the server if it is available.
121121
/// </summary>
122122
/// <returns>Application Id or null</returns>
123-
public abstract Guid? GetApplicationIdOrNull();
123+
public abstract Task<ServerApplicationIdentity> GetServerApplicationIdentityOrNull();
124124

125125
/// <summary>
126126
/// Dispose method for cleaning Interop client object.
@@ -146,6 +146,7 @@ public void Dispose()
146146
/// 3. Calls RegisterOnline callback to make ARM call (from caller context)
147147
/// 4. Persists registered server resource from cloud to local FileSyncSvc service
148148
/// </summary>
149+
/// <param name="storageSyncServiceTenantId">Storage Sync Service TenantId</param>
149150
/// <param name="managementEndpointUri">Management endpoint Uri</param>
150151
/// <param name="subscriptionId">Subscription Id</param>
151152
/// <param name="storageSyncServiceName">Storage Sync Service Name</param>
@@ -160,6 +161,7 @@ public void Dispose()
160161
/// <param name="assignIdentity">Assign Identity</param>
161162
/// <returns>Registered Server Resource</returns>
162163
public RegisteredServer Register(
164+
string storageSyncServiceTenantId,
163165
Uri managementEndpointUri,
164166
Guid subscriptionId,
165167
string storageSyncServiceName,
@@ -174,7 +176,17 @@ public RegisteredServer Register(
174176
bool assignIdentity)
175177
{
176178
// Discover the server type , Get the application id,
177-
Guid? applicationId = assignIdentity ? GetApplicationIdOrNull() : null;
179+
ServerApplicationIdentity serverApplicationIdentity = assignIdentity ? GetServerApplicationIdentityOrNull().GetAwaiter().GetResult() : null;
180+
Guid? applicationId = serverApplicationIdentity?.ApplicationId;
181+
182+
if (serverApplicationIdentity != null && serverApplicationIdentity.TenantId != Guid.Empty)
183+
{
184+
// Check that tenants match
185+
if (!string.Equals(storageSyncServiceTenantId, serverApplicationIdentity.TenantId.ToString(), StringComparison.OrdinalIgnoreCase))
186+
{
187+
throw new ServerRegistrationException(ServerRegistrationErrorCode.ServerAndSyncServiceTenantMismatched);
188+
}
189+
}
178190

179191
// Set the registry key for ServerAuthType
180192
RegistryUtility.WriteValue(StorageSyncConstants.ServerAuthRegistryKeyName,

src/StorageSync/StorageSync/Interop/Enums/ServerRegistrationErrorCode.cs

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -134,7 +134,12 @@ public enum ServerRegistrationErrorCode
134134
/// <summary>
135135
/// Monitoring Service Endpoint Invalid or Not Set
136136
/// </summary>
137-
MonitoringServiceEndpointInvalidOrNotSet
137+
MonitoringServiceEndpointInvalidOrNotSet,
138+
139+
/// <summary>
140+
/// Server and Sync Service Tenant Mismatched.
141+
/// </summary>
142+
ServerAndSyncServiceTenantMismatched
138143

139144
}
140145
}

src/StorageSync/StorageSync/Interop/Interfaces/IServerManagedIdentityProvider.cs

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
using Commands.StorageSync.Interop.Interfaces;
22
using Microsoft.Azure.Commands.StorageSync.Interop.Enums;
33
using System;
4+
using System.Threading.Tasks;
45

56
namespace Microsoft.Azure.Commands.StorageSync.Interop.ManagedIdentity
67
{
@@ -20,14 +21,11 @@ public interface IServerManagedIdentityProvider
2021
LocalServerType GetServerType(IEcsManagement ecsManagement);
2122

2223
/// <summary>
23-
/// Gets the server's application id by trying to get a token and parsing for the oid
24-
/// We choose to get the applicationId from the token rather than making a Get call on the resource
25-
/// because we don't know the permissions the user has on the resource
24+
/// Gets the server's application identity (application ID and tenant ID) asynchronously by trying to get a token from the Arc/Azure IMDS endpoint and parsing for the oid and tenant ID.
2625
/// </summary>
2726
/// <param name="serverType">ServerType: Hybrid or Azure</param>
2827
/// <param name="throwIfNotFound">Whether to throw an exception if an Application ID is not available</param>
2928
/// <param name="validateSystemAssignedManagedIdentity">Whether to validate that the Application Id belongs to a System-Assigned Managed Identity</param>
30-
/// <returns>Server's applicationId if it's available, Guid.Empty otherwise</returns>
31-
Guid GetServerApplicationId(LocalServerType serverType, bool throwIfNotFound = true, bool validateSystemAssignedManagedIdentity = true);
29+
Task<ServerApplicationIdentity> GetServerApplicationIdentityAsync(LocalServerType serverType, bool throwIfNotFound = true, bool validateSystemAssignedManagedIdentity = true);
3230
}
3331
}

src/StorageSync/StorageSync/Interop/Interfaces/ISyncServerRegistration.cs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ public interface ISyncServerRegistration : IDisposable
3232
/// 2. Sets up ServerRegistrationData
3333
/// 3. Calls RegisterOnline callback to make ARM call (from caller context)
3434
/// 4. Persists registered server resource from cloud to local FileSyncSvc service
35+
/// <param name="storageSyncServiceTenantId">Storage Sync Service TenantId</param>
3536
/// <param name="managementEndpointUri">Management endpoint Uri</param>
3637
/// <param name="subscriptionId">Subscription Id</param>
3738
/// <param name="storageSyncServiceName">Storage Sync Service Name</param>
@@ -47,6 +48,7 @@ public interface ISyncServerRegistration : IDisposable
4748
/// <returns>Registered Server Resource</returns>
4849
/// </summary>
4950
RegisteredServer Register(
51+
string storageSyncServiceTenantId,
5052
Uri managementEndpointUri,
5153
Guid subscriptionId,
5254
string storageSyncServiceName,

0 commit comments

Comments
 (0)