Skip to content

Commit

Permalink
Additional Checks for ACL configuration (#650)
Browse files Browse the repository at this point in the history 
* check if ACL authenticator is enabled

* add check for acl file definition

* make ACL default

* fix acl test

* rename validation methods
  • Loading branch information
@vazois
vazois committed Sep 9, 2024
1 parent 6c68094 commit 739ef1c
Show file tree
Hide file tree
Showing 4 changed files with 67 additions and 8 deletions.
2 changes: 1 addition & 1 deletion libs/host/Configuration/Options.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ internal sealed class Options
[Option("clean-cluster-config", Required = false, HelpText = "Start with clean cluster config.")]
public bool? CleanClusterConfig { get; set; }

[Option("auth", Required = false, HelpText = "Authentication mode of Garnet. This impacts how AUTH command is processed and how clients are authenticated against Garnet. Value options: NoAuth, Password, Aad, ACL")]
[Option("auth", Required = false, Default = GarnetAuthenticationMode.ACL, HelpText = "Authentication mode of Garnet. This impacts how AUTH command is processed and how clients are authenticated against Garnet. Value options: NoAuth, Password, Aad, ACL")]
public GarnetAuthenticationMode AuthenticationMode { get; set; }

[Option("password", Required = false, HelpText = "Authentication string for password authentication.")]
Expand Down
67 changes: 62 additions & 5 deletions libs/server/Resp/ACLCommands.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@

using System;
using System.Diagnostics;
using System.Text;
using Garnet.common;
using Garnet.server.ACL;
using Garnet.server.Auth;
Expand All @@ -17,6 +16,37 @@ namespace Garnet.server
/// </summary>
internal sealed unsafe partial class RespServerSession : ServerSessionBase
{
private bool ValidateACLAuthenticator()
{
if (_authenticator is null or not GarnetACLAuthenticator)
{
while (!RespWriteUtils.WriteError(CmdStrings.RESP_ERR_ACL_AUTH_DISABLED, ref dcurr, dend))
SendAndReset();
return false;
}
return true;
}

private bool ValidateACLFileUse()
{
if (storeWrapper.serverOptions.AuthSettings is not AclAuthenticationSettings)
{
while (!RespWriteUtils.WriteError(CmdStrings.RESP_ERR_ACL_AUTH_DISABLED, ref dcurr, dend))
SendAndReset();
return false;
}

var aclAuthenticationSettings = (AclAuthenticationSettings)storeWrapper.serverOptions.AuthSettings;
if (aclAuthenticationSettings.AclConfigurationFile == null)
{
while (!RespWriteUtils.WriteError(CmdStrings.RESP_ERR_ACL_AUTH_FILE_DISABLED, ref dcurr, dend))
SendAndReset();
return false;
}

return true;
}

/// <summary>
/// Processes ACL LIST subcommand.
/// </summary>
Expand All @@ -31,8 +61,10 @@ private bool NetworkAclList()
}
else
{
GarnetACLAuthenticator aclAuthenticator = (GarnetACLAuthenticator)_authenticator;
if (!ValidateACLAuthenticator())
return true;

var aclAuthenticator = (GarnetACLAuthenticator)_authenticator;
var users = aclAuthenticator.GetAccessControlList().GetUsers();
while (!RespWriteUtils.WriteArrayLength(users.Count, ref dcurr, dend))
SendAndReset();
Expand Down Expand Up @@ -61,8 +93,10 @@ private bool NetworkAclUsers()
}
else
{
GarnetACLAuthenticator aclAuthenticator = (GarnetACLAuthenticator)_authenticator;
if (!ValidateACLAuthenticator())
return true;

var aclAuthenticator = (GarnetACLAuthenticator)_authenticator;
var users = aclAuthenticator.GetAccessControlList().GetUsers();
while (!RespWriteUtils.WriteArrayLength(users.Count, ref dcurr, dend))
SendAndReset();
Expand Down Expand Up @@ -91,6 +125,9 @@ private bool NetworkAclCat()
}
else
{
if (!ValidateACLAuthenticator())
return true;

var categories = ACLParser.ListCategories();
RespWriteUtils.WriteArrayLength(categories.Count, ref dcurr, dend);

Expand Down Expand Up @@ -118,6 +155,9 @@ private bool NetworkAclSetUser()
}
else
{
if (!ValidateACLAuthenticator())
return true;

var aclAuthenticator = (GarnetACLAuthenticator)_authenticator;

// REQUIRED: username
Expand Down Expand Up @@ -173,8 +213,10 @@ private bool NetworkAclDelUser()
}
else
{
var aclAuthenticator = (GarnetACLAuthenticator)_authenticator;
if (!ValidateACLAuthenticator())
return true;

var aclAuthenticator = (GarnetACLAuthenticator)_authenticator;
var successfulDeletes = 0;

try
Expand Down Expand Up @@ -223,7 +265,10 @@ private bool NetworkAclWhoAmI()
}
else
{
GarnetACLAuthenticator aclAuthenticator = (GarnetACLAuthenticator)_authenticator;
if (!ValidateACLAuthenticator())
return true;

var aclAuthenticator = (GarnetACLAuthenticator)_authenticator;

// Return the name of the currently authenticated user.
Debug.Assert(aclAuthenticator.GetUser() != null);
Expand All @@ -249,6 +294,12 @@ private bool NetworkAclLoad()
}
else
{
if (!ValidateACLAuthenticator())
return true;

if (!ValidateACLFileUse())
return true;

// NOTE: This is temporary as long as ACL operations are only supported when using the ACL authenticator
Debug.Assert(storeWrapper.serverOptions.AuthSettings != null);
Debug.Assert(storeWrapper.serverOptions.AuthSettings.GetType().BaseType == typeof(AclAuthenticationSettings));
Expand Down Expand Up @@ -285,6 +336,12 @@ private bool NetworkAclSave()
SendAndReset();
}

if (!ValidateACLAuthenticator())
return true;

if (!ValidateACLFileUse())
return true;

// NOTE: This is temporary as long as ACL operations are only supported when using the ACL authenticator
Debug.Assert(storeWrapper.serverOptions.AuthSettings != null);
Debug.Assert(storeWrapper.serverOptions.AuthSettings.GetType().BaseType == typeof(AclAuthenticationSettings));
Expand Down
2 changes: 2 additions & 0 deletions libs/server/Resp/CmdStrings.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -176,6 +176,8 @@ static partial class CmdStrings
public static ReadOnlySpan<byte> RESP_ERR_CANNOT_LIST_CLIENTS => "ERR Clients cannot be listed."u8;
public static ReadOnlySpan<byte> RESP_ERR_NO_SUCH_CLIENT => "ERR No such client"u8;
public static ReadOnlySpan<byte> RESP_ERR_INVALID_CLIENT_ID => "ERR Invalid client ID"u8;
public static ReadOnlySpan<byte> RESP_ERR_ACL_AUTH_DISABLED => "ERR ACL Authenticator is disabled."u8;
public static ReadOnlySpan<byte> RESP_ERR_ACL_AUTH_FILE_DISABLED => "ERR This Garnet instance is not configured to use an ACL file. Please restart server with --acl-file option."u8;

/// <summary>
/// Response string templates
Expand Down
4 changes: 2 additions & 2 deletions test/Garnet.test/Resp/ACL/RespCommandTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -208,7 +208,7 @@ static async Task DoAclLoadAsync(GarnetClient client)
}
catch (Exception e)
{
if (e.Message != "ERR Cannot find ACL configuration file ''")
if (e.Message != "ERR This Garnet instance is not configured to use an ACL file. Please restart server with --acl-file option.")
{
throw;
}
Expand All @@ -234,7 +234,7 @@ static async Task DoAclSaveAsync(GarnetClient client)
}
catch (Exception e)
{
if (e.Message != "ERR ACL configuration file not set.")
if (e.Message != "ERR This Garnet instance is not configured to use an ACL file. Please restart server with --acl-file option.")
{
throw;
}
Expand Down

0 comments on commit 739ef1c

Please sign in to comment.