Skip to content

Commit 60afe4c

Browse files
jpapiezCopilot
andauthored
fix(security): validate slicer worker API keys (fail closed) + stop leaking ApiKey (#652)
## Summary Fixes a **Critical** pre-existing vulnerability (#650): the slicer worker API-key filters failed **open** and `SlicersController` leaked worker API keys. ## Problem `[RequireSlicerApiKey]` / `[RequireSlicerServiceApiKey]` resolved `ISlicerApiKeyValidator` via `GetService<>()` and, when it was null, **passed the request through**. That validator was **never implemented or registered**, so the filters always failed open → slicer worker REST endpoints (register / rotate-key / heartbeat / deregister / list) were effectively unauthenticated. `SlicersController.ListAsync()` also returned `SlicerService` objects **including the worker `ApiKey`**, exposing worker credentials to any caller. ## Fix - **`SlicerApiKeyValidator`** implemented and registered in `AddSlicerApiServices` (both monolith and slicer-host compositions), so the filters actually validate: - `[RequireSlicerServiceApiKey]` → shared registration key (config/env). - `[RequireSlicerApiKey]` → the per-service key stored on `SlicerService.ApiKey`, scoped to the requested service id; compared with `CryptographicOperations.FixedTimeEquals`; null/empty rejected. - **Filters fail CLOSED** — a missing validator returns 401 in Production (explicit, logged Dev/Testing bypass only, keyed off `IHostEnvironment`). - **No more key leak** — `ListAsync`/`GetAsync` return a redacted `SlicerServiceResponseDto`; only `register`/`rotate-key` return a freshly-issued key to the worker. - **Worker flow preserved** — shared-key/per-service-key resolution treats blank config as absent and uses an aligned precedence on both the worker client and the server validator, so Docker/prod workers (which supply `WorkerAuth__SharedApiKey`) authenticate correctly. Docs updated (`docs/SLICER_WORKER_API_KEYS.md`). ## Ops note Because auth now fails closed, a deployment must configure the shared worker key (`WorkerAuth:SharedKey`/`SharedApiKey`, `SlicerRegistry:ApiKey`, or the `WORKER_SHARED_API_KEY`/`SLICER_REGISTRATION_KEY` env). Documented in `SLICER_WORKER_API_KEYS.md`. ## Validation - `dotnet build` ✅; slicer module tests ✅ (612), worker tests ✅ (89), focused auth tests ✅. No migration (response-DTO redaction only). ## Review Mandatory 3-way adversarial gate; a round caught a real worker-registration break (empty-`??` config precedence) which was fixed. Final verdict unanimous **APPROVE**: Bishop (Opus) ✅ · Hicks (GPT-5.5) ✅ · Vasquez (Gemini 3.1 Pro) ✅. Closes #650 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 67f7293 commit 60afe4c

12 files changed

Lines changed: 627 additions & 51 deletions

File tree

docs/SLICER_WORKER_API_KEYS.md

Lines changed: 16 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -229,10 +229,18 @@ Docker Compose brings up 3 worker containers with corresponding keys.
229229

230230
**Endpoint**: `POST /api/slicers/register`
231231
**Location**: `src/api/Controllers/SlicersController.cs`
232-
**Authentication**:
233-
- Checks for `X-Slicer-ApiKey` header
234-
- Validates against static registry key (for registration) OR service-specific key (for updates)
235-
- Stores ApiKey in `SlicerService` record for future operations
232+
**Authentication**:
233+
- Checks for `X-Slicer-ApiKey` (legacy/current worker header) and
234+
`X-Slicer-Api-Key` (compatibility header).
235+
- `GET /api/slicers` and `POST /api/slicers/register` require the configured
236+
shared registration key (`WorkerAuth:SharedApiKey`, `WorkerAuth:SharedKey`,
237+
`SlicerRegistry:ApiKey`, `WORKER_SHARED_API_KEY`, or
238+
`SLICER_REGISTRATION_KEY`).
239+
- Per-service lifecycle endpoints (`GET /api/slicers/{id}`, heartbeat,
240+
deregister, rotate-key) require the generated key stored on that
241+
`SlicerService.ApiKey` record.
242+
- Read endpoints return a redacted DTO and do not include `apiKey`; register
243+
and rotate-key responses still return the newly issued key to the worker.
236244

237245
**Response**:
238246
```json
@@ -250,8 +258,10 @@ The worker registration flow is implemented in three files:
250258
- Registers SlicerRegistrationClient and RegistrationBackgroundService as hosted services
251259

252260
**2. `src/orcaslicer-worker/Services/SlicerRegistrationClient.cs`** - HTTP communication
253-
- Reads SlicerRegistry:ApiKey from configuration
254-
- Sends X-Slicer-ApiKey header with registration requests
261+
- Reads `SlicerRegistry:ApiKey`, then falls back to `WorkerAuth:SharedApiKey`,
262+
`WorkerAuth:SharedKey`, `WORKER_SHARED_API_KEY`, and
263+
`SLICER_REGISTRATION_KEY` for registration.
264+
- Sends `X-Slicer-ApiKey` header with registration and lifecycle requests.
255265

256266
**3. `src/orcaslicer-worker/Services/RegistrationBackgroundService.cs`** - Lifecycle management
257267
- Automatically calls RegisterAsync() on startup

src/orcaslicer-worker/Services/SlicerRegistrationClient.cs

Lines changed: 24 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -96,14 +96,18 @@ public SlicerRegistrationClient(
9696
string json = JsonSerializer.Serialize(registrationDto);
9797
StringContent content = new StringContent(json, Encoding.UTF8, "application/json");
9898

99-
// Add API key header if configured
100-
string? apiKey = _configuration["SlicerRegistry:ApiKey"];
101-
if (!string.IsNullOrEmpty(apiKey))
99+
using HttpRequestMessage request = new HttpRequestMessage(HttpMethod.Post, $"{_apiBaseUrl}/api/slicers/register")
102100
{
103-
_httpClient.DefaultRequestHeaders.Add("X-Slicer-ApiKey", apiKey);
101+
Content = content
102+
};
103+
104+
string? apiKey = ResolveRegistrationApiKey(_configuration);
105+
if (!string.IsNullOrWhiteSpace(apiKey))
106+
{
107+
request.Headers.Add("X-Slicer-ApiKey", apiKey);
104108
}
105109

106-
HttpResponseMessage response = await _httpClient.PostAsync($"{_apiBaseUrl}/api/slicers/register", content, cancellationToken);
110+
HttpResponseMessage response = await _httpClient.SendAsync(request, cancellationToken);
107111

108112
if (!response.IsSuccessStatusCode)
109113
{
@@ -211,4 +215,19 @@ private class RegistrationResponse
211215
[System.Text.Json.Serialization.JsonPropertyName("apiKey")]
212216
public string ApiKey { get; init; } = string.Empty;
213217
}
218+
219+
internal static string? ResolveRegistrationApiKey(IConfiguration configuration)
220+
{
221+
return FirstNonBlank(
222+
configuration["WorkerAuth:SharedKey"],
223+
configuration["WorkerAuth:SharedApiKey"],
224+
configuration["SlicerRegistry:ApiKey"],
225+
Environment.GetEnvironmentVariable("WORKER_SHARED_API_KEY"),
226+
Environment.GetEnvironmentVariable("SLICER_REGISTRATION_KEY"));
227+
}
228+
229+
private static string? FirstNonBlank(params string?[] candidates)
230+
{
231+
return candidates.FirstOrDefault(candidate => !string.IsNullOrWhiteSpace(candidate));
232+
}
214233
}

src/slicer/Farm.Slicer.Module.Api/Controllers/SlicersController.cs

Lines changed: 26 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,6 @@ namespace Farm.Slicer.Module.Api.Controllers;
1515

1616
// Slicer workers authenticate through the slicer API-key filters, not PrintFarmer bearer tokens.
1717
[AllowAnonymous]
18-
[RequireSlicerApiKey]
1918
public class SlicersController(ISlicersService service) : ControllerBase
2019
{
2120
private readonly ISlicersService _service = service ?? throw new ArgumentNullException(nameof(service));
@@ -24,17 +23,19 @@ public class SlicersController(ISlicersService service) : ControllerBase
2423
/// Lists all registered slicer services.
2524
/// </summary>
2625
[HttpGet]
26+
[RequireSlicerApiKey]
2727
public async Task<IActionResult> ListAsync()
2828
{
2929
IReadOnlyList<SlicerService> list = await _service.ListAsync(HttpContext.RequestAborted);
30-
return Ok(list);
30+
return Ok(list.Select(ToResponseDto).ToList());
3131
}
3232

3333
/// <summary>
3434
/// Registers a new slicer service.
3535
/// </summary>
3636
/// <param name="dto">Registration data.</param>
3737
[HttpPost("register")]
38+
[RequireSlicerApiKey]
3839
public async Task<IActionResult> RegisterAsync([FromBody] RegisterSlicerDto dto)
3940
{
4041
CancellationToken ct = HttpContext.RequestAborted;
@@ -52,7 +53,7 @@ public async Task<IActionResult> RegisterAsync([FromBody] RegisterSlicerDto dto)
5253
public async Task<IActionResult> GetAsync(Guid id)
5354
{
5455
SlicerService? svc = await _service.GetAsync(id, HttpContext.RequestAborted);
55-
return svc == null ? NotFound() : Ok(svc);
56+
return svc == null ? NotFound() : Ok(ToResponseDto(svc));
5657
}
5758

5859
/// <summary>
@@ -91,4 +92,26 @@ public async Task<IActionResult> RotateApiKeyAsync(Guid id)
9192
string? newApiKey = await _service.RotateApiKeyAsync(id, HttpContext.RequestAborted);
9293
return newApiKey == null ? NotFound() : Ok(new { id, apiKey = newApiKey });
9394
}
95+
96+
private static SlicerServiceResponseDto ToResponseDto(SlicerService service)
97+
{
98+
return new SlicerServiceResponseDto
99+
{
100+
Id = service.Id,
101+
Name = service.Name,
102+
SlicerType = service.SlicerType,
103+
Version = service.Version,
104+
Host = service.Host,
105+
UiManifestUrl = service.UiManifestUrl,
106+
CapabilitiesJson = service.CapabilitiesJson,
107+
MaxConcurrentJobs = service.MaxConcurrentJobs,
108+
Status = service.Status,
109+
LastSeen = service.LastSeen,
110+
ApiKeyRotatedAt = service.ApiKeyRotatedAt,
111+
CreatedAt = service.CreatedAt,
112+
UpdatedAt = service.UpdatedAt,
113+
Tags = service.Tags,
114+
InstanceId = service.InstanceId,
115+
};
116+
}
94117
}

src/slicer/Farm.Slicer.Module.Api/Filters/SlicerApiKeyFilters.cs

Lines changed: 72 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
using Microsoft.AspNetCore.Mvc;
22
using Microsoft.AspNetCore.Mvc.Filters;
3+
using Microsoft.Extensions.Hosting;
4+
using Microsoft.Extensions.Logging;
35

46
namespace Farm.Slicer.Module.Api.Filters;
57

@@ -12,20 +14,28 @@ namespace Farm.Slicer.Module.Api.Filters;
1214
public sealed class RequireSlicerApiKeyAttribute : Attribute, IAsyncActionFilter
1315
{
1416
/// <summary>The header name for the slicer API key.</summary>
15-
public const string HeaderName = "X-Slicer-Api-Key";
17+
public const string HeaderName = "X-Slicer-ApiKey";
18+
19+
/// <summary>Alternate dashed header name accepted for compatibility.</summary>
20+
public const string AlternateHeaderName = "X-Slicer-Api-Key";
1621

1722
/// <inheritdoc />
1823
public async Task OnActionExecutionAsync(ActionExecutingContext context, ActionExecutionDelegate next)
1924
{
2025
var validator = context.HttpContext.RequestServices.GetService<ISlicerApiKeyValidator>();
2126
if (validator is null)
2227
{
23-
// No validator registered — pass through (development mode)
24-
await next();
28+
if (SlicerApiKeyFilterHelpers.AllowMissingValidatorInDevelopment(context, nameof(RequireSlicerApiKeyAttribute)))
29+
{
30+
await next();
31+
return;
32+
}
33+
34+
context.Result = new UnauthorizedObjectResult(new { error = "Slicer API key validation is not configured." });
2535
return;
2636
}
2737

28-
string? apiKey = context.HttpContext.Request.Headers[HeaderName].FirstOrDefault();
38+
string? apiKey = SlicerApiKeyFilterHelpers.ReadHeader(context, HeaderName, AlternateHeaderName);
2939
if (!await validator.ValidateSharedKeyAsync(apiKey, context.HttpContext.RequestAborted))
3040
{
3141
context.Result = new UnauthorizedObjectResult(new { error = "Invalid or missing slicer API key." });
@@ -45,27 +55,80 @@ public async Task OnActionExecutionAsync(ActionExecutingContext context, ActionE
4555
public sealed class RequireSlicerServiceApiKeyAttribute : Attribute, IAsyncActionFilter
4656
{
4757
/// <summary>The header name for the per-service API key.</summary>
48-
public const string HeaderName = "X-Slicer-Service-Api-Key";
58+
public const string HeaderName = "X-Slicer-ApiKey";
59+
60+
/// <summary>Alternate service-specific header name accepted for compatibility.</summary>
61+
public const string AlternateHeaderName = "X-Slicer-Service-Api-Key";
4962

5063
/// <inheritdoc />
5164
public async Task OnActionExecutionAsync(ActionExecutingContext context, ActionExecutionDelegate next)
5265
{
5366
var validator = context.HttpContext.RequestServices.GetService<ISlicerApiKeyValidator>();
5467
if (validator is null)
5568
{
56-
await next();
69+
if (SlicerApiKeyFilterHelpers.AllowMissingValidatorInDevelopment(context, nameof(RequireSlicerServiceApiKeyAttribute)))
70+
{
71+
await next();
72+
return;
73+
}
74+
75+
context.Result = new UnauthorizedObjectResult(new { error = "Slicer service API key validation is not configured." });
5776
return;
5877
}
5978

60-
string? apiKey = context.HttpContext.Request.Headers[HeaderName].FirstOrDefault();
61-
if (!await validator.ValidateServiceKeyAsync(apiKey, context.HttpContext.RequestAborted))
79+
string? apiKey = SlicerApiKeyFilterHelpers.ReadHeader(context, HeaderName, AlternateHeaderName, RequireSlicerApiKeyAttribute.AlternateHeaderName);
80+
Guid? serviceId = TryGetServiceId(context);
81+
if (!await validator.ValidateServiceKeyAsync(apiKey, serviceId, context.HttpContext.RequestAborted))
6282
{
6383
context.Result = new UnauthorizedObjectResult(new { error = "Invalid or missing service API key." });
6484
return;
6585
}
6686

6787
await next();
6888
}
89+
90+
private static Guid? TryGetServiceId(ActionExecutingContext context)
91+
{
92+
if (context.RouteData.Values.TryGetValue("id", out object? routeId)
93+
&& Guid.TryParse(routeId?.ToString(), out Guid serviceId))
94+
{
95+
return serviceId;
96+
}
97+
98+
return null;
99+
}
100+
}
101+
102+
internal static class SlicerApiKeyFilterHelpers
103+
{
104+
public static string? ReadHeader(ActionExecutingContext context, params string[] headerNames)
105+
{
106+
foreach (string headerName in headerNames)
107+
{
108+
string? value = context.HttpContext.Request.Headers[headerName].FirstOrDefault();
109+
if (!string.IsNullOrWhiteSpace(value))
110+
{
111+
return value;
112+
}
113+
}
114+
115+
return null;
116+
}
117+
118+
public static bool AllowMissingValidatorInDevelopment(ActionExecutingContext context, string filterName)
119+
{
120+
IHostEnvironment? env = context.HttpContext.RequestServices.GetService<IHostEnvironment>();
121+
ILogger? logger = context.HttpContext.RequestServices.GetService<ILoggerFactory>()?.CreateLogger(filterName);
122+
123+
if (env is not null && (env.IsDevelopment() || env.IsEnvironment("Testing")))
124+
{
125+
logger?.LogWarning("{FilterName} has no ISlicerApiKeyValidator registered; bypassing only because environment is {EnvironmentName}.", filterName, env.EnvironmentName);
126+
return true;
127+
}
128+
129+
logger?.LogError("{FilterName} has no ISlicerApiKeyValidator registered; rejecting request fail-closed.", filterName);
130+
return false;
131+
}
69132
}
70133

71134
/// <summary>
@@ -78,5 +141,5 @@ public interface ISlicerApiKeyValidator
78141
Task<bool> ValidateSharedKeyAsync(string? apiKey, CancellationToken ct = default);
79142

80143
/// <summary>Validate a per-service slicer API key.</summary>
81-
Task<bool> ValidateServiceKeyAsync(string? apiKey, CancellationToken ct = default);
144+
Task<bool> ValidateServiceKeyAsync(string? apiKey, Guid? serviceId, CancellationToken ct = default);
82145
}
Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,83 @@
1+
using System.Security.Cryptography;
2+
using System.Text;
3+
using Farm.Slicer.Module.Api.Filters;
4+
using Farm.Slicer.Module.Data;
5+
using Farm.Slicer.Module.Domain;
6+
using Farm.Slicer.Module.Services.Configuration;
7+
using Microsoft.EntityFrameworkCore;
8+
using Microsoft.Extensions.Configuration;
9+
using Microsoft.Extensions.Hosting;
10+
using Microsoft.Extensions.Logging;
11+
12+
namespace Farm.Slicer.Module.Api.Services;
13+
14+
/// <summary>
15+
/// Validates slicer registry shared keys and per-service worker keys.
16+
/// </summary>
17+
public sealed class SlicerApiKeyValidator(
18+
IConfiguration configuration,
19+
SlicerDbContext db,
20+
IHostEnvironment env,
21+
ILogger<SlicerApiKeyValidator> logger) : ISlicerApiKeyValidator
22+
{
23+
private readonly string? _sharedKey = FirstNonBlank(
24+
configuration.GetSection(WorkerAuthSettings.SectionName)["SharedKey"],
25+
configuration.GetSection(WorkerAuthSettings.SectionName)["SharedApiKey"],
26+
configuration["SlicerRegistry:ApiKey"],
27+
Environment.GetEnvironmentVariable("WORKER_SHARED_API_KEY"),
28+
Environment.GetEnvironmentVariable("SLICER_REGISTRATION_KEY"));
29+
30+
private readonly SlicerDbContext _db = db ?? throw new ArgumentNullException(nameof(db));
31+
private readonly IHostEnvironment _env = env ?? throw new ArgumentNullException(nameof(env));
32+
private readonly ILogger<SlicerApiKeyValidator> _logger = logger ?? throw new ArgumentNullException(nameof(logger));
33+
34+
/// <inheritdoc />
35+
public Task<bool> ValidateSharedKeyAsync(string? apiKey, CancellationToken ct = default)
36+
{
37+
if (string.IsNullOrWhiteSpace(_sharedKey))
38+
{
39+
bool bypass = _env.IsDevelopment() || _env.IsEnvironment("Testing");
40+
if (bypass)
41+
{
42+
_logger.LogWarning("No slicer shared API key is configured; allowing request only because environment is {EnvironmentName}.", _env.EnvironmentName);
43+
}
44+
45+
return Task.FromResult(bypass);
46+
}
47+
48+
return Task.FromResult(FixedTimeEquals(apiKey, _sharedKey));
49+
}
50+
51+
/// <inheritdoc />
52+
public async Task<bool> ValidateServiceKeyAsync(string? apiKey, Guid? serviceId, CancellationToken ct = default)
53+
{
54+
if (string.IsNullOrWhiteSpace(apiKey) || serviceId is null)
55+
{
56+
return false;
57+
}
58+
59+
SlicerService? service = await _db.SlicerServices
60+
.AsNoTracking()
61+
.FirstOrDefaultAsync(s => s.Id == serviceId.Value, ct);
62+
63+
return FixedTimeEquals(apiKey, service?.ApiKey);
64+
}
65+
66+
private static bool FixedTimeEquals(string? presented, string? expected)
67+
{
68+
if (string.IsNullOrWhiteSpace(presented) || string.IsNullOrWhiteSpace(expected))
69+
{
70+
return false;
71+
}
72+
73+
byte[] presentedBytes = Encoding.UTF8.GetBytes(presented);
74+
byte[] expectedBytes = Encoding.UTF8.GetBytes(expected);
75+
return presentedBytes.Length == expectedBytes.Length
76+
&& CryptographicOperations.FixedTimeEquals(presentedBytes, expectedBytes);
77+
}
78+
79+
private static string? FirstNonBlank(params string?[] candidates)
80+
{
81+
return candidates.FirstOrDefault(candidate => !string.IsNullOrWhiteSpace(candidate));
82+
}
83+
}

src/slicer/Farm.Slicer.Module.Api/SlicerApiExtensions.cs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,7 @@ public static IServiceCollection AddSlicerApiServices(this IServiceCollection se
5656
_ = services.AddScoped<ISlicersService, SlicersService>();
5757
_ = services.AddScoped<IProfilesService, ProfilesService>();
5858
_ = services.AddSingleton<IWorkerAuthService, WorkerAuthService>();
59+
_ = services.AddScoped<Filters.ISlicerApiKeyValidator, SlicerApiKeyValidator>();
5960

6061
// Artifact services
6162
_ = services.Configure<Farm.Infrastructure.Settings.ArtifactStorageSettings>(configuration.GetSection(Farm.Infrastructure.Settings.ArtifactStorageSettings.SectionName));

0 commit comments

Comments
 (0)