diff --git a/agent/ShopDbEndpointAgent.Core/CertificateRenewal.cs b/agent/ShopDbEndpointAgent.Core/CertificateRenewal.cs index 100c170c..a01329e3 100644 --- a/agent/ShopDbEndpointAgent.Core/CertificateRenewal.cs +++ b/agent/ShopDbEndpointAgent.Core/CertificateRenewal.cs @@ -129,10 +129,16 @@ public class CertificateRenewal // Attach current (expiring) certificate to authenticate renewal request handler.ClientCertificates.Add(currentCert); - // Accept server cert (TODO: validate via ServerCertThumbprint if configured) + // Server certificate validation: MUST validate, never accept blindly handler.ServerCertificateCustomValidationCallback = (sender, cert, chain, errors) => { - if (cert == null) return false; + if (cert == null) + { + logger.LogError("Server provided no certificate during renewal request"); + return false; + } + + // If server cert pinning is configured, validate thumbprint match if (!string.IsNullOrWhiteSpace(config.ServerCertThumbprint)) { var actualThumbprint = cert.GetCertHashString(); @@ -145,16 +151,39 @@ public class CertificateRenewal expectedThumbprint, actualThumbprint); return false; } + logger.LogDebug("Server certificate validated via pinning: {Thumbprint}", actualThumbprint); + return true; } - return true; + + // No pinning configured: validate against Windows trusted roots + // This prevents MITM attacks when pinning is not explicitly configured + if (errors == System.Net.Security.SslPolicyErrors.None) + { + // Certificate is valid and trusted by Windows + return true; + } + + logger.LogError( + "Server certificate validation failed during renewal: {Errors}. " + + "Configure ServerCertThumbprint for certificate pinning or ensure " + + "server certificate is trusted by Windows certificate store.", + errors); + return false; }; using var http = new HttpClient(handler) { Timeout = TimeSpan.FromSeconds(30) }; + + // Add X-API-Key header for API key fallback (when cert is expired and nginx rejects it) + http.DefaultRequestHeaders.Add("X-API-Key", config.ApiKey); + var url = config.BaseUrl.TrimEnd('/') + RENEWAL_ENDPOINT; logger.LogInformation("Requesting certificate renewal from {Url}", url); - var httpResponse = await http.PostAsync(url, new StringContent("", Encoding.UTF8, "application/json")); + // Send hostname in request body for API key fallback authentication + var requestBody = JsonSerializer.Serialize(new { hostname = Environment.MachineName }); + var httpResponse = await http.PostAsync(url, + new StringContent(requestBody, Encoding.UTF8, "application/json")); if (!httpResponse.IsSuccessStatusCode) { var errorBody = await httpResponse.Content.ReadAsStringAsync(); diff --git a/agent/ShopDbEndpointAgent.Core/EndpointCollector.cs b/agent/ShopDbEndpointAgent.Core/EndpointCollector.cs index 08604605..f1d6e8f2 100644 --- a/agent/ShopDbEndpointAgent.Core/EndpointCollector.cs +++ b/agent/ShopDbEndpointAgent.Core/EndpointCollector.cs @@ -1,6 +1,8 @@ using System.Diagnostics; using System.Diagnostics.Eventing.Reader; using System.Management; +using System.Security.Cryptography; +using System.Text; using System.Text.Json; using Microsoft.Extensions.Logging; using Microsoft.Win32; @@ -934,11 +936,15 @@ public sealed class EndpointCollector : ICollector if (!seen.Add(displayName + "\u0000" + displayVersion)) continue; + var fingerprint = ComputeApplicationFingerprint( + displayName, displayVersion, publisher); + result.Add(new { displayname = displayName, displayversion = displayVersion, publisher, + fingerprint, }); } catch (Exception error) @@ -953,6 +959,19 @@ public sealed class EndpointCollector : ICollector } } + // Compute SHA256 fingerprint for application inventory delta optimization. + // Format: SHA256(displayname|displayversion|publisher) as lowercase hex string. + // Matches backend algorithm in plugins/endpoints/plugin.py and backfill script. + // Empty strings are preserved (not converted to null) to match backend behavior. + private static string ComputeApplicationFingerprint( + string displayName, string displayVersion, string publisher) + { + var normalized = $"{displayName}|{displayVersion}|{publisher}"; + var bytes = Encoding.UTF8.GetBytes(normalized); + var hash = SHA256.HashData(bytes); + return Convert.ToHexString(hash).ToLowerInvariant(); + } + // Whether Windows is waiting on a reboot. // // There is no single API for this, so we check the three signals Windows diff --git a/agent/ShopDbEndpointAgent.Tray/ShopDbEndpointAgent.Tray.csproj b/agent/ShopDbEndpointAgent.Tray/ShopDbEndpointAgent.Tray.csproj index ea8ef3a8..6c60b080 100644 --- a/agent/ShopDbEndpointAgent.Tray/ShopDbEndpointAgent.Tray.csproj +++ b/agent/ShopDbEndpointAgent.Tray/ShopDbEndpointAgent.Tray.csproj @@ -92,7 +92,11 @@ No tray code changes, but deployed together after wilm merge. --> - 1.6.8 + + + 1.6.10 true win-x64 true diff --git a/agent/ShopDbEndpointAgent/Program.cs b/agent/ShopDbEndpointAgent/Program.cs index 19735c72..8e0ca23a 100644 --- a/agent/ShopDbEndpointAgent/Program.cs +++ b/agent/ShopDbEndpointAgent/Program.cs @@ -173,8 +173,8 @@ static async Task RunHostAsync(AgentConfig config, string agentVersion, string logDirectory, bool forceConsole) { // Certificate renewal check: if client cert expires within 30 days, request - // renewal from server, install new cert, and exit so service manager restarts us. - // This runs BEFORE the host starts so a renewed cert is picked up immediately. + // renewal from server, install new cert, and gracefully stop so service manager + // restarts us. This runs BEFORE the host starts so a renewed cert is picked up. var renewalLogger = LoggerFactory.Create(logging => ConfigureLogging(logging, logDirectory)).CreateLogger(); var renewal = new CertificateRenewal(renewalLogger, config); @@ -185,12 +185,14 @@ static async Task RunHostAsync(AgentConfig config, string agentVersion, if (didRenew) { renewalLogger.LogInformation( - "Certificate renewed successfully. Exiting so service manager restarts " + - "the agent with new certificate."); - // Exit code 0 so service manager sees this as normal termination and - // restarts the service (if configured as Automatic). The new process - // will pick up the updated thumbprint from appsettings.json. - Environment.Exit(0); + "Certificate renewed successfully. Service will exit gracefully " + + "and service manager will restart with new certificate."); + // Return immediately without starting the host. The service exits gracefully + // with code 0, and Windows Service Manager restarts it (if configured as + // Automatic). The new process picks up the updated thumbprint from + // appsettings.json. This is safer than Environment.Exit(0) which aborts + // in-flight work and can corrupt local state. + return; } } catch (Exception ex) diff --git a/agent/ShopDbEndpointAgent/ShopDbEndpointAgent.csproj b/agent/ShopDbEndpointAgent/ShopDbEndpointAgent.csproj index d34a88df..8b7b22c6 100644 --- a/agent/ShopDbEndpointAgent/ShopDbEndpointAgent.csproj +++ b/agent/ShopDbEndpointAgent/ShopDbEndpointAgent.csproj @@ -233,7 +233,22 @@ variables are expanded BEFORE the checks so %VAR% cannot smuggle a traversal or UNC path past them. Minor because it is a new action name - an older agent asked for it fails closed on the unknown-action default. --> -1.16.0 + + +1.18.0 true win-x64 true diff --git a/docker-compose.yml b/docker-compose.yml index 117d591f..0fefd312 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -99,6 +99,22 @@ services: # time, so the shared mount is not contended. - instance_data:/app/instance + nginx: + image: nginx:1.27-alpine + restart: unless-stopped + depends_on: + - api + ports: + - "${API_PORT:-5001}:443" + volumes: + - ./nginx.conf:/etc/nginx/nginx.conf:ro + - ./certs:/etc/nginx/certs:ro + healthcheck: + test: ["CMD", "wget", "--spider", "-q", "--no-check-certificate", "https://localhost/api/dashboard/health"] + interval: 30s + timeout: 5s + retries: 3 + api: build: . restart: unless-stopped @@ -123,8 +139,11 @@ services: # badge label to DEV here - a raw source build is by definition not a # release - while still allowing an override from .env. SHOPDB_ENV_LABEL: ${SHOPDB_ENV_LABEL:-DEV} + # Enable ProxyFix for nginx reverse proxy + TRUSTED_PROXY_COUNT: 1 ports: - - "${API_PORT:-5001}:5001" + # Expose on localhost only for debugging (bypass nginx) + - "127.0.0.1:5002:5001" # Terminate TLS in gunicorn with the mounted self-signed cert so the browser # sees a secure context. navigator.clipboard and other secure-context-only # web APIs refuse to run over plain http on a non-localhost origin, so the @@ -165,24 +184,16 @@ services: # indistinguishable from a fast one in the log, and latency has to be # reconstructed from outside the container. Keep in sync with the # Dockerfile CMD. + # %({X-Real-IP}i)s shows the real client IP from nginx, not the nginx container IP - --access-logformat - - '%(h)s %(l)s %(u)s %(t)s "%(r)s" %(s)s %(b)s %(D)sus "%(f)s" "%(a)s"' + - '%({X-Real-IP}i)s %(l)s %(u)s %(t)s "%(r)s" %(s)s %(b)s %(D)sus "%(f)s" "%(a)s"' - --error-logfile - "-" - - --certfile - - /app/certs/server.crt - - --keyfile - - /app/certs/server.key + # NO TLS in gunicorn - nginx handles it - wsgi:app healthcheck: - # Match the TLS listener: probe over https and accept the self-signed cert - # (-k). The Dockerfile default probes http and would stay red once gunicorn - # only speaks TLS. - # /api/dashboard/health is the public probe route and answers 200 even when - # the database is down, so the body's status field is what decides health. - # Probing POST /api/auth/login instead logged a 400 every 30s and exercised - # the credential path from a health check. - test: ["CMD-SHELL", "curl --silent --insecure https://localhost:5001/api/dashboard/health | grep -q '\"status\":\"ok\"'"] + # Probe plain HTTP (nginx handles TLS now) + test: ["CMD-SHELL", "curl --silent http://localhost:5001/api/dashboard/health | grep -q '\"status\":\"ok\"'"] interval: 30s timeout: 5s start_period: 10s @@ -194,6 +205,8 @@ services: # site-agent packaging route reads uninstall-shopdbagent.ps1 from here at # download time, so it must be visible in the container. - ./scripts:/app/scripts:ro + # Core modules for development (auth, settings, etc). Dev stack only. + - ./shopdb:/app/shopdb:ro # Self-signed TLS material for the dev/test https listener. Read-only; # generated on the host under ./certs and never baked into the image. - ./certs:/app/certs:ro diff --git a/plugins/endpoints/api/routes.py b/plugins/endpoints/api/routes.py index 1fd8a1c8..1dad1e25 100644 --- a/plugins/endpoints/api/routes.py +++ b/plugins/endpoints/api/routes.py @@ -3449,12 +3449,11 @@ def download_app_package(appversionid: int): def renew_client_certificate(): """Agent self-renewal: generate new cert when expiring < 30 days. - Requires authentication via EXISTING client certificate. Agent checks cert - expiry on startup, POSTs to this endpoint when < 30 days remaining, receives - new certificate as password-protected PFX, installs it, updates config, and - restarts. + Dual-auth: Prefers client certificate (even if expiring), falls back to API key + + hostname for expired certs. This allows agents to renew expired certificates + that would otherwise fail at TLS handshake. - Request: Empty body (agent identity from cert authentication) + Request: {"hostname": "MACHINE-NAME"} (required when using API key fallback) Response: { "pfx": "base64-encoded PFX/P12 certificate with private key", "password": "random password for PFX import (use once, discard)", @@ -3463,7 +3462,8 @@ def renew_client_certificate(): } Security: - - MUST authenticate via valid client certificate (not expired) + - Prefers client certificate authentication (even expired certs) + - Falls back to API key + hostname when cert missing or nginx rejects expired cert - Rate limited: 1 renewal per asset per 24 hours - Logs all renewal operations to AuditLog - New cert valid for 2 years from generation time @@ -3471,16 +3471,44 @@ def renew_client_certificate(): from flask import g import base64 - # Require client certificate authentication - # (Cannot renew without proving you hold current cert) - if not hasattr(g, 'cert_authenticated_asset_id'): - return error_response( - ErrorCodes.UNAUTHORIZED, - 'Certificate renewal requires client certificate authentication', - http_code=401) + # Dual-auth: Try client certificate first (preferred), fall back to API key + hostname + asset_id = None + asset = None - asset_id = g.cert_authenticated_asset_id - asset = db.session.get(Asset, asset_id) + if hasattr(g, 'cert_authenticated_asset_id'): + # Strong authentication: client certificate (may be expiring but still valid for renewal) + asset_id = g.cert_authenticated_asset_id + # Row-level lock to prevent concurrent renewal requests from racing + asset = db.session.query(Asset).with_for_update().filter_by(assetid=asset_id).first() + else: + # Fallback: API key + hostname (for expired certs that nginx rejected at handshake) + error = _collector_auth_or_error() + if error is not None: + return error + + # Get hostname from request body + payload = request.get_json(silent=True) or {} + hostname = (payload.get('hostname') or '').strip() + if not hostname: + return error_response( + ErrorCodes.VALIDATION_ERROR, + 'Certificate renewal without client cert requires hostname in request body', + http_code=400) + + # Resolve hostname to asset + from ..models import EndpointState + state = EndpointState.query.filter_by(hostname=hostname).first() + if not state: + return error_response( + ErrorCodes.NOT_FOUND, + f'Unknown hostname: {hostname}', + http_code=404) + + asset_id = state.assetid + # Row-level lock to prevent concurrent renewal requests from racing + asset = db.session.query(Asset).with_for_update().filter_by(assetid=asset_id).first() + + if not asset: if not asset: return error_response(ErrorCodes.NOT_FOUND, f'Asset {asset_id} not found', http_code=404) @@ -3496,15 +3524,32 @@ def renew_client_certificate(): hostname = state.hostname + # Validate hostname before using in certificate generation + # CN field max length is 64 characters; allow only safe hostname characters + import re + if not hostname or len(hostname) > 64 or not re.match(r'^[a-zA-Z0-9._-]+$', hostname): + return error_response( + ErrorCodes.VALIDATION_ERROR, + f'Invalid hostname format: {hostname}. Must be 1-64 characters, alphanumeric plus dot/hyphen/underscore.', + http_code=400) + # Rate limit: Only allow renewal once per 24 hours per asset # (Prevents abuse if agent gets stuck in renewal loop) + # Use timezone-aware comparison to prevent clock manipulation bypass if asset.cert_issued_at: - hours_since_issue = (datetime.now(timezone.utc).replace(tzinfo=None) - - asset.cert_issued_at).total_seconds() / 3600 - if hours_since_issue < 24: + from datetime import timedelta + now = datetime.now(timezone.utc) + # Ensure cert_issued_at is timezone-aware UTC + issued_at = asset.cert_issued_at.replace(tzinfo=timezone.utc) \ + if asset.cert_issued_at.tzinfo is None \ + else asset.cert_issued_at.astimezone(timezone.utc) + time_since_issue = now - issued_at + + if time_since_issue < timedelta(hours=24): + hours_since = time_since_issue.total_seconds() / 3600 return error_response( ErrorCodes.VALIDATION_ERROR, - f'Certificate was issued {hours_since_issue:.1f} hours ago. ' + f'Certificate was issued {hours_since:.1f} hours ago. ' f'Renewal only allowed after 24 hours.', http_code=429) @@ -3516,6 +3561,16 @@ def renew_client_certificate(): except Exception as e: current_app.logger.error( f'[cert_renewal] Failed to generate certificate for asset {asset_id}: {e}') + # Audit: Certificate generation failure + try: + AuditLog.log( + action='endpoints.cert.renewal_failed', + userid=None, + entityid=asset_id, + entityname=asset.assetnumber, + details=f'Certificate generation failed: {str(e)[:500]}') + except Exception: + pass # Don't fail the error response on audit failure return error_response( ErrorCodes.INTERNAL_ERROR, 'Certificate generation failed', @@ -3533,6 +3588,16 @@ def renew_client_certificate(): db.session.rollback() current_app.logger.error( f'[cert_renewal] Failed to update database for asset {asset_id}: {e}') + # Audit: Database update failure + try: + AuditLog.log( + action='endpoints.cert.db_update_failed', + userid=None, + entityid=asset_id, + entityname=asset.assetnumber, + details=f'Failed to update database with new certificate: {str(e)[:500]}') + except Exception: + pass # Don't fail the error response on audit failure return error_response( ErrorCodes.INTERNAL_ERROR, 'Database update failed', diff --git a/plugins/endpoints/models/endpoint.py b/plugins/endpoints/models/endpoint.py index 66211c65..f0a4cd36 100644 --- a/plugins/endpoints/models/endpoint.py +++ b/plugins/endpoints/models/endpoint.py @@ -464,8 +464,15 @@ class EndpointApplication(db.Model): # When this snapshot row was written (the report that produced it). recordedat = db.Column(db.DateTime, nullable=False, default=datetime.utcnow) + # SHA256 fingerprint for delta-based updates: hash of displayname, displayversion, + # publisher. Allows efficient comparison to detect changes without comparing all + # columns. Computed by agent and sent in payload. Nullable for backward compatibility + # during migration - backfill script will populate existing rows. + app_fingerprint = db.Column(db.String(64), nullable=True) + __table_args__ = ( db.Index('idx_endpointapplications_assetname', 'assetid', 'displayname'), + db.Index('idx_endpointapplications_fingerprint', 'assetid', 'app_fingerprint', unique=True), ) def __repr__(self): diff --git a/shopdb/core/auth/client_cert.py b/shopdb/core/auth/client_cert.py index f783d82b..48a65843 100644 --- a/shopdb/core/auth/client_cert.py +++ b/shopdb/core/auth/client_cert.py @@ -157,10 +157,12 @@ def authenticate_by_client_cert(): return None # Check certificate revocation (cert_revoked_at is set when admin revokes) - if hasattr(asset, 'cert_revoked_at') and asset.cert_revoked_at: + # Fail closed: if column missing (should never happen after migration 0022), + # treat as database error and reject authentication + if asset.cert_revoked_at: current_app.logger.warning( f'[client_cert] Revoked certificate for asset {asset.assetid} ' - f'(revoked {asset.cert_revoked_at})') + f'(revoked {asset.cert_revoked_at}: {asset.cert_revocation_reason or "no reason"})') # Audit: Attempt to use revoked certificate try: AuditLog.log( @@ -171,7 +173,7 @@ def authenticate_by_client_cert(): details=f'Revoked at {asset.cert_revoked_at}: ' f'{asset.cert_revocation_reason or "no reason"}') except Exception: - pass + pass # Don't fail authentication on audit failure return None # Check certificate expiration