Skip to content

Commit

Permalink
Removed obsolete and vulnerable Microsoft.Azure.Services.AppAuthentic…
Browse files Browse the repository at this point in the history
…ation

* Fixes vulnerablities found in #417
* Microsoft.Azure.Services.AppAuthentication is obsolete. SqlClient was updated to version 3.0.0 which has integrated AD authentication capablities which can be configured in the connection string (https://learn.microsoft.com/en-us/sql/connect/ado-net/sql/azure-active-directory-authentication?view=sql-server-ver16).
* Removed sink options UseAzureManagedIdentity, AzureServiceTokenProviderResource, AzureTenantId and any related code.
  • Loading branch information
ckadluba committed Oct 17, 2022
1 parent 507d323 commit cfa976b
Show file tree
Hide file tree
Showing 19 changed files with 23 additions and 393 deletions.
32 changes: 1 addition & 31 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -243,9 +243,6 @@ Basic settings of the sink are configured using the properties in a `MSSqlServer
* `BatchPostingLimit`
* `BatchPeriod`
* `EagerlyEmitFirstEvent`
* `UseAzureManagedIdentity`
* `AzureServiceTokenProviderResource`
* `AzureTenantId`

### TableName

Expand Down Expand Up @@ -283,34 +280,6 @@ This setting is not used by the audit sink as it writes each event immediately a
A Flag to eagerly write a batch to the database containing the first received event regardless of `BatchPostingLimit` or `BatchPeriod`. It defaults to `true`.
This setting is not used by the audit sink as it writes each event immediately and not in a batched manner.

### UseAzureManagedIdentity

A flag specifiying to use Azure Managed Identities for authenticating with an Azure SQL server. It defaults to `false`. If enabled the property `AzureServiceTokenProviderResource` must be set as well.

**IMPORTANT:** Azure Managed Identities is only supported for the target frameworks .NET Framework 4.7.2+ and .NET (Core) 2.2+. Setting this to `true` when targeting a different framework results in an exception.

See [Azure AD-managed identities for Azure resources documentation](https://docs.microsoft.com/en-us/azure/active-directory/managed-identities-azure-resources/) for details on how to configure and use Azure Managed Identitites.

### AzureServiceTokenProviderResource

Specifies the token provider resource to be used for aquiring an authentication token when using Azure Managed Identities for authenticating with an Azure SQL server. This setting is only used if `UseAzureManagedIdentity` is set to `true`. For Azure SQL databases this value will always be `https://database.windows.net/`.

### AzureTenantId

Specifies the tenant ID of the the tenant the Azure SQL database exists in. This only needs to be set if the user authenticating against the database is in a different tenant to the database. This will most likely be the case when you are debugging locally and authenticating as yourself rather than the app to be deployed to.

```
.WriteTo.MSSqlServer(
Environment.GetEnvironmentVariable("LogConnection"),
sinkOptions: new MSSqlServerSinkOptions()
{
TableName = "_Log",
UseAzureManagedIdentity = true,
AzureServiceTokenProviderResource = "https://database.windows.net/",
AzureTenantId = Environment.GetEnvironmentVariable("AZURE_TENANT_ID")
}
```


## ColumnOptions Object

Expand Down Expand Up @@ -825,6 +794,7 @@ WHERE

Feature | Notes
:--- | :---
`UseAzureManagedIdentity` | Since the update of Microsoft.Data.SqlClient in sink release 5.8.0 Active Directory auth capabilities of SqlClient can be used. You can specify one of the supported AD authentication methods, which include Azure Managed Identites, directly in the connection string. Refer to the [SqlClient documentation](https://learn.microsoft.com/en-us/sql/connect/ado-net/sql/azure-active-directory-authentication?view=sql-server-ver16) for details.
`AdditionalDataColumns` | Use the `AdditionalColumns` collection instead. Configuring the sink no longer relies upon .NET `DataColumn` objects or .NET `System` types.
`Id.BigInt` | Use `Id.DataType = SqlDb.BigInt` instead. (The `BigInt` property was only available in dev packages).
`Binary` and `VarBinary` | Due to the way Serilog represents property data internally, it isn't possible for the sink to access property data as a byte array, so the sink can't write to these column types.
Expand Down
8 changes: 2 additions & 6 deletions sample/WorkerServiceDemo/appsettings.json
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,7 @@
"connectionString": "Server=localhost;Database=LogTest;Integrated Security=SSPI;",
"sinkOptionsSection": {
"tableName": "LogEvents",
"autoCreateSqlTable": true,
"useAzureManagedIdentity": false,
"azureServiceTokenProviderResource": "https://database.windpws.net/"
"autoCreateSqlTable": true
},
"restrictedToMinimumLevel": "Information",
"columnOptionsSection": {
Expand Down Expand Up @@ -49,9 +47,7 @@
"logEventFormatter": "WorkerServiceDemo.CustomLogEventFormatter::Formatter, WorkerServiceDemo",
"sinkOptionsSection": {
"tableName": "LogEventsAudit",
"autoCreateSqlTable": true,
"useAzureManagedIdentity": false,
"azureServiceTokenProviderResource": "https://database.windpws.net/"
"autoCreateSqlTable": true
},
"columnOptionsSection": {
"addStandardColumns": [ "LogEvent" ],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ public MSSqlServerSinkOptions ConfigureSinkOptions(MSSqlServerSinkOptions sinkOp

ReadTableOptions(config, sinkOptions);
ReadBatchSettings(config, sinkOptions);
ReadAzureManagedIdentitiesOptions(config, sinkOptions);

return sinkOptions;
}
Expand All @@ -34,12 +33,5 @@ private static void ReadBatchSettings(IConfigurationSection config, MSSqlServerS
SetProperty.IfNotNull<string>(config["batchPeriod"], val => sinkOptions.BatchPeriod = TimeSpan.Parse(val, CultureInfo.InvariantCulture));
SetProperty.IfNotNull<bool>(config["eagerlyEmitFirstEvent"], val => sinkOptions.EagerlyEmitFirstEvent = val);
}

private static void ReadAzureManagedIdentitiesOptions(IConfigurationSection config, MSSqlServerSinkOptions sinkOptions)
{
SetProperty.IfNotNull<bool>(config["useAzureManagedIdentity"], val => sinkOptions.UseAzureManagedIdentity = val);
SetProperty.IfNotNull<string>(config["azureServiceTokenProviderResource"], val => sinkOptions.AzureServiceTokenProviderResource = val);
SetProperty.IfNotNull<string>(config["azureTenantId"], val => sinkOptions.AzureTenantId = val);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -185,25 +185,6 @@ internal set
base[nameof(PrimaryKeyColumnName)] = value;
}
}

[ConfigurationProperty(nameof(UseAzureManagedIdentity))]
public ValueConfigElement UseAzureManagedIdentity
{
get => (ValueConfigElement)base[nameof(UseAzureManagedIdentity)];
}

[ConfigurationProperty(nameof(AzureServiceTokenProviderResource))]
public ValueConfigElement AzureServiceTokenProviderResource
{
get => (ValueConfigElement)base[nameof(AzureServiceTokenProviderResource)];
}


[ConfigurationProperty(nameof(AzureTenantId))]
public ValueConfigElement AzureTenantId
{
get => (ValueConfigElement)base[nameof(AzureTenantId)];
}
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ public MSSqlServerSinkOptions ConfigureSinkOptions(MSSqlServerConfigurationSecti
{
ReadTableOptions(config, sinkOptions);
ReadBatchSettings(config, sinkOptions);
ReadAzureManagedIdentitiesOptions(config, sinkOptions);

return sinkOptions;
}
Expand All @@ -32,13 +31,5 @@ private static void ReadBatchSettings(MSSqlServerConfigurationSection config, MS
SetProperty.IfProvided<bool>(config.EagerlyEmitFirstEvent, nameof(config.EagerlyEmitFirstEvent.Value),
value => sinkOptions.EagerlyEmitFirstEvent = value);
}

private static void ReadAzureManagedIdentitiesOptions(MSSqlServerConfigurationSection config, MSSqlServerSinkOptions sinkOptions)
{
SetProperty.IfProvided<bool>(config.UseAzureManagedIdentity, nameof(config.UseAzureManagedIdentity.Value),
value => sinkOptions.UseAzureManagedIdentity = value);
SetProperty.IfProvided<string>(config.AzureServiceTokenProviderResource, nameof(config.AzureServiceTokenProviderResource.Value),
value => sinkOptions.AzureServiceTokenProviderResource = value);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -45,16 +45,12 @@
<Compile Remove="Configuration\Extensions\System.Configuration\**\*.*" />
<Compile Remove="Configuration\Implementations\Microsoft.Extensions.Configuration\**\*.*" />
<Compile Remove="Configuration\Implementations\System.Configuration\**\*.*" />
<Compile Remove="Sinks\MSSqlServer\Platform\AzureManagedServiceAuthenticator.cs" />
<Compile Remove="Sinks\MSSqlServer\Platform\AzureManagedServiceAuthenticatorStub.cs" />
<!-- Show in VStudio, but MSBuild ignores these (indicates files are not code, non-published-content, etc.) -->
<None Include="Configuration\Extensions\Hybrid\**\*.*" />
<None Include="Configuration\Extensions\Microsoft.Extensions.Configuration\**\*.*" />
<None Include="Configuration\Extensions\System.Configuration\**\*.*" />
<None Include="Configuration\Implementations\Microsoft.Extensions.Configuration\**\*.*" />
<None Include="Configuration\Implementations\System.Configuration\**\*.*" />
<None Include="Sinks\MSSqlServer\Platform\AzureManagedServiceAuthenticator.cs" />
<None Include="Sinks\MSSqlServer\Platform\AzureManagedServiceAuthenticatorStub.cs" />
<!-- ItemGroups below with TFM conditions will re-include the compile targets -->
</ItemGroup>

Expand All @@ -64,7 +60,6 @@
<PackageReference Include="Microsoft.Extensions.Options.ConfigurationExtensions" Version="3.1.4" />
<Compile Include="Configuration\Extensions\Microsoft.Extensions.Configuration\**\*.cs" />
<Compile Include="Configuration\Implementations\Microsoft.Extensions.Configuration\**\*.cs" />
<Compile Include="Sinks\MSSqlServer\Platform\AzureManagedServiceAuthenticatorStub.cs" />
</ItemGroup>

<ItemGroup Condition=" '$(TargetFramework)' == 'net462' ">
Expand All @@ -75,19 +70,16 @@
<Compile Include="Configuration\Extensions\Hybrid\**\*.cs" />
<Compile Include="Configuration\Implementations\Microsoft.Extensions.Configuration\**\*.cs" />
<Compile Include="Configuration\Implementations\System.Configuration\**\*.cs" />
<Compile Include="Sinks\MSSqlServer\Platform\AzureManagedServiceAuthenticatorStub.cs" />
</ItemGroup>

<ItemGroup Condition=" '$(TargetFramework)' == 'netcoreapp3.1' Or '$(TargetFramework)' == 'net472' ">
<PackageReference Include="System.Configuration.ConfigurationManager" Version="4.7.0" />
<PackageReference Include="Microsoft.Data.SqlClient" Version="3.0.0" />
<PackageReference Include="Microsoft.Azure.Services.AppAuthentication" Version="1.4.0" />
<PackageReference Include="Microsoft.Extensions.Configuration" Version="3.1.4" />
<PackageReference Include="Microsoft.Extensions.Options.ConfigurationExtensions" Version="3.1.4" />
<Compile Include="Configuration\Extensions\Hybrid\**\*.cs" />
<Compile Include="Configuration\Implementations\Microsoft.Extensions.Configuration\**\*.cs" />
<Compile Include="Configuration\Implementations\System.Configuration\**\*.cs" />
<Compile Include="Sinks\MSSqlServer\Platform\AzureManagedServiceAuthenticator.cs" />
</ItemGroup>

<ItemGroup>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,12 +21,7 @@ internal static SinkDependencies Create(
var sqlConnectionFactory =
new SqlConnectionFactory(connectionString,
sinkOptions?.EnlistInTransaction ?? default,
sinkOptions?.UseAzureManagedIdentity ?? default,
new SqlConnectionStringBuilderWrapper(),
new AzureManagedServiceAuthenticator(
sinkOptions?.UseAzureManagedIdentity ?? default,
sinkOptions.AzureServiceTokenProviderResource,
sinkOptions.AzureTenantId));
new SqlConnectionStringBuilderWrapper());
var logEventDataGenerator =
new LogEventDataGenerator(columnOptions,
new StandardColumnDataGenerator(columnOptions, formatProvider,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,20 +66,5 @@ internal MSSqlServerSinkOptions(
/// Flag to eagerly emit a batch containing the first received event (default: true)
/// </summary>
public bool EagerlyEmitFirstEvent { get; set; }

/// <summary>
/// Flag to enable SQL authentication using Azure Managed Identities (default: false)
/// </summary>
public bool UseAzureManagedIdentity { get; set; }

/// <summary>
/// Azure service token provider to be used for Azure Managed Identities
/// </summary>
public string AzureServiceTokenProviderResource { get; set; }

/// <summary>
/// ID of the tenant where the Azure resource exists
/// </summary>
public string AzureTenantId { get; set; }
}
}

This file was deleted.

This file was deleted.

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,9 @@ internal class SqlConnectionWrapper : ISqlConnectionWrapper
private readonly SqlConnection _sqlConnection;
private bool _disposedValue;

public SqlConnectionWrapper(string connectionString, string accessToken)
public SqlConnectionWrapper(string connectionString)
{
_sqlConnection = new SqlConnection(connectionString);
_sqlConnection.AccessToken = accessToken;
}

public string ConnectionString => _sqlConnection.ConnectionString;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,42 +6,30 @@ namespace Serilog.Sinks.MSSqlServer.Platform
internal class SqlConnectionFactory : ISqlConnectionFactory
{
private readonly string _connectionString;
private readonly bool _useAzureManagedIdentity;
private readonly ISqlConnectionStringBuilderWrapper _sqlConnectionStringBuilderWrapper;
private readonly IAzureManagedServiceAuthenticator _azureManagedServiceAuthenticator;

public SqlConnectionFactory(
string connectionString,
bool enlistInTransaction,
bool useAzureManagedIdentity,
ISqlConnectionStringBuilderWrapper sqlConnectionStringBuilderWrapper,
IAzureManagedServiceAuthenticator azureManagedServiceAuthenticator)
ISqlConnectionStringBuilderWrapper sqlConnectionStringBuilderWrapper)
{
if (string.IsNullOrWhiteSpace(connectionString))
{
throw new ArgumentNullException(nameof(connectionString));
}
_sqlConnectionStringBuilderWrapper = sqlConnectionStringBuilderWrapper
?? throw new ArgumentNullException(nameof(sqlConnectionStringBuilderWrapper));
_azureManagedServiceAuthenticator = azureManagedServiceAuthenticator
?? throw new ArgumentNullException(nameof(azureManagedServiceAuthenticator));

// Add 'Enlist=false', so that ambient transactions (TransactionScope) will not affect/rollback logging
// unless sink option EnlistInTransaction is set to true.
_sqlConnectionStringBuilderWrapper.ConnectionString = connectionString;
_sqlConnectionStringBuilderWrapper.Enlist = enlistInTransaction;
_connectionString = _sqlConnectionStringBuilderWrapper.ConnectionString;

_useAzureManagedIdentity = useAzureManagedIdentity;
}

public ISqlConnectionWrapper Create()
{
var accessToken = _useAzureManagedIdentity
? _azureManagedServiceAuthenticator.GetAuthenticationToken().GetAwaiter().GetResult()
: default;

return new SqlConnectionWrapper(_connectionString, accessToken);
return new SqlConnectionWrapper(_connectionString);
}
}
}
Loading

0 comments on commit cfa976b

Please sign in to comment.