From 9cccbfae9aea11038bf8c070038085c7e0a8aa17 Mon Sep 17 00:00:00 2001 From: Renn F Date: Sat, 17 Oct 2026 06:33:39 +0200 Subject: [PATCH 1/5] feat: add opt-in Guard Core application layer protection Guard Core (spring-boot-guard 1.2.0) adds per-client request budgets, attack screening with auto-ban, upload limits, access control on the state-changing endpoints and behavior rules. It is off unless guard.enabled=true: the auto-configuration is filtered out before it is imported, so nothing is registered and behavior is unchanged by default. The Kotlin and coroutines BOMs are imported before the Spring Boot BOM because Spring Boot 3.5 manages Kotlin 1.9 and coroutines 1.8, while the guard needs Kotlin 2 and coroutines 1.11. --- pom.xml | 5 + server/pom.xml | 23 +++ server/src/main/config/application.properties | 86 +++++++++++ .../guard/GuardAdminAccessValidator.java | 144 ++++++++++++++++++ .../guard/GuardAutoConfigurationGate.java | 34 +++++ .../guard/GuardClientIdentityReport.java | 48 ++++++ .../keking/web/controller/FileController.java | 16 ++ .../controller/OnlinePreviewController.java | 3 + .../main/resources/META-INF/spring.factories | 1 + 9 files changed, 360 insertions(+) create mode 100644 server/src/main/java/cn/keking/guard/GuardAdminAccessValidator.java create mode 100644 server/src/main/java/cn/keking/guard/GuardAutoConfigurationGate.java create mode 100644 server/src/main/java/cn/keking/guard/GuardClientIdentityReport.java create mode 100644 server/src/main/resources/META-INF/spring.factories diff --git a/pom.xml b/pom.xml index ebdcf33d1..e7ec46521 100644 --- a/pom.xml +++ b/pom.xml @@ -64,6 +64,11 @@ 1.70 + + 1.2.0 + 2.4.21 + 1.11.0 + 1.4.21 2.7.7 diff --git a/server/pom.xml b/server/pom.xml index 0ab813349..062837090 100644 --- a/server/pom.xml +++ b/server/pom.xml @@ -13,6 +13,22 @@ + + + org.jetbrains.kotlin + kotlin-bom + ${kotlin-bom.version} + import + pom + + + org.jetbrains.kotlinx + kotlinx-coroutines-bom + ${kotlinx-coroutines-bom.version} + import + pom + org.springframework.boot spring-boot-dependencies @@ -59,6 +75,13 @@ true + + + com.guard-core + spring-boot-guard + ${spring-boot-guard.version} + + org.jodconverter diff --git a/server/src/main/config/application.properties b/server/src/main/config/application.properties index 9bc1bd9e7..b80f93f3f 100644 --- a/server/src/main/config/application.properties +++ b/server/src/main/config/application.properties @@ -480,3 +480,89 @@ kk.scriptjs = false # 纯文本文件类型，直接显示 simText = ${KK_SIMTEXT:txt,html,htm,asp,jsp,xml,json,properties,md,gitignore,log,java,py,c,cpp,sql,sh,bat,m,bas,prg,cmd} + + +############################################################################### +# 十五、Guard Core 应用层防护（可选，默认关闭，需要重启生效） +############################################################################### +# 开启后在应用内增加：每个客户端的访问频率限制、对路径/查询参数/请求头的攻击特征检测（目录穿越、注入、XSS、扫描探测） +# 并自动封禁持续探测的客户端、上传请求限制，以及对写操作接口的 IP 白名单/密钥保护。 +# 它不替代 trust.host / not.trust.host（SSRF 防护）和各个安全修复，只限制谁能访问、访问多快，并封禁探测者。 +# 注意：Guard 会解码 Base64 参数（url / urlPath / urls / fileName）并检测解码后的内容，但 AES 加密（encryption=aes）的 url 无法检测。 +# 域名、内网地址的校验仍由 TrustHostFilter 完成。 +# 详见 SECURITY_CONFIG.md + +# 总开关，默认 false：false 时不会注册任何 Guard 组件，行为与之前完全一致 +guard.enabled = ${KK_GUARD_ENABLED:false} + +# 仅记录不拦截（观察模式）：先用 true 上线，确认没有误报后再改为 false +guard.passive-mode = ${KK_GUARD_PASSIVE_MODE:false} + +# 反向代理（nginx 等）的 IP/CIDR，多个用逗号隔开。只信任来自这些地址的 X-Forwarded-For，其他来源一律使用 TCP 连接地址。 +# 在反向代理之后部署时必须设置，否则所有客户端会共用代理的地址。 +# 不要同时设置 server.forward-headers-strategy=native/framework，二选一 +guard.trusted-proxies = ${KK_GUARD_TRUSTED_PROXIES:} + +# 默认使用进程内存保存计数和封禁（多实例之间不共享，重启后清空）。 +# 多实例部署需要共享时，设置为 true 并配置 Redis 地址（lettuce 格式，如 redis://:password@127.0.0.1:6379/0） +guard.enable-redis = ${KK_GUARD_ENABLE_REDIS:false} +guard.redis-url = ${KK_GUARD_REDIS_URL:redis://127.0.0.1:6379} + +# 每个客户端的全局请求限制：窗口(秒)内最多请求数。预览页面会加载大量静态资源，所以全局值较大 +guard.enable-rate-limiting = true +guard.rate-limit = ${KK_GUARD_RATE_LIMIT:600} +guard.rate-limit-window = 60 + +# 消耗 CPU 的接口单独限制（转换文档、读取远程文件、入队转换、上传、删除），格式：窗口 60 秒内的最大请求数 +guard.endpoint-rate-limits[/onlinePreview].requests = ${KK_GUARD_RATE_LIMIT_PREVIEW:60} +guard.endpoint-rate-limits[/onlinePreview].window = 60 +guard.endpoint-rate-limits[/picturesPreview].requests = ${KK_GUARD_RATE_LIMIT_PREVIEW:60} +guard.endpoint-rate-limits[/picturesPreview].window = 60 +guard.endpoint-rate-limits[/getCorsFile].requests = ${KK_GUARD_RATE_LIMIT_CORSFILE:120} +guard.endpoint-rate-limits[/getCorsFile].window = 60 +guard.endpoint-rate-limits[/addTask].requests = ${KK_GUARD_RATE_LIMIT_ADDTASK:30} +guard.endpoint-rate-limits[/addTask].window = 60 +guard.endpoint-rate-limits[/fileUpload].requests = ${KK_GUARD_RATE_LIMIT_WRITE:10} +guard.endpoint-rate-limits[/fileUpload].window = 60 +guard.endpoint-rate-limits[/createFolder].requests = ${KK_GUARD_RATE_LIMIT_WRITE:10} +guard.endpoint-rate-limits[/createFolder].window = 60 +guard.endpoint-rate-limits[/deleteFile].requests = ${KK_GUARD_RATE_LIMIT_WRITE:10} +guard.endpoint-rate-limits[/deleteFile].window = 60 + +# 攻击特征检测（路径、查询参数、请求头、请求体）。默认类别不包含： +# ssrf：预览内网地址（url=http://192.168...）是 kkFileView 的正常用法，SSRF 由 trust.host / not.trust.host 负责 +# sensitive_file、recon：会把 .java .py .sql .sh .jsp .asp 等源码文件的访问路径当作探测，而 kkFileView 会预览这类文件 +# （例如压缩包内或 demo 目录中的源码文件），误报会导致这些文件无法预览。大量遍历不同路径的扫描由全局请求限制限速 +# 需要调整时，通过环境变量 KK_GUARD_DETECTION_CATEGORIES 覆盖（逗号分隔） +guard.enable-penetration-detection = true +guard.enabled-detection-categories = ${KK_GUARD_DETECTION_CATEGORIES:xss,sqli,dir_traversal,path_traversal,cmd_injection,file_inclusion,ldap,xml,nosql,file_upload,template,http_split,cms_probing,proto_pollution,code_injection,deserialization} + +# 持续触发检测的客户端自动封禁：累计次数达到阈值后封禁指定秒数 +guard.enable-ip-banning = true +guard.auto-ban-threshold = ${KK_GUARD_AUTO_BAN_THRESHOLD:10} +guard.auto-ban-duration = ${KK_GUARD_AUTO_BAN_DURATION:3600} + +# fileName 是 Base64 编码的 file://localhost/文件名（演示页的删除请求），解码后会被当作 file:// 文件包含，需要排除； +# 文件名本身仍由 FileController 校验。如果使用 local.preview.dir（file:// 预览），还需要把 url、urlPath、urls、currentUrl 加入 guard.excluded-detection-params +guard.excluded-detection-body-fields = fileName + +# 行为规则：同一接口上累计大量 404 的客户端封禁 1 小时（按接口统计，不是按路径统计）。 +# 它能封禁反复读取不存在的远程文件（/getCorsFile）的客户端，但不能封禁遍历大量不同静态路径的扫描器，那类扫描由全局请求限制限制速度 +guard.global-behavior-rules[0].rule-type = return_pattern +guard.global-behavior-rules[0].pattern = status:404 +guard.global-behavior-rules[0].threshold = ${KK_GUARD_BAN_404_THRESHOLD:50} +guard.global-behavior-rules[0].window = 300 +guard.global-behavior-rules[0].action = ban +guard.global-behavior-rules[0].ban-duration = 3600 +# 删除接口按响应内容统计失败次数（删除密码错误），只缓冲该接口的响应 +guard.behavior-scan-response-body = true + +# 响应头和 CORS 保持关闭：kkFileView 常被嵌入 iframe，Guard 的 X-Frame-Options 等响应头会破坏嵌入 +guard.security-headers.enabled = false +guard.enable-cors = false + +# 写操作接口（上传、创建文件夹、删除、addTask）的访问控制，默认都为空（不限制）： +# 允许访问的 IP/CIDR，多个用逗号隔开，例如 127.0.0.1,10.0.0.0/8 +guard.kk.admin-allowed-ips = ${KK_GUARD_ADMIN_ALLOWED_IPS:} +# 访问密钥，调用方在请求头 X-KK-Admin-Key 中携带。同时设置时两个条件都必须满足 +guard.kk.admin-key = ${KK_GUARD_ADMIN_KEY:} diff --git a/server/src/main/java/cn/keking/guard/GuardAdminAccessValidator.java b/server/src/main/java/cn/keking/guard/GuardAdminAccessValidator.java new file mode 100644 index 000000000..d352cf107 --- /dev/null +++ b/server/src/main/java/cn/keking/guard/GuardAdminAccessValidator.java @@ -0,0 +1,144 @@ +package cn.keking.guard; + +import com.guardcore.protocols.GuardRequest; +import com.guardcore.protocols.GuardResponse; +import com.guardcore.spring.CustomRouteValidator; +import kotlin.coroutines.Continuation; +import org.springframework.beans.factory.annotation.Value; +import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty; +import org.springframework.stereotype.Component; + +import java.net.InetAddress; +import java.net.UnknownHostException; +import java.nio.charset.StandardCharsets; +import java.security.MessageDigest; +import java.util.ArrayList; +import java.util.LinkedHashMap; +import java.util.List; +import java.util.Map; +import java.util.regex.Pattern; + +/** + * Limits who may reach the state-changing endpoints (upload, folder creation, delete, addTask). + * Both checks are optional and off by default: an IP allowlist ({@code guard.kk.admin-allowed-ips}) + * and an API key ({@code guard.kk.admin-key}, sent in the {@code X-KK-Admin-Key} header). + * When both are set, both must pass. The client IP is the one Guard Core resolved, so it honours + * {@code guard.trusted-proxies} and ignores a spoofed X-Forwarded-For from any other peer. + */ +@Component(GuardAdminAccessValidator.BEAN_NAME) +@ConditionalOnProperty(name = "guard.enabled", havingValue = "true") +public class GuardAdminAccessValidator implements CustomRouteValidator { + + public static final String BEAN_NAME = "kkGuardAdminAccessValidator"; + static final String KEY_HEADER = "x-kk-admin-key"; + private static final Pattern IPV4_LITERAL = Pattern.compile("(\\d{1,3}\\.){3}\\d{1,3}"); + private static final Pattern IPV6_LITERAL = Pattern.compile("[0-9a-fA-F:.]*:[0-9a-fA-F:.]*"); + + private final List allowedNetworks = new ArrayList<>(); + private final byte[] adminKey; + + public GuardAdminAccessValidator(@Value("${guard.kk.admin-allowed-ips:}") List allowedIps, + @Value("${guard.kk.admin-key:}") String adminKey) { + for (String entry : allowedIps) { + if (!entry.isBlank()) { + allowedNetworks.add(Network.parse(entry.trim())); + } + } + this.adminKey = adminKey.getBytes(StandardCharsets.UTF_8); + } + + @Override + public Object validate(GuardRequest request, Continuation continuation) { + if (isIpAllowed(request) && isKeyAccepted(request)) { + return null; + } + return forbidden(); + } + + private boolean isIpAllowed(GuardRequest request) { + if (allowedNetworks.isEmpty()) { + return true; + } + Object clientIp = request.getState().getAttr("client_ip"); + return clientIp instanceof String ip && allowedNetworks.stream().anyMatch(network -> network.contains(ip)); + } + + private boolean isKeyAccepted(GuardRequest request) { + if (adminKey.length == 0) { + return true; + } + String supplied = request.getHeaders().get(KEY_HEADER); + return supplied != null && MessageDigest.isEqual(adminKey, supplied.getBytes(StandardCharsets.UTF_8)); + } + + private static GuardResponse forbidden() { + byte[] body = "Forbidden".getBytes(StandardCharsets.UTF_8); + Map headers = new LinkedHashMap<>(); + headers.put("Content-Type", "text/plain; charset=utf-8"); + headers.put("Content-Length", String.valueOf(body.length)); + return new GuardResponse() { + @Override public int getStatusCode() { return 403; } + @Override public Map getHeaders() { return headers; } + @Override public byte[] getBody() { return body; } + }; + } + + /** An IP or CIDR literal. Typos fail startup instead of silently widening or narrowing access. */ + record Network(byte[] address, int prefixLength) { + + static Network parse(String entry) { + String[] parts = entry.split("/", -1); + byte[] address = parts.length <= 2 ? literal(parts[0]) : null; + if (address == null) { + throw invalid(entry); + } + int prefix = address.length * 8; + if (parts.length == 2) { + try { + prefix = Integer.parseInt(parts[1]); + } catch (NumberFormatException e) { + throw invalid(entry); + } + if (prefix < 0 || prefix > address.length * 8) { + throw invalid(entry); + } + } + return new Network(address, prefix); + } + + boolean contains(String ip) { + byte[] candidate = literal(ip); + if (candidate == null || candidate.length != address.length) { + return false; + } + int fullBytes = prefixLength / 8; + for (int i = 0; i < fullBytes; i++) { + if (candidate[i] != address[i]) { + return false; + } + } + int remainingBits = prefixLength % 8; + if (remainingBits == 0) { + return true; + } + int mask = 0xFF << (8 - remainingBits); + return (candidate[fullBytes] & mask) == (address[fullBytes] & mask); + } + + /** Only literals reach InetAddress, so this never performs a DNS lookup. IPv4-mapped IPv6 becomes IPv4. */ + private static byte[] literal(String value) { + if (!IPV4_LITERAL.matcher(value).matches() && !IPV6_LITERAL.matcher(value).matches()) { + return null; + } + try { + return InetAddress.getByName(value).getAddress(); + } catch (UnknownHostException e) { + return null; + } + } + + private static IllegalArgumentException invalid(String entry) { + return new IllegalArgumentException("guard.kk.admin-allowed-ips: invalid entry '" + entry + "'"); + } + } +} diff --git a/server/src/main/java/cn/keking/guard/GuardAutoConfigurationGate.java b/server/src/main/java/cn/keking/guard/GuardAutoConfigurationGate.java new file mode 100644 index 000000000..f03f521ad --- /dev/null +++ b/server/src/main/java/cn/keking/guard/GuardAutoConfigurationGate.java @@ -0,0 +1,34 @@ +package cn.keking.guard; + +import org.springframework.boot.autoconfigure.AutoConfigurationImportFilter; +import org.springframework.boot.autoconfigure.AutoConfigurationMetadata; +import org.springframework.context.EnvironmentAware; +import org.springframework.core.env.Environment; + +/** + * Keeps Guard Core out of the application unless {@code guard.enabled=true}. + * spring-boot-guard activates itself whenever it is on the classpath, so the opt-in + * is enforced here, before its auto-configuration is imported: when the property is + * absent or false no guard bean, filter or property binding exists. + */ +public class GuardAutoConfigurationGate implements AutoConfigurationImportFilter, EnvironmentAware { + + static final String GUARD_AUTO_CONFIGURATION = "com.guardcore.spring.GuardSecurityAutoConfiguration"; + + private Environment environment; + + @Override + public void setEnvironment(Environment environment) { + this.environment = environment; + } + + @Override + public boolean[] match(String[] autoConfigurationClasses, AutoConfigurationMetadata autoConfigurationMetadata) { + boolean enabled = environment.getProperty("guard.enabled", Boolean.class, false); + boolean[] outcome = new boolean[autoConfigurationClasses.length]; + for (int i = 0; i < outcome.length; i++) { + outcome[i] = enabled || !GUARD_AUTO_CONFIGURATION.equals(autoConfigurationClasses[i]); + } + return outcome; + } +} diff --git a/server/src/main/java/cn/keking/guard/GuardClientIdentityReport.java b/server/src/main/java/cn/keking/guard/GuardClientIdentityReport.java new file mode 100644 index 000000000..eaa88bc13 --- /dev/null +++ b/server/src/main/java/cn/keking/guard/GuardClientIdentityReport.java @@ -0,0 +1,48 @@ +package cn.keking.guard; + +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; +import org.springframework.boot.autoconfigure.condition.ConditionalOnProperty; +import org.springframework.boot.cloud.CloudPlatform; +import org.springframework.boot.context.properties.bind.Bindable; +import org.springframework.boot.context.properties.bind.Binder; +import org.springframework.core.env.Environment; +import org.springframework.stereotype.Component; + +import java.util.List; + +/** + * Logs, once at startup, which layer decides the client address, so Tomcat and Guard Core cannot disagree + * about proxy trust unnoticed. Tomcat rewrites the remote address only when server.forward-headers-strategy + * is native or framework (or a cloud platform that implies it); Guard Core trusts X-Forwarded-For only from + * guard.trusted-proxies. Use one of the two, not both. + */ +@Component +@ConditionalOnProperty(name = "guard.enabled", havingValue = "true") +public class GuardClientIdentityReport { + + private static final Logger logger = LoggerFactory.getLogger(GuardClientIdentityReport.class); + + public GuardClientIdentityReport(Environment environment) { + List trustedProxies = Binder.get(environment) + .bind("guard.trusted-proxies", Bindable.listOf(String.class)).orElse(List.of()); + String strategy = environment.getProperty("server.forward-headers-strategy"); + CloudPlatform platform = CloudPlatform.getActive(environment); + boolean tomcatRewrites = strategy != null + ? !"none".equalsIgnoreCase(strategy) + : platform != null && platform.isUsingForwardHeaders(); + if (tomcatRewrites && !trustedProxies.isEmpty()) { + logger.warn("Guard Core: Tomcat already rewrites the client address from forwarded headers " + + "(server.forward-headers-strategy) and guard.trusted-proxies is also set. Use only one of them, " + + "and keep server.tomcat.remoteip.internal-proxies as narrow as guard.trusted-proxies."); + } else if (tomcatRewrites) { + logger.info("Guard Core: client address comes from Tomcat (server.forward-headers-strategy), " + + "guard.trusted-proxies is empty."); + } else if (trustedProxies.isEmpty()) { + logger.info("Guard Core: client address is the TCP peer, X-Forwarded-For is ignored. Behind a reverse proxy " + + "set guard.trusted-proxies, otherwise every client shares the proxy address."); + } else { + logger.info("Guard Core: client address is the TCP peer, X-Forwarded-For is trusted only from {}.", trustedProxies); + } + } +} diff --git a/server/src/main/java/cn/keking/web/controller/FileController.java b/server/src/main/java/cn/keking/web/controller/FileController.java index 57e77e06c..f9bbca7e6 100644 --- a/server/src/main/java/cn/keking/web/controller/FileController.java +++ b/server/src/main/java/cn/keking/web/controller/FileController.java @@ -1,12 +1,18 @@ package cn.keking.web.controller; import cn.keking.config.ConfigConstants; +import cn.keking.guard.GuardAdminAccessValidator; import cn.keking.model.FileType; import cn.keking.model.ReturnResponse; import cn.keking.utils.CaptchaUtil; import cn.keking.utils.DateUtils; import cn.keking.utils.KkFileUtils; import cn.keking.utils.WebUtils; +import com.guardcore.spring.GuardContentTypeFilter; +import com.guardcore.spring.GuardCustomValidation; +import com.guardcore.spring.GuardDetectionExclusion; +import com.guardcore.spring.GuardMaxRequestSize; +import com.guardcore.spring.GuardReturnMonitor; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.springframework.util.ObjectUtils; @@ -153,6 +159,12 @@ public String getStats() { } } + // Same ceiling as spring.servlet.multipart.max-request-size, checked on Content-Length before the body is parsed. + @GuardContentTypeFilter(allowedTypes = "multipart/form-data") + @GuardMaxRequestSize(sizeBytes = 524288000) + // Path, query and headers are still scanned. The multipart body is not: reading it for the scan leaves the upload without its file part. + @GuardDetectionExclusion(scanBody = false) + @GuardCustomValidation(validatorBean = GuardAdminAccessValidator.BEAN_NAME) @PostMapping("/fileUpload") public ReturnResponse fileUpload(@RequestParam("file") MultipartFile file, @RequestParam(value = "path", defaultValue = "") String path) { @@ -180,6 +192,7 @@ public ReturnResponse fileUpload(@RequestParam("file") MultipartFile fil } } + @GuardCustomValidation(validatorBean = GuardAdminAccessValidator.BEAN_NAME) @PostMapping("/createFolder") public ReturnResponse createFolder(@RequestParam(value = "path", defaultValue = "") String path, @RequestParam("folderName") String folderName) { @@ -237,6 +250,9 @@ private Path createUploadDirectory(String requestedPath) throws IOException { return current; } + @GuardCustomValidation(validatorBean = GuardAdminAccessValidator.BEAN_NAME) + // Every failed delete (wrong password, unknown file) answers {"code":1,...}: ban a client that keeps failing, i.e. guessing the password. + @GuardReturnMonitor(pattern = "\"code\":1", maxOccurrences = 5, window = 3600, action = "ban") @PostMapping("/deleteFile") public ReturnResponse deleteFile(HttpServletRequest request, String fileName, String password) { ReturnResponse checkResult = this.deleteFileCheck(request, fileName, password); diff --git a/server/src/main/java/cn/keking/web/controller/OnlinePreviewController.java b/server/src/main/java/cn/keking/web/controller/OnlinePreviewController.java index 3d5e4d155..9120f79e7 100644 --- a/server/src/main/java/cn/keking/web/controller/OnlinePreviewController.java +++ b/server/src/main/java/cn/keking/web/controller/OnlinePreviewController.java @@ -1,6 +1,7 @@ package cn.keking.web.controller; import cn.keking.config.ConfigConstants; +import cn.keking.guard.GuardAdminAccessValidator; import cn.keking.model.FileAttribute; import cn.keking.service.FileHandlerService; import cn.keking.service.FilePreview; @@ -8,6 +9,7 @@ import cn.keking.service.cache.CacheService; import cn.keking.service.impl.OtherFilePreviewImpl; import cn.keking.utils.*; +import com.guardcore.spring.GuardCustomValidation; import fr.opensagres.xdocreport.core.io.IOUtils; import org.apache.commons.codec.binary.Base64; import org.apache.hc.client5.http.impl.classic.CloseableHttpClient; @@ -268,6 +270,7 @@ public void getCorsFile(@RequestParam String urlPath, * * @param url 请编码后在入队 */ + @GuardCustomValidation(validatorBean = GuardAdminAccessValidator.BEAN_NAME) @GetMapping("/addTask") @ResponseBody public String addQueueTask(@RequestParam String url, diff --git a/server/src/main/resources/META-INF/spring.factories b/server/src/main/resources/META-INF/spring.factories new file mode 100644 index 000000000..5608a2e0b --- /dev/null +++ b/server/src/main/resources/META-INF/spring.factories @@ -0,0 +1 @@ +org.springframework.boot.autoconfigure.AutoConfigurationImportFilter=cn.keking.guard.GuardAutoConfigurationGate From 529e81d8f19697d124bfb6fe91e8c6e0728ca464 Mon Sep 17 00:00:00 2001 From: Renn F Date: Sat, 17 Oct 2026 06:33:40 +0200 Subject: [PATCH 2/5] test: cover the opt-in Guard Core integration Runs the real application with the guard off and on: no component registered and unchanged responses by default; budgets give 429; traversal, XSS and scanner probes are rejected and repeated probing is banned; upload content type and size; admin allowlist and key; intranet previews are not blocked; a spoofed X-Forwarded-For does not reset a budget; large downloads stream. --- .../keking/guard/GuardAdminAccessTests.java | 68 ++++ .../guard/GuardAdminAccessValidatorTests.java | 94 ++++++ .../GuardAutoConfigurationGateTests.java | 35 ++ .../guard/GuardClientIdentityReportTests.java | 81 +++++ .../cn/keking/guard/GuardDisabledTests.java | 44 +++ .../cn/keking/guard/GuardEnabledTests.java | 309 ++++++++++++++++++ .../cn/keking/guard/GuardHttpTestSupport.java | 81 +++++ .../keking/guard/GuardPassiveModeTests.java | 25 ++ .../guard/GuardUntrustedProxyTests.java | 39 +++ 9 files changed, 776 insertions(+) create mode 100644 server/src/test/java/cn/keking/guard/GuardAdminAccessTests.java create mode 100644 server/src/test/java/cn/keking/guard/GuardAdminAccessValidatorTests.java create mode 100644 server/src/test/java/cn/keking/guard/GuardAutoConfigurationGateTests.java create mode 100644 server/src/test/java/cn/keking/guard/GuardClientIdentityReportTests.java create mode 100644 server/src/test/java/cn/keking/guard/GuardDisabledTests.java create mode 100644 server/src/test/java/cn/keking/guard/GuardEnabledTests.java create mode 100644 server/src/test/java/cn/keking/guard/GuardHttpTestSupport.java create mode 100644 server/src/test/java/cn/keking/guard/GuardPassiveModeTests.java create mode 100644 server/src/test/java/cn/keking/guard/GuardUntrustedProxyTests.java diff --git a/server/src/test/java/cn/keking/guard/GuardAdminAccessTests.java b/server/src/test/java/cn/keking/guard/GuardAdminAccessTests.java new file mode 100644 index 000000000..bc67675b7 --- /dev/null +++ b/server/src/test/java/cn/keking/guard/GuardAdminAccessTests.java @@ -0,0 +1,68 @@ +package cn.keking.guard; + +import org.junit.jupiter.api.Test; +import org.springframework.test.context.TestPropertySource; + +import java.net.http.HttpResponse; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotEquals; + +/** Access control on the state-changing endpoints: an IP allowlist and an API key, both required when both are set. */ +@TestPropertySource(properties = { + "guard.enabled=true", + "guard.trusted-proxies=127.0.0.1", + "guard.rate-limit=1000", + "guard.kk.admin-allowed-ips=203.0.113.0/24,2001:db8::/32", + "guard.kk.admin-key=s3cret" +}) +class GuardAdminAccessTests extends GuardHttpTestSupport { + + private static final String ALLOWED_IP = "203.0.113.9"; + private static final String KEY_HEADER = "X-KK-Admin-Key"; + + private static void assertRefused(HttpResponse response) { + assertEquals(403, response.statusCode()); + assertEquals("Forbidden", response.body()); + } + + private static void assertReachedTheController(HttpResponse response) { + assertNotEquals("Forbidden", response.body()); + } + + @Test + void deleteIsRefusedFromAnAddressOutsideTheAllowlist() throws Exception { + assertRefused(postForm("/deleteFile", newClientIp(), "fileName=x&password=y", KEY_HEADER, "s3cret")); + } + + @Test + void deleteIsRefusedWithoutTheKeyOrWithAWrongKey() throws Exception { + assertRefused(postForm("/deleteFile", ALLOWED_IP, "fileName=x&password=y")); + assertRefused(postForm("/deleteFile", ALLOWED_IP, "fileName=x&password=y", KEY_HEADER, "wrong")); + } + + @Test + void deleteReachesTheControllerFromTheAllowlistWithTheKey() throws Exception { + assertReachedTheController(postForm("/deleteFile", ALLOWED_IP, "fileName=x&password=y", KEY_HEADER, "s3cret")); + assertReachedTheController(postForm("/deleteFile", "2001:db8::17", "fileName=x&password=y", KEY_HEADER, "s3cret")); + } + + @Test + void createFolderUploadAndAddTaskAreProtectedToo() throws Exception { + String outsider = newClientIp(); + assertRefused(postForm("/createFolder", outsider, "folderName=a")); + assertRefused(postMultipart("/fileUpload", outsider)); + assertRefused(get("/addTask?url=" + base64("http://files.example/a.pdf"), outsider)); + + assertReachedTheController(postForm("/createFolder", ALLOWED_IP, "folderName=a", KEY_HEADER, "s3cret")); + assertReachedTheController(postMultipart("/fileUpload", ALLOWED_IP, KEY_HEADER, "s3cret")); + } + + @Test + void previewAndListingStayOpen() throws Exception { + String outsider = newClientIp(); + assertNotEquals("Forbidden", get("/onlinePreview?url=" + base64("http://files.example/a.pdf"), outsider).body()); + assertEquals(200, postForm("/listFiles", outsider, "path=&page=0&size=5").statusCode()); + assertEquals(200, status("/index", outsider)); + } +} diff --git a/server/src/test/java/cn/keking/guard/GuardAdminAccessValidatorTests.java b/server/src/test/java/cn/keking/guard/GuardAdminAccessValidatorTests.java new file mode 100644 index 000000000..aa8a92717 --- /dev/null +++ b/server/src/test/java/cn/keking/guard/GuardAdminAccessValidatorTests.java @@ -0,0 +1,94 @@ +package cn.keking.guard; + +import com.guardcore.protocols.GuardRequest; +import com.guardcore.protocols.GuardResponse; +import com.guardcore.protocols.RequestState; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; + +import java.util.List; +import java.util.Map; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertInstanceOf; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +class GuardAdminAccessValidatorTests { + + private static GuardRequest request(String clientIp, Map headers) { + GuardRequest request = mock(GuardRequest.class); + RequestState state = mock(RequestState.class); + when(state.getAttr("client_ip")).thenReturn(clientIp); + when(request.getState()).thenReturn(state); + when(request.getHeaders()).thenReturn(headers); + return request; + } + + private static Object validate(GuardAdminAccessValidator validator, String clientIp, Map headers) { + return validator.validate(request(clientIp, headers), null); + } + + private static void assertForbidden(Object outcome) { + GuardResponse response = assertInstanceOf(GuardResponse.class, outcome); + assertEquals(403, response.getStatusCode()); + } + + @Test + void allowsEveryoneWhenNothingIsConfigured() { + GuardAdminAccessValidator validator = new GuardAdminAccessValidator(List.of(), ""); + assertNull(validate(validator, "198.51.100.1", Map.of())); + } + + @Test + void allowlistAcceptsAddressesAndCidrRangesOnly() { + GuardAdminAccessValidator validator = new GuardAdminAccessValidator(List.of("10.0.0.0/8", "192.0.2.7", "2001:db8::/32"), ""); + assertNull(validate(validator, "10.200.3.4", Map.of())); + assertNull(validate(validator, "192.0.2.7", Map.of())); + assertNull(validate(validator, "2001:db8:1::5", Map.of())); + assertForbidden(validate(validator, "11.0.0.1", Map.of())); + assertForbidden(validate(validator, "192.0.2.8", Map.of())); + assertForbidden(validate(validator, "2001:db9::1", Map.of())); + assertForbidden(validate(validator, null, Map.of())); + assertForbidden(validate(validator, "not-an-ip", Map.of())); + } + + @Test + void prefixesThatAreNotAMultipleOfEightBitsAreHonoured() { + GuardAdminAccessValidator validator = new GuardAdminAccessValidator(List.of("172.16.0.0/12"), ""); + assertNull(validate(validator, "172.31.255.255", Map.of())); + assertForbidden(validate(validator, "172.32.0.1", Map.of())); + } + + @Test + void anIpv4MappedIpv6PeerMatchesAnIpv4Entry() { + GuardAdminAccessValidator validator = new GuardAdminAccessValidator(List.of("192.0.2.0/24"), ""); + assertNull(validate(validator, "::ffff:192.0.2.9", Map.of())); + } + + @Test + void keyMustMatchExactly() { + GuardAdminAccessValidator validator = new GuardAdminAccessValidator(List.of(), "s3cret"); + assertNull(validate(validator, "198.51.100.1", Map.of("x-kk-admin-key", "s3cret"))); + assertForbidden(validate(validator, "198.51.100.1", Map.of())); + assertForbidden(validate(validator, "198.51.100.1", Map.of("x-kk-admin-key", "s3cret "))); + assertForbidden(validate(validator, "198.51.100.1", Map.of("x-kk-admin-key", "S3CRET"))); + } + + @Test + void bothMustPassWhenBothAreConfigured() { + GuardAdminAccessValidator validator = new GuardAdminAccessValidator(List.of("192.0.2.0/24"), "s3cret"); + assertNull(validate(validator, "192.0.2.9", Map.of("x-kk-admin-key", "s3cret"))); + assertForbidden(validate(validator, "192.0.2.9", Map.of())); + assertForbidden(validate(validator, "198.51.100.1", Map.of("x-kk-admin-key", "s3cret"))); + } + + @ParameterizedTest + @ValueSource(strings = {"10.0.0", "10.0.0.256", "10.0.0.0/33", "10.0.0.0/-1", "10.0.0.0/8/8", "example.com", "::1/129", "10.0.0.0/x"}) + void aMistypedEntryFailsStartup(String entry) { + assertThrows(IllegalArgumentException.class, () -> new GuardAdminAccessValidator(List.of(entry), "")); + } +} diff --git a/server/src/test/java/cn/keking/guard/GuardAutoConfigurationGateTests.java b/server/src/test/java/cn/keking/guard/GuardAutoConfigurationGateTests.java new file mode 100644 index 000000000..ee3f099f7 --- /dev/null +++ b/server/src/test/java/cn/keking/guard/GuardAutoConfigurationGateTests.java @@ -0,0 +1,35 @@ +package cn.keking.guard; + +import org.junit.jupiter.api.Test; +import org.springframework.mock.env.MockEnvironment; + +import static org.junit.jupiter.api.Assertions.assertArrayEquals; + +class GuardAutoConfigurationGateTests { + + private static final String[] CLASSES = { + GuardAutoConfigurationGate.GUARD_AUTO_CONFIGURATION, + "org.springframework.boot.autoconfigure.web.servlet.WebMvcAutoConfiguration" + }; + + private static boolean[] match(MockEnvironment environment) { + GuardAutoConfigurationGate gate = new GuardAutoConfigurationGate(); + gate.setEnvironment(environment); + return gate.match(CLASSES, null); + } + + @Test + void keepsGuardOutWhenThePropertyIsMissing() { + assertArrayEquals(new boolean[]{false, true}, match(new MockEnvironment())); + } + + @Test + void keepsGuardOutWhenThePropertyIsFalse() { + assertArrayEquals(new boolean[]{false, true}, match(new MockEnvironment().withProperty("guard.enabled", "false"))); + } + + @Test + void importsGuardOnlyWhenThePropertyIsTrue() { + assertArrayEquals(new boolean[]{true, true}, match(new MockEnvironment().withProperty("guard.enabled", "true"))); + } +} diff --git a/server/src/test/java/cn/keking/guard/GuardClientIdentityReportTests.java b/server/src/test/java/cn/keking/guard/GuardClientIdentityReportTests.java new file mode 100644 index 000000000..5d720a853 --- /dev/null +++ b/server/src/test/java/cn/keking/guard/GuardClientIdentityReportTests.java @@ -0,0 +1,81 @@ +package cn.keking.guard; + +import ch.qos.logback.classic.Level; +import ch.qos.logback.classic.Logger; +import ch.qos.logback.classic.spi.ILoggingEvent; +import ch.qos.logback.core.read.ListAppender; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.slf4j.LoggerFactory; +import org.springframework.mock.env.MockEnvironment; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertTrue; + +class GuardClientIdentityReportTests { + + private final Logger logger = (Logger) LoggerFactory.getLogger(GuardClientIdentityReport.class); + private final ListAppender logs = new ListAppender<>(); + + @BeforeEach + void captureLogs() { + logs.start(); + logger.addAppender(logs); + } + + @AfterEach + void releaseLogs() { + logger.detachAppender(logs); + } + + private ILoggingEvent report(MockEnvironment environment) { + new GuardClientIdentityReport(environment); + assertEquals(1, logs.list.size()); + return logs.list.get(0); + } + + @Test + void warnsWhenTomcatAndGuardBothRewriteTheClientAddress() { + ILoggingEvent event = report(new MockEnvironment() + .withProperty("server.forward-headers-strategy", "native") + .withProperty("guard.trusted-proxies[0]", "10.0.0.0/8")); + assertEquals(Level.WARN, event.getLevel()); + assertTrue(event.getFormattedMessage().contains("Use only one of them")); + } + + @Test + void saysTheTcpPeerIsUsedWhenNothingIsTrusted() { + ILoggingEvent event = report(new MockEnvironment()); + assertEquals(Level.INFO, event.getLevel()); + assertTrue(event.getFormattedMessage().contains("TCP peer, X-Forwarded-For is ignored")); + } + + @Test + void namesTheTrustedProxies() { + ILoggingEvent event = report(new MockEnvironment().withProperty("guard.trusted-proxies", "10.0.0.7")); + assertEquals(Level.INFO, event.getLevel()); + assertTrue(event.getFormattedMessage().contains("[10.0.0.7]")); + } + + @Test + void saysTomcatDecidesWhenOnlyTomcatRewrites() { + ILoggingEvent event = report(new MockEnvironment().withProperty("server.forward-headers-strategy", "framework")); + assertEquals(Level.INFO, event.getLevel()); + assertTrue(event.getFormattedMessage().contains("comes from Tomcat")); + } + + @Test + void anExplicitNoneKeepsTomcatOutOfIt() { + ILoggingEvent event = report(new MockEnvironment().withProperty("server.forward-headers-strategy", "none")); + assertTrue(event.getFormattedMessage().contains("TCP peer")); + } + + @Test + void aCloudPlatformThatImpliesForwardedHeadersCountsAsTomcatRewriting() { + ILoggingEvent event = report(new MockEnvironment() + .withProperty("VCAP_APPLICATION", "{}") + .withProperty("guard.trusted-proxies", "10.0.0.7")); + assertEquals(Level.WARN, event.getLevel()); + } +} diff --git a/server/src/test/java/cn/keking/guard/GuardDisabledTests.java b/server/src/test/java/cn/keking/guard/GuardDisabledTests.java new file mode 100644 index 000000000..66bea0fc7 --- /dev/null +++ b/server/src/test/java/cn/keking/guard/GuardDisabledTests.java @@ -0,0 +1,44 @@ +package cn.keking.guard; + +import com.guardcore.spring.GuardFilter; +import com.guardcore.spring.GuardMiddleware; +import org.junit.jupiter.api.Test; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.boot.web.servlet.FilterRegistrationBean; +import org.springframework.context.ApplicationContext; + +import java.net.http.HttpResponse; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNotEquals; +import static org.junit.jupiter.api.Assertions.assertTrue; + +/** The shipped defaults: Guard Core is on the classpath but nothing of it is registered and behaviour is unchanged. */ +class GuardDisabledTests extends GuardHttpTestSupport { + + @Autowired + ApplicationContext context; + + @Test + void registersNoGuardComponent() { + assertEquals(0, context.getBeanNamesForType(GuardMiddleware.class).length); + assertEquals(0, context.getBeanNamesForType(GuardFilter.class).length); + assertTrue(context.getBeansOfType(FilterRegistrationBean.class).values().stream() + .noneMatch(registration -> registration.getFilter() instanceof GuardFilter)); + assertEquals(0, context.getBeanNamesForType(GuardAdminAccessValidator.class).length); + assertFalse(context.containsBean("guardJsonConverterOrdering")); + } + + @Test + void leavesRequestsUntouched() throws Exception { + String client = newClientIp(); + for (int i = 0; i < 700; i++) { + assertEquals(200, status("/index", client), "request " + i + " was throttled"); + } + HttpResponse attack = get("/index?q=%3Cscript%3Ealert(1)%3C/script%3E", client); + assertEquals(200, attack.statusCode()); + assertNotEquals("Suspicious activity detected", attack.body()); + assertTrue(attack.headers().firstValue("X-Content-Type-Options").isEmpty()); + } +} diff --git a/server/src/test/java/cn/keking/guard/GuardEnabledTests.java b/server/src/test/java/cn/keking/guard/GuardEnabledTests.java new file mode 100644 index 000000000..492e63fc4 --- /dev/null +++ b/server/src/test/java/cn/keking/guard/GuardEnabledTests.java @@ -0,0 +1,309 @@ +package cn.keking.guard; + +import cn.keking.config.ConfigConstants; +import com.sun.net.httpserver.HttpServer; +import jakarta.servlet.Filter; +import jakarta.servlet.http.HttpServletRequest; +import org.junit.jupiter.api.AfterAll; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; +import org.springframework.boot.test.context.TestConfiguration; +import org.springframework.boot.web.servlet.FilterRegistrationBean; +import org.springframework.context.annotation.Bean; +import org.springframework.core.Ordered; +import org.springframework.test.context.TestPropertySource; +import org.springframework.web.util.ContentCachingResponseWrapper; + +import java.io.InputStream; +import java.io.OutputStream; +import java.net.InetSocketAddress; +import java.net.Socket; +import java.net.URLEncoder; +import java.net.http.HttpResponse; +import java.nio.charset.StandardCharsets; +import java.nio.file.Path; +import java.util.Map; +import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.TimeUnit; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNotEquals; +import static org.junit.jupiter.api.Assertions.assertTrue; + +/** Guard Core switched on with the shipped configuration, plus small budgets so the limits can be reached in a test. */ +@TestPropertySource(properties = { + "guard.enabled=true", + "guard.trusted-proxies=127.0.0.1", + "guard.rate-limit=8", + "guard.endpoint-rate-limits[/picturesPreview].requests=3", + "guard.auto-ban-threshold=3", + // a list in a test property source replaces the whole configured list, so the rule is repeated in full + "guard.global-behavior-rules[0].rule-type=return_pattern", + "guard.global-behavior-rules[0].pattern=status:404", + "guard.global-behavior-rules[0].threshold=3", + "guard.global-behavior-rules[0].window=300", + "guard.global-behavior-rules[0].action=ban", + "guard.global-behavior-rules[0].ban-duration=3600" +}) +class GuardEnabledTests extends GuardHttpTestSupport { + + private static final int CHUNK_BYTES = 1024 * 1024; + private static final Map RESPONSE_TYPES = new ConcurrentHashMap<>(); + + @TempDir + static Path fileDir; + private static String previousFileDir; + private static HttpServer upstream; + private static final CountDownLatch RELEASE_REST_OF_FILE = new CountDownLatch(1); + + /** Records which response object the application receives, so a caching wrapper would be visible. */ + @TestConfiguration + static class ResponseTypeProbe { + @Bean + FilterRegistrationBean responseTypeProbeFilter() { + FilterRegistrationBean registration = new FilterRegistrationBean<>((request, response, chain) -> { + RESPONSE_TYPES.put(((HttpServletRequest) request).getRequestURI(), + response instanceof ContentCachingResponseWrapper ? "buffered" : "streamed"); + chain.doFilter(request, response); + }); + registration.setOrder(Ordered.LOWEST_PRECEDENCE); + return registration; + } + } + + @BeforeEach + void previewAnyHostIntoTheTemporaryDirectory() { + // The application re-reads its configuration file while starting, so this is set after the context is up. + ConfigConstants.setTrustHostValue("*"); + ConfigConstants.setFileDirValue(fileDir.toString()); + } + + @BeforeAll + static void startUpstream() throws Exception { + previousFileDir = ConfigConstants.getFileDir(); + upstream = HttpServer.create(new InetSocketAddress("127.0.0.1", 0), 0); + upstream.createContext("/hello.png", exchange -> { + byte[] body = "hello from the intranet".getBytes(StandardCharsets.UTF_8); + exchange.getResponseHeaders().add("Content-Type", "image/png"); + exchange.sendResponseHeaders(200, body.length); + try (OutputStream out = exchange.getResponseBody()) { + out.write(body); + } + }); + upstream.createContext("/missing", exchange -> { + exchange.sendResponseHeaders(404, -1); + exchange.close(); + }); + upstream.createContext("/big.pdf", exchange -> { + exchange.getResponseHeaders().add("Content-Type", "application/pdf"); + exchange.sendResponseHeaders(200, 0); + try (OutputStream out = exchange.getResponseBody()) { + byte[] chunk = new byte[CHUNK_BYTES]; + out.write(chunk); + out.flush(); + try { + RELEASE_REST_OF_FILE.await(60, TimeUnit.SECONDS); + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + } + for (int i = 0; i < 7; i++) { + out.write(chunk); + } + } + }); + upstream.start(); + } + + @AfterAll + static void stopUpstreamAndRestoreConfiguration() { + RELEASE_REST_OF_FILE.countDown(); + upstream.stop(0); + ConfigConstants.setTrustHostValue("default"); + ConfigConstants.setFileDirValue(previousFileDir); + } + + private static String upstreamUrl(String path) { + return "http://127.0.0.1:" + upstream.getAddress().getPort() + path; + } + + @Test + void globalBudgetGivesTooManyRequests() throws Exception { + String client = newClientIp(); + for (int i = 0; i < 8; i++) { + assertEquals(200, status("/index", client), "request " + i); + } + assertEquals(429, status("/index", client)); + assertEquals(200, status("/index", newClientIp()), "another client keeps its own budget"); + } + + @Test + void expensiveEndpointHasItsOwnSmallerBudget() throws Exception { + String client = newClientIp(); + for (int i = 0; i < 3; i++) { + assertNotEquals(429, status("/picturesPreview", client), "request " + i); + } + assertEquals(429, status("/picturesPreview", client)); + assertEquals(200, status("/index", client), "the other routes are still within the global budget"); + } + + @Test + void traversalAndXssInTheQueryAreRejected() throws Exception { + assertEquals(400, status("/index?file=../../../../etc/passwd", newClientIp())); + assertEquals(400, status("/index?q=%3Cscript%3Ealert(1)%3C/script%3E", newClientIp())); + assertEquals(400, status("/index?x=1%27%20OR%201%3D1--", newClientIp())); + } + + @Test + void traversalInAFormBodyFieldIsRejected() throws Exception { + assertEquals(400, postForm("/listFiles", newClientIp(), "path=../../../../etc&page=0&size=5").statusCode()); + assertEquals(200, postForm("/listFiles", newClientIp(), "path=&page=0&size=5").statusCode()); + } + + @Test + void scannerPathIsRejected() throws Exception { + assertEquals(400, status("/wp-login.php", newClientIp())); + } + + @Test + void clientThatKeepsProbingIsBanned() throws Exception { + String attacker = newClientIp(); + assertEquals(400, status("/index?file=../../../../etc/passwd", attacker)); + for (int i = 0; i < 2; i++) { + status("/index?file=../../../../etc/passwd", attacker); + } + assertEquals(403, status("/index", attacker), "banned after repeated attacks"); + assertEquals(200, status("/index", newClientIp()), "other clients are not affected"); + } + + @Test + void clientThatKeepsHittingTheSameMissingPathIsBanned() throws Exception { + String client = newClientIp(); + for (int i = 0; i < 4; i++) { + assertEquals(404, status("/no-such-file", client)); + } + assertEquals(403, status("/index", client), "banned after repeated 404 responses on one endpoint"); + } + + @Test + void clientThatKeepsRequestingMissingRemoteFilesIsBanned() throws Exception { + String client = newClientIp(); + for (int i = 0; i < 4; i++) { + assertEquals(404, status("/getCorsFile?urlPath=" + base64(upstreamUrl("/missing/" + i + ".pdf")), client)); + } + assertEquals(403, status("/index", client), "the handler is one endpoint, whatever the remote file"); + } + + @Test + void notFoundResponsesAreCountedPerEndpointNotPerPath() throws Exception { + String scanner = newClientIp(); + for (int i = 0; i < 6; i++) { + assertEquals(404, status("/no-such-file-" + i, scanner)); + } + assertEquals(200, status("/index", scanner), "a scan over distinct paths is only slowed down by the budgets, not banned"); + } + + @Test + void clientThatKeepsFailingToDeleteIsBanned() throws Exception { + String client = newClientIp(); + String body = "fileName=" + base64("file://localhost/x.txt") + "&password=guess"; + for (int i = 0; i < 6; i++) { + HttpResponse response = postForm("/deleteFile", client, body); + assertEquals(200, response.statusCode(), "attempt " + i + " is screened and reaches the controller"); + assertTrue(response.body().contains("\"code\":1")); + } + assertEquals(403, postForm("/deleteFile", client, body).statusCode(), "banned after repeated failures"); + } + + @Test + void intranetPreviewUrlIsNotBlocked() throws Exception { + String client = newClientIp(); + String source = upstreamUrl("/hello.png"); + HttpResponse preview = send(request("/onlinePreview?url=" + URLEncoder.encode(base64(source), StandardCharsets.UTF_8)) + .header("X-Forwarded-For", client) + .header("Referer", "http://192.168.1.10/portal/documents") + .GET().build()); + assertEquals(200, preview.statusCode()); + assertTrue(preview.body().contains(source), "the preview page was rendered for the intranet address"); + + HttpResponse proxied = send(request("/getCorsFile?urlPath=" + URLEncoder.encode(base64(source), StandardCharsets.UTF_8)) + .header("X-Forwarded-For", client) + .header("Referer", "http://192.168.1.10/portal/documents") + .GET().build()); + assertEquals(200, proxied.statusCode()); + assertEquals("hello from the intranet", proxied.body()); + } + + @Test + void ordinaryPreviewRequestsAreNotFlagged() throws Exception { + // Untrusted hosts are refused by the application right after the guard, so nothing is downloaded here. + ConfigConstants.setTrustHostValue("default"); + String client = newClientIp(); + String[] sources = { + "https://oss-cn-hangzhou.aliyuncs.com/bucket/%E5%90%88%E5%90%8C%20(%E7%BB%88%E7%89%88).docx?Expires=1760000000&OSSAccessKeyId=LTAI5tAbCdEf&Signature=aB%2Bc%2FdE%3D", + "https://files.example.com/O%27Brien%20notes.xlsx?token=eyJhbGciOiJIUzI1NiJ9.eyJzdWIiOiIxIn0.c2lnbmF0dXJl", + "ftp://user:p%40ss@10.0.0.5:21/share/report.pdf", + "http://192.168.1.10:8080/files/a--b%20select%20union.pdf", + "http://127.0.0.1:9000/bucket/a&b+c,d.pptx;jsessionid=0123456789abcdef", + "https://h.example/a.pdf?x=1&y=%E4%B8%AD%E6%96%87#page=3" + }; + for (String source : sources) { + String url = URLEncoder.encode(base64(source), StandardCharsets.UTF_8); + String optional = "&fullfilename=O%27Brien%20%E5%90%88%E5%90%8C.docx&watermarkTxt=O%27Brien%20%26%20Sons%20%3Cinternal%3E&page=2&kkagent=false"; + assertNotEquals(400, status("/onlinePreview?url=" + url + optional, client), source); + } + } + + @Test + void uploadWithTheWrongContentTypeIsRejected() throws Exception { + assertEquals(415, postForm("/fileUpload", newClientIp(), "a=b").statusCode()); + } + + @Test + void uploadAboveTheSizeLimitIsRejectedBeforeTheBodyIsRead() throws Exception { + String request = "POST /fileUpload HTTP/1.1\r\nHost: 127.0.0.1\r\nX-Forwarded-For: " + newClientIp() + "\r\n" + + "Content-Type: multipart/form-data; boundary=kkguard\r\nContent-Length: 600000000\r\nConnection: close\r\n\r\n"; + try (Socket socket = new Socket("127.0.0.1", port)) { + socket.getOutputStream().write(request.getBytes(StandardCharsets.UTF_8)); + socket.getOutputStream().flush(); + String statusLine = new String(socket.getInputStream().readNBytes(12), StandardCharsets.UTF_8); + assertEquals("HTTP/1.1 413", statusLine); + } + } + + @Test + void wellFormedUploadReachesTheController() throws Exception { + HttpResponse response = postMultipart("/fileUpload", newClientIp()); + assertEquals(200, response.statusCode()); + assertTrue(response.body().contains("\"code\":1"), "the controller answered, upload is disabled by default"); + } + + @Test + void largeCorsFileDownloadStreamsWhileTheHandlerIsStillRunning() throws Exception { + String path = "/getCorsFile?urlPath=" + base64(upstreamUrl("/big.pdf")); + HttpResponse response = client.send( + request(path).header("X-Forwarded-For", newClientIp()).GET().build(), HttpResponse.BodyHandlers.ofInputStream()); + assertEquals(200, response.statusCode()); + assertEquals("streamed", RESPONSE_TYPES.get("/getCorsFile")); + try (InputStream body = response.body()) { + // Tomcat keeps the last partial 8 KB block of what it was given until more arrives, so ask for a bit less than the whole chunk. + byte[] first = body.readNBytes(CHUNK_BYTES - 64 * 1024); + assertEquals(CHUNK_BYTES - 64 * 1024, first.length); + assertEquals(1, RELEASE_REST_OF_FILE.getCount(), "the first chunk arrived before the upstream finished"); + RELEASE_REST_OF_FILE.countDown(); + assertEquals(8L * CHUNK_BYTES - first.length, body.readAllBytes().length); + } + } + + @Test + void guardResponsesAndSecurityHeadersStayOff() throws Exception { + HttpResponse response = get("/index", newClientIp()); + assertTrue(response.headers().firstValue("X-Content-Type-Options").isEmpty()); + assertTrue(response.headers().firstValue("X-Frame-Options").isEmpty()); + assertTrue(response.headers().firstValue("Strict-Transport-Security").isEmpty()); + assertTrue(response.headers().firstValue("Access-Control-Allow-Origin").isEmpty()); + } +} diff --git a/server/src/test/java/cn/keking/guard/GuardHttpTestSupport.java b/server/src/test/java/cn/keking/guard/GuardHttpTestSupport.java new file mode 100644 index 000000000..d0e2bc048 --- /dev/null +++ b/server/src/test/java/cn/keking/guard/GuardHttpTestSupport.java @@ -0,0 +1,81 @@ +package cn.keking.guard; + +import cn.keking.ServerMain; +import cn.keking.service.OfficePluginManager; +import org.springframework.beans.factory.annotation.Value; +import org.springframework.boot.test.context.SpringBootTest; +import org.springframework.test.context.bean.override.mockito.MockitoBean; + +import java.io.IOException; +import java.net.URI; +import java.net.http.HttpClient; +import java.net.http.HttpRequest; +import java.net.http.HttpResponse; +import java.nio.charset.StandardCharsets; +import java.util.Base64; +import java.util.concurrent.atomic.AtomicInteger; + +/** + * Boots the real application on a random port. The Office manager is replaced by a mock so the tests do not start LibreOffice. + * Every test uses its own client address (sent as X-Forwarded-For) so budgets and bans never leak between tests. + */ +@SpringBootTest(classes = ServerMain.class, webEnvironment = SpringBootTest.WebEnvironment.RANDOM_PORT) +abstract class GuardHttpTestSupport { + + private static final AtomicInteger NEXT_CLIENT = new AtomicInteger(1); + + @MockitoBean + OfficePluginManager officePluginManager; + + @Value("${local.server.port}") + int port; + + final HttpClient client = HttpClient.newBuilder().version(HttpClient.Version.HTTP_1_1).build(); + + static String newClientIp() { + return "198.51.100." + NEXT_CLIENT.getAndIncrement(); + } + + static String base64(String value) { + return Base64.getEncoder().encodeToString(value.getBytes(StandardCharsets.UTF_8)); + } + + HttpRequest.Builder request(String pathAndQuery) { + return HttpRequest.newBuilder(URI.create("http://127.0.0.1:" + port + pathAndQuery)); + } + + HttpResponse get(String pathAndQuery, String clientIp) throws IOException, InterruptedException { + return send(request(pathAndQuery).header("X-Forwarded-For", clientIp).GET().build()); + } + + int status(String pathAndQuery, String clientIp) throws IOException, InterruptedException { + return get(pathAndQuery, clientIp).statusCode(); + } + + HttpResponse send(HttpRequest request) throws IOException, InterruptedException { + return client.send(request, HttpResponse.BodyHandlers.ofString()); + } + + HttpResponse postForm(String path, String clientIp, String body, String... extraHeaders) throws IOException, InterruptedException { + HttpRequest.Builder builder = request(path).header("X-Forwarded-For", clientIp) + .header("Content-Type", "application/x-www-form-urlencoded") + .POST(HttpRequest.BodyPublishers.ofString(body)); + for (int i = 0; i < extraHeaders.length; i += 2) { + builder.header(extraHeaders[i], extraHeaders[i + 1]); + } + return send(builder.build()); + } + + HttpResponse postMultipart(String path, String clientIp, String... extraHeaders) throws IOException, InterruptedException { + String boundary = "kkguard"; + String body = "--" + boundary + "\r\nContent-Disposition: form-data; name=\"file\"; filename=\"a.txt\"\r\n" + + "Content-Type: text/plain\r\n\r\nhello\r\n--" + boundary + "--\r\n"; + HttpRequest.Builder builder = request(path).header("X-Forwarded-For", clientIp) + .header("Content-Type", "multipart/form-data; boundary=" + boundary) + .POST(HttpRequest.BodyPublishers.ofString(body)); + for (int i = 0; i < extraHeaders.length; i += 2) { + builder.header(extraHeaders[i], extraHeaders[i + 1]); + } + return send(builder.build()); + } +} diff --git a/server/src/test/java/cn/keking/guard/GuardPassiveModeTests.java b/server/src/test/java/cn/keking/guard/GuardPassiveModeTests.java new file mode 100644 index 000000000..51b8fe58a --- /dev/null +++ b/server/src/test/java/cn/keking/guard/GuardPassiveModeTests.java @@ -0,0 +1,25 @@ +package cn.keking.guard; + +import org.junit.jupiter.api.Test; +import org.springframework.test.context.TestPropertySource; + +import static org.junit.jupiter.api.Assertions.assertEquals; + +/** The observation mode used to roll the guard out: violations are only logged, nothing is blocked. */ +@TestPropertySource(properties = { + "guard.enabled=true", + "guard.passive-mode=true", + "guard.trusted-proxies=127.0.0.1", + "guard.rate-limit=3" +}) +class GuardPassiveModeTests extends GuardHttpTestSupport { + + @Test + void attacksAndOverBudgetRequestsAreLoggedNotBlocked() throws Exception { + String client = newClientIp(); + assertEquals(200, status("/index?file=../../../../etc/passwd", client)); + for (int i = 0; i < 6; i++) { + assertEquals(200, status("/index", client), "request " + i); + } + } +} diff --git a/server/src/test/java/cn/keking/guard/GuardUntrustedProxyTests.java b/server/src/test/java/cn/keking/guard/GuardUntrustedProxyTests.java new file mode 100644 index 000000000..f3b6081f4 --- /dev/null +++ b/server/src/test/java/cn/keking/guard/GuardUntrustedProxyTests.java @@ -0,0 +1,39 @@ +package cn.keking.guard; + +import org.junit.jupiter.api.Test; +import org.springframework.test.context.TestPropertySource; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotEquals; + +/** No trusted proxy is configured: the client is the TCP peer and X-Forwarded-For is never believed. */ +@TestPropertySource(properties = { + "guard.enabled=true", + "guard.trusted-proxies=", + "guard.rate-limit=1000", + "guard.endpoint-rate-limits[/picturesPreview].requests=3", + "guard.kk.admin-allowed-ips=203.0.113.0/24" +}) +class GuardUntrustedProxyTests extends GuardHttpTestSupport { + + @Test + void spoofedForwardedForDoesNotResetTheBudget() throws Exception { + for (int i = 0; i < 3; i++) { + assertNotEquals(429, status("/picturesPreview", newClientIp()), "request " + i); + } + assertEquals(429, status("/picturesPreview", newClientIp()), "a fresh spoofed address does not buy a fresh budget"); + } + + @Test + void spoofedForwardedForDoesNotGrantAdminAccess() throws Exception { + assertEquals(403, postForm("/createFolder", "203.0.113.9", "folderName=a").statusCode()); + } + + @Test + void aLoopbackClientIsRejectedButNeverBanned() throws Exception { + for (int i = 0; i < 12; i++) { + assertEquals(400, status("/index?file=../../../../etc/passwd", "ignored")); + } + assertEquals(200, status("/index", "ignored"), "the engine does not ban loopback addresses"); + } +} From 0c5b6123c786fd6601853b3fa43e4dca03094ac6 Mon Sep 17 00:00:00 2001 From: Renn F Date: Sat, 17 Oct 2026 06:33:40 +0200 Subject: [PATCH 3/5] docs: describe the opt-in Guard Core protection and its limits --- SECURITY_CONFIG.md | 39 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 39 insertions(+) diff --git a/SECURITY_CONFIG.md b/SECURITY_CONFIG.md index ad96085bb..eb33f2839 100644 --- a/SECURITY_CONFIG.md +++ b/SECURITY_CONFIG.md @@ -155,6 +155,45 @@ trust.host = *.example.com - `*.example.com` 会匹配 `cdn.example.com`、`api.internal.example.com`，但不匹配根域 `example.com` - 对于 IP 风格通配（如 `192.168.*`、`10.*`），仅匹配字面量 IPv4 地址，不匹配域名 +## 🧱 应用层防护 Guard Core（可选，默认关闭） + +kkFileView 可以选择启用 [Guard Core](https://github.com/Guard-Core/spring-boot-guard)（spring-boot-guard，MIT），在应用内增加请求频率限制、攻击特征检测和自动封禁。它**不替代** `trust.host` / `not.trust.host` 和各个安全修复，只限制谁能访问这些接口、访问多快，并封禁持续探测的客户端。 + +Guard Core is an optional, off-by-default application-layer guard (rate limits, attack screening, auto-ban). It does not replace `trust.host` / `not.trust.host` or the code fixes for the advisories. It limits who can reach the routes, how fast, and bans clients that keep probing. + +### 启用 / Enable + +```properties +guard.enabled = true +# 或环境变量 / or: KK_GUARD_ENABLED=true +``` + +不设置 `guard.enabled` 或设置为 `false` 时，不会注册任何 Guard 组件，行为与之前完全一致。建议先用 `KK_GUARD_PASSIVE_MODE=true`（只记录不拦截）观察一段时间，再关闭观察模式。完整配置见 `application.properties` 的“十五、Guard Core 应用层防护”。 + +### 它做什么 / What it does + +| 风险 / Risk | 措施 / Measure | 配置 / Setting | +|---|---|---| +| 转换、预览、远程读取、入队被刷（LibreOffice 转换消耗大量 CPU） | 每个客户端的全局和分接口请求限制，超限返回 429 | `guard.rate-limit`、`guard.endpoint-rate-limits[...]` | +| 目录穿越、注入、XSS、扫描探测（路径、查询参数、请求头、请求体，包含 Base64 解码后的 url） | 攻击特征检测，返回 400，持续触发的客户端自动封禁 | `guard.enabled-detection-categories`、`guard.auto-ban-*` | +| 反复读取不存在的远程文件 | 同一接口累计大量 404 的客户端封禁 | `guard.global-behavior-rules[0]` | +| 删除密码被猜解 | 删除失败超过 5 次的客户端封禁 1 小时 | `FileController#deleteFile` 上的 `@GuardReturnMonitor` | +| 上传请求过大或类型不对 | `/fileUpload` 只接受 `multipart/form-data`，Content-Length 超过 500MB 直接返回 413 | `FileController#fileUpload` 上的注解 | +| 写操作接口被匿名调用（上传、创建文件夹、删除、addTask） | IP 白名单和/或密钥（请求头 `X-KK-Admin-Key`），同时设置时两者都必须满足 | `KK_GUARD_ADMIN_ALLOWED_IPS`、`KK_GUARD_ADMIN_KEY` | + +### 诚实的限制 / Honest limits + +- 不检测 `ssrf` 类别：预览内网地址是正常用法，SSRF 仍由 `trust.host` / `not.trust.host` 负责。Guard 会解码 Base64 的 `url` 并检测解码后的内容，但 AES 加密（`encryption=aes`）的 `url` 无法检测。 +- 默认不启用 `sensitive_file` 和 `recon` 类别：它们会把 `.java`、`.py`、`.sql`、`.sh`、`.jsp`、`.asp` 等源码文件的访问路径当作探测，而 kkFileView 会预览这类文件。可以用 `KK_GUARD_DETECTION_CATEGORIES` 覆盖。 +- 使用 `local.preview.dir`（`file://` 预览）时，需要把 `url`、`urlPath`、`urls`、`currentUrl` 加入 `guard.excluded-detection-params`，否则 `file://` 会被当作文件包含攻击。 +- 404 规则按接口统计，不是按路径统计：遍历大量不同静态路径的扫描器只会被请求限制限速，不会被封禁。 +- Guard 看不到上传文件部分的内容，`Content-Type` 检查的是请求而不是文件部分，文件类型仍由 `prohibit` 控制。声明的 Content-Length 达到 2GB 或以上时，引擎的大小检查会抛出异常并返回 500（请求仍然被拒绝）。 +- 引擎不会封禁回环地址（127.0.0.1）：来自回环地址的探测会被拒绝，但不会被封禁。 +- 计数和封禁默认保存在进程内存中：多实例之间不共享，重启后清空。需要共享时设置 `KK_GUARD_ENABLE_REDIS=true` 和 `KK_GUARD_REDIS_URL`（独立于 `cache.type=redis`）。 +- 全局限制默认 600 次/分钟：预览页面会加载大量静态资源，限制按客户端地址统计，办公室出口共用一个地址时请调大 `KK_GUARD_RATE_LIMIT`。 +- 响应头和 CORS 保持关闭：kkFileView 常被嵌入 iframe，Guard 的 `X-Frame-Options` 会破坏嵌入。 +- 反向代理之后必须设置 `KK_GUARD_TRUSTED_PROXIES`（代理的 IP/CIDR），否则所有客户端共用代理的地址。只信任来自这些地址的 `X-Forwarded-For`。不要同时设置 `server.forward-headers-strategy=native/framework`，两者二选一；启动日志会说明当前由谁决定客户端地址。 + ## 🚨 安全事件响应 如果发现可疑的预览请求： From f08e45b3d6697be2e2f2f330974c4c9482bd3473 Mon Sep 17 00:00:00 2001 From: Renn F Date: Sat, 17 Oct 2026 12:52:04 +0200 Subject: [PATCH 4/5] feat: switch the guard with the spring-boot-guard 1.3.0 guard.enabled property Bump spring-boot-guard to 1.3.0 and drop GuardAutoConfigurationGate and its spring.factories entry: the library now honors guard.enabled itself. Its default is true, so ServerMain repeats guard.enabled=false as the lowest-precedence default, which keeps the guard off for operators whose own application.properties copy predates it. --- pom.xml | 2 +- server/src/main/config/application.properties | 2 +- .../src/main/java/cn/keking/ServerMain.java | 15 +++++-- .../guard/GuardAutoConfigurationGate.java | 34 --------------- .../main/resources/META-INF/spring.factories | 1 - .../java/cn/keking/ServerMainTestAccess.java | 14 +++++++ ...ActuatorOverrideExposureSecurityTests.java | 3 +- .../GuardAutoConfigurationGateTests.java | 35 ---------------- .../cn/keking/guard/GuardDisabledTests.java | 14 +++++++ .../cn/keking/guard/GuardEnabledTests.java | 14 +++++++ .../guard/GuardOffWithoutConfigFileTests.java | 42 +++++++++++++++++++ 11 files changed, 100 insertions(+), 76 deletions(-) delete mode 100644 server/src/main/java/cn/keking/guard/GuardAutoConfigurationGate.java delete mode 100644 server/src/main/resources/META-INF/spring.factories create mode 100644 server/src/test/java/cn/keking/ServerMainTestAccess.java delete mode 100644 server/src/test/java/cn/keking/guard/GuardAutoConfigurationGateTests.java create mode 100644 server/src/test/java/cn/keking/guard/GuardOffWithoutConfigFileTests.java diff --git a/pom.xml b/pom.xml index e7ec46521..2899d076f 100644 --- a/pom.xml +++ b/pom.xml @@ -65,7 +65,7 @@ 1.70 - 1.2.0 + 1.3.0 2.4.21 1.11.0 diff --git a/server/src/main/config/application.properties b/server/src/main/config/application.properties index b80f93f3f..d86dd8bc8 100644 --- a/server/src/main/config/application.properties +++ b/server/src/main/config/application.properties @@ -492,7 +492,7 @@ simText = ${KK_SIMTEXT:txt,html,htm,asp,jsp,xml,json,properties,md,gitignore,log # 域名、内网地址的校验仍由 TrustHostFilter 完成。 # 详见 SECURITY_CONFIG.md -# 总开关，默认 false：false 时不会注册任何 Guard 组件，行为与之前完全一致 +# 总开关（spring-boot-guard 的 guard.enabled，库的默认值是 true，这里改为默认 false）：false 时不会注册任何 Guard 组件，行为与之前完全一致 guard.enabled = ${KK_GUARD_ENABLED:false} # 仅记录不拦截（观察模式）：先用 true 上线，确认没有误报后再改为 false diff --git a/server/src/main/java/cn/keking/ServerMain.java b/server/src/main/java/cn/keking/ServerMain.java index 0e5b52988..0fb413014 100644 --- a/server/src/main/java/cn/keking/ServerMain.java +++ b/server/src/main/java/cn/keking/ServerMain.java @@ -18,12 +18,21 @@ public class ServerMain { private static final Logger logger = LoggerFactory.getLogger(ServerMain.class); + /** + * spring-boot-guard is on unless {@code guard.enabled=false}. The shipped application.properties turns it off, but + * {@code -Dspring.config.location} replaces that file with the operator's own copy, so the opt-in default is repeated + * here as the lowest-precedence property: any config file, environment variable or argument still wins. + */ + static SpringApplicationBuilder applicationBuilder() { + return new SpringApplicationBuilder(ServerMain.class) + .logStartupInfo(false) + .properties("guard.enabled=false"); + } + public static void main(String[] args) { StopWatch stopWatch = new StopWatch(); stopWatch.start(); - ConfigurableApplicationContext context = new SpringApplicationBuilder(ServerMain.class) - .logStartupInfo(false) - .run(args); + ConfigurableApplicationContext context = applicationBuilder().run(args); stopWatch.stop(); ServerProperties serverProperties = context.getBean(ServerProperties.class); Integer port = serverProperties.getPort(); diff --git a/server/src/main/java/cn/keking/guard/GuardAutoConfigurationGate.java b/server/src/main/java/cn/keking/guard/GuardAutoConfigurationGate.java deleted file mode 100644 index f03f521ad..000000000 --- a/server/src/main/java/cn/keking/guard/GuardAutoConfigurationGate.java +++ /dev/null @@ -1,34 +0,0 @@ -package cn.keking.guard; - -import org.springframework.boot.autoconfigure.AutoConfigurationImportFilter; -import org.springframework.boot.autoconfigure.AutoConfigurationMetadata; -import org.springframework.context.EnvironmentAware; -import org.springframework.core.env.Environment; - -/** - * Keeps Guard Core out of the application unless {@code guard.enabled=true}. - * spring-boot-guard activates itself whenever it is on the classpath, so the opt-in - * is enforced here, before its auto-configuration is imported: when the property is - * absent or false no guard bean, filter or property binding exists. - */ -public class GuardAutoConfigurationGate implements AutoConfigurationImportFilter, EnvironmentAware { - - static final String GUARD_AUTO_CONFIGURATION = "com.guardcore.spring.GuardSecurityAutoConfiguration"; - - private Environment environment; - - @Override - public void setEnvironment(Environment environment) { - this.environment = environment; - } - - @Override - public boolean[] match(String[] autoConfigurationClasses, AutoConfigurationMetadata autoConfigurationMetadata) { - boolean enabled = environment.getProperty("guard.enabled", Boolean.class, false); - boolean[] outcome = new boolean[autoConfigurationClasses.length]; - for (int i = 0; i < outcome.length; i++) { - outcome[i] = enabled || !GUARD_AUTO_CONFIGURATION.equals(autoConfigurationClasses[i]); - } - return outcome; - } -} diff --git a/server/src/main/resources/META-INF/spring.factories b/server/src/main/resources/META-INF/spring.factories deleted file mode 100644 index 5608a2e0b..000000000 --- a/server/src/main/resources/META-INF/spring.factories +++ /dev/null @@ -1 +0,0 @@ -org.springframework.boot.autoconfigure.AutoConfigurationImportFilter=cn.keking.guard.GuardAutoConfigurationGate diff --git a/server/src/test/java/cn/keking/ServerMainTestAccess.java b/server/src/test/java/cn/keking/ServerMainTestAccess.java new file mode 100644 index 000000000..da1a38dfc --- /dev/null +++ b/server/src/test/java/cn/keking/ServerMainTestAccess.java @@ -0,0 +1,14 @@ +package cn.keking; + +import org.springframework.boot.builder.SpringApplicationBuilder; + +/** Lets tests outside the cn.keking package build the application exactly as main() does. */ +public final class ServerMainTestAccess { + + private ServerMainTestAccess() { + } + + public static SpringApplicationBuilder applicationBuilder() { + return ServerMain.applicationBuilder(); + } +} diff --git a/server/src/test/java/cn/keking/config/ActuatorOverrideExposureSecurityTests.java b/server/src/test/java/cn/keking/config/ActuatorOverrideExposureSecurityTests.java index 75155ab40..515249b5f 100644 --- a/server/src/test/java/cn/keking/config/ActuatorOverrideExposureSecurityTests.java +++ b/server/src/test/java/cn/keking/config/ActuatorOverrideExposureSecurityTests.java @@ -3,7 +3,8 @@ import org.springframework.boot.test.context.SpringBootTest; @SpringBootTest(classes = ActuatorExposureSecurityTests.ManagementApp.class, - properties = "spring.config.name=test", + // guard.enabled: without application.properties spring-boot-guard turns itself on, and this test is about actuator only + properties = {"spring.config.name=test", "guard.enabled=false"}, webEnvironment = SpringBootTest.WebEnvironment.RANDOM_PORT) class ActuatorOverrideExposureSecurityTests extends ActuatorExposureSecurityTests { } diff --git a/server/src/test/java/cn/keking/guard/GuardAutoConfigurationGateTests.java b/server/src/test/java/cn/keking/guard/GuardAutoConfigurationGateTests.java deleted file mode 100644 index ee3f099f7..000000000 --- a/server/src/test/java/cn/keking/guard/GuardAutoConfigurationGateTests.java +++ /dev/null @@ -1,35 +0,0 @@ -package cn.keking.guard; - -import org.junit.jupiter.api.Test; -import org.springframework.mock.env.MockEnvironment; - -import static org.junit.jupiter.api.Assertions.assertArrayEquals; - -class GuardAutoConfigurationGateTests { - - private static final String[] CLASSES = { - GuardAutoConfigurationGate.GUARD_AUTO_CONFIGURATION, - "org.springframework.boot.autoconfigure.web.servlet.WebMvcAutoConfiguration" - }; - - private static boolean[] match(MockEnvironment environment) { - GuardAutoConfigurationGate gate = new GuardAutoConfigurationGate(); - gate.setEnvironment(environment); - return gate.match(CLASSES, null); - } - - @Test - void keepsGuardOutWhenThePropertyIsMissing() { - assertArrayEquals(new boolean[]{false, true}, match(new MockEnvironment())); - } - - @Test - void keepsGuardOutWhenThePropertyIsFalse() { - assertArrayEquals(new boolean[]{false, true}, match(new MockEnvironment().withProperty("guard.enabled", "false"))); - } - - @Test - void importsGuardOnlyWhenThePropertyIsTrue() { - assertArrayEquals(new boolean[]{true, true}, match(new MockEnvironment().withProperty("guard.enabled", "true"))); - } -} diff --git a/server/src/test/java/cn/keking/guard/GuardDisabledTests.java b/server/src/test/java/cn/keking/guard/GuardDisabledTests.java index 66bea0fc7..94bdfab8f 100644 --- a/server/src/test/java/cn/keking/guard/GuardDisabledTests.java +++ b/server/src/test/java/cn/keking/guard/GuardDisabledTests.java @@ -2,10 +2,13 @@ import com.guardcore.spring.GuardFilter; import com.guardcore.spring.GuardMiddleware; +import com.guardcore.spring.GuardProperties; +import com.guardcore.spring.GuardSecurityAutoConfiguration; import org.junit.jupiter.api.Test; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.boot.web.servlet.FilterRegistrationBean; import org.springframework.context.ApplicationContext; +import org.springframework.core.env.Environment; import java.net.http.HttpResponse; @@ -20,6 +23,17 @@ class GuardDisabledTests extends GuardHttpTestSupport { @Autowired ApplicationContext context; + @Autowired + Environment environment; + + @Test + void shippedConfigurationSwitchesTheLibraryOff() { + // spring-boot-guard defaults guard.enabled to true, so application.properties is what keeps the guard off. + assertEquals("false", environment.getProperty("guard.enabled")); + assertEquals(0, context.getBeanNamesForType(GuardSecurityAutoConfiguration.class).length); + assertEquals(0, context.getBeanNamesForType(GuardProperties.class).length); + } + @Test void registersNoGuardComponent() { assertEquals(0, context.getBeanNamesForType(GuardMiddleware.class).length); diff --git a/server/src/test/java/cn/keking/guard/GuardEnabledTests.java b/server/src/test/java/cn/keking/guard/GuardEnabledTests.java index 492e63fc4..7bbe093d1 100644 --- a/server/src/test/java/cn/keking/guard/GuardEnabledTests.java +++ b/server/src/test/java/cn/keking/guard/GuardEnabledTests.java @@ -3,14 +3,18 @@ import cn.keking.config.ConfigConstants; import com.sun.net.httpserver.HttpServer; import jakarta.servlet.Filter; +import com.guardcore.spring.GuardMiddleware; +import com.guardcore.spring.GuardProperties; import jakarta.servlet.http.HttpServletRequest; import org.junit.jupiter.api.AfterAll; import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.io.TempDir; +import org.springframework.beans.factory.annotation.Autowired; import org.springframework.boot.test.context.TestConfiguration; import org.springframework.boot.web.servlet.FilterRegistrationBean; +import org.springframework.context.ApplicationContext; import org.springframework.context.annotation.Bean; import org.springframework.core.Ordered; import org.springframework.test.context.TestPropertySource; @@ -54,6 +58,9 @@ class GuardEnabledTests extends GuardHttpTestSupport { private static final int CHUNK_BYTES = 1024 * 1024; private static final Map RESPONSE_TYPES = new ConcurrentHashMap<>(); + @Autowired + ApplicationContext context; + @TempDir static Path fileDir; private static String previousFileDir; @@ -257,6 +264,13 @@ void ordinaryPreviewRequestsAreNotFlagged() throws Exception { } } + @Test + void enablingThePropertyRegistersTheGuard() { + assertEquals(1, context.getBeanNamesForType(GuardMiddleware.class).length); + assertEquals(1, context.getBeanNamesForType(GuardProperties.class).length); + assertEquals(1, context.getBeanNamesForType(GuardAdminAccessValidator.class).length); + } + @Test void uploadWithTheWrongContentTypeIsRejected() throws Exception { assertEquals(415, postForm("/fileUpload", newClientIp(), "a=b").statusCode()); diff --git a/server/src/test/java/cn/keking/guard/GuardOffWithoutConfigFileTests.java b/server/src/test/java/cn/keking/guard/GuardOffWithoutConfigFileTests.java new file mode 100644 index 000000000..43d02a06d --- /dev/null +++ b/server/src/test/java/cn/keking/guard/GuardOffWithoutConfigFileTests.java @@ -0,0 +1,42 @@ +package cn.keking.guard; + +import cn.keking.ServerMainTestAccess; +import com.guardcore.spring.GuardMiddleware; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; +import org.springframework.context.ConfigurableApplicationContext; + +import java.nio.file.Files; +import java.nio.file.Path; + +import static org.junit.jupiter.api.Assertions.assertEquals; + +/** + * Operators start kkFileView with -Dspring.config.location pointing at their own copy of application.properties, + * which replaces the shipped one. A copy from before the guard existed has no guard.enabled, and spring-boot-guard + * turns itself on when the property is missing, so the opt-in default has to come from the application itself. + */ +class GuardOffWithoutConfigFileTests { + + @TempDir + Path directory; + + @Test + void guardStaysOffWhenTheOperatorsConfigFileDoesNotMentionIt() throws Exception { + Path config = Files.writeString(directory.resolve("application.properties"), "server.port=0\n"); + try (ConfigurableApplicationContext context = ServerMainTestAccess.applicationBuilder() + .run("--spring.config.location=" + config, "--spring.main.lazy-initialization=true")) { + assertEquals("false", context.getEnvironment().getProperty("guard.enabled")); + assertEquals(0, context.getBeanNamesForType(GuardMiddleware.class).length); + } + } + + @Test + void guardCanStillBeSwitchedOnFromTheConfigFile() throws Exception { + Path config = Files.writeString(directory.resolve("application.properties"), "server.port=0\nguard.enabled=true\nguard.enable-redis=false\n"); + try (ConfigurableApplicationContext context = ServerMainTestAccess.applicationBuilder() + .run("--spring.config.location=" + config, "--spring.main.lazy-initialization=true")) { + assertEquals(1, context.getBeanNamesForType(GuardMiddleware.class).length); + } + } +} From bd68f544f74bbea218364022e93b5f519de8e0fc Mon Sep 17 00:00:00 2001 From: Renn F Date: Sat, 17 Oct 2026 12:52:11 +0200 Subject: [PATCH 5/5] test: cover real uploads and document why /fileUpload skips body scanning spring-boot-guard 1.3.0 keeps the multipart parts, but it scans file part payloads along with form fields, and kkFileView stores and previews html, sql and shell sources. Keep scanBody=false on /fileUpload, say why, and test that an upload is stored intact and a script-bearing html file is not refused. --- SECURITY_CONFIG.md | 2 +- .../keking/web/controller/FileController.java | 2 +- .../cn/keking/guard/GuardUploadTests.java | 90 +++++++++++++++++++ 3 files changed, 92 insertions(+), 2 deletions(-) create mode 100644 server/src/test/java/cn/keking/guard/GuardUploadTests.java diff --git a/SECURITY_CONFIG.md b/SECURITY_CONFIG.md index eb33f2839..3c1caec3c 100644 --- a/SECURITY_CONFIG.md +++ b/SECURITY_CONFIG.md @@ -187,7 +187,7 @@ guard.enabled = true - 默认不启用 `sensitive_file` 和 `recon` 类别：它们会把 `.java`、`.py`、`.sql`、`.sh`、`.jsp`、`.asp` 等源码文件的访问路径当作探测，而 kkFileView 会预览这类文件。可以用 `KK_GUARD_DETECTION_CATEGORIES` 覆盖。 - 使用 `local.preview.dir`（`file://` 预览）时，需要把 `url`、`urlPath`、`urls`、`currentUrl` 加入 `guard.excluded-detection-params`，否则 `file://` 会被当作文件包含攻击。 - 404 规则按接口统计，不是按路径统计：遍历大量不同静态路径的扫描器只会被请求限制限速，不会被封禁。 -- Guard 看不到上传文件部分的内容，`Content-Type` 检查的是请求而不是文件部分，文件类型仍由 `prohibit` 控制。声明的 Content-Length 达到 2GB 或以上时，引擎的大小检查会抛出异常并返回 500（请求仍然被拒绝）。 +- `/fileUpload` 的 multipart 请求体不做检测（`scanBody = false`）：spring-boot-guard 1.3.0 会检测表单字段和文件部分的内容，而 kkFileView 会保存并预览 html、sql、sh 等源码，这类文件会被当作攻击拒绝。路径、查询参数和请求头仍然检测，`path` 表单字段由 `FileController` 校验。Guard 看不到上传文件部分的内容，`Content-Type` 检查的是请求而不是文件部分，文件类型仍由 `prohibit` 控制。声明的 Content-Length 达到 2GB 或以上时，引擎的大小检查会抛出异常并返回 500（请求仍然被拒绝）。 - 引擎不会封禁回环地址（127.0.0.1）：来自回环地址的探测会被拒绝，但不会被封禁。 - 计数和封禁默认保存在进程内存中：多实例之间不共享，重启后清空。需要共享时设置 `KK_GUARD_ENABLE_REDIS=true` 和 `KK_GUARD_REDIS_URL`（独立于 `cache.type=redis`）。 - 全局限制默认 600 次/分钟：预览页面会加载大量静态资源，限制按客户端地址统计，办公室出口共用一个地址时请调大 `KK_GUARD_RATE_LIMIT`。 diff --git a/server/src/main/java/cn/keking/web/controller/FileController.java b/server/src/main/java/cn/keking/web/controller/FileController.java index f9bbca7e6..a1057bee6 100644 --- a/server/src/main/java/cn/keking/web/controller/FileController.java +++ b/server/src/main/java/cn/keking/web/controller/FileController.java @@ -162,7 +162,7 @@ public String getStats() { // Same ceiling as spring.servlet.multipart.max-request-size, checked on Content-Length before the body is parsed. @GuardContentTypeFilter(allowedTypes = "multipart/form-data") @GuardMaxRequestSize(sizeBytes = 524288000) - // Path, query and headers are still scanned. The multipart body is not: reading it for the scan leaves the upload without its file part. + // Path, query and headers are still scanned. The multipart body is not: spring-boot-guard 1.3.0 scans file part payloads along with form fields, and kkFileView stores and previews html, sql and shell sources that look like attacks. @GuardDetectionExclusion(scanBody = false) @GuardCustomValidation(validatorBean = GuardAdminAccessValidator.BEAN_NAME) @PostMapping("/fileUpload") diff --git a/server/src/test/java/cn/keking/guard/GuardUploadTests.java b/server/src/test/java/cn/keking/guard/GuardUploadTests.java new file mode 100644 index 000000000..3b64cd52a --- /dev/null +++ b/server/src/test/java/cn/keking/guard/GuardUploadTests.java @@ -0,0 +1,90 @@ +package cn.keking.guard; + +import cn.keking.config.ConfigConstants; +import org.junit.jupiter.api.AfterAll; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.springframework.test.context.TestPropertySource; + +import java.io.ByteArrayOutputStream; +import java.net.http.HttpRequest; +import java.net.http.HttpResponse; +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.Path; + +import static org.junit.jupiter.api.Assertions.assertArrayEquals; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertTrue; + +/** Real uploads through the guard with the shipped detection settings: the file lands intact and previewable source files are not rejected. */ +@TestPropertySource(properties = {"guard.enabled=true", "guard.trusted-proxies=127.0.0.1", "file.dir=${java.io.tmpdir}/kkfileview-guard-upload-tests"}) +class GuardUploadTests extends GuardHttpTestSupport { + + private static final String BOUNDARY = "kkguardupload"; + + private static Boolean previousUploadDisable; + + @BeforeAll + static void rememberConfiguration() { + previousUploadDisable = ConfigConstants.getFileUploadDisable(); + } + + @AfterAll + static void restoreConfiguration() throws Exception { + ConfigConstants.setFileUploadDisableValue(previousUploadDisable); + deleteRecursively(uploadRoot()); + } + + @BeforeEach + void enableUploads() { + ConfigConstants.setFileUploadDisableValue(false); + } + + private static Path uploadRoot() { + return Path.of(System.getProperty("java.io.tmpdir"), "kkfileview-guard-upload-tests"); + } + + private static void deleteRecursively(Path root) throws Exception { + if (Files.exists(root)) { + try (var paths = Files.walk(root)) { + paths.sorted(java.util.Comparator.reverseOrder()).forEach(path -> path.toFile().delete()); + } + } + } + + private HttpResponse upload(String fileName, byte[] content, String clientIp) throws Exception { + ByteArrayOutputStream body = new ByteArrayOutputStream(); + body.write(("--" + BOUNDARY + "\r\nContent-Disposition: form-data; name=\"file\"; filename=\"" + fileName + "\"\r\n" + + "Content-Type: application/octet-stream\r\n\r\n").getBytes(StandardCharsets.UTF_8)); + body.write(content); + body.write(("\r\n--" + BOUNDARY + "--\r\n").getBytes(StandardCharsets.UTF_8)); + return send(request("/fileUpload").header("X-Forwarded-For", clientIp) + .header("Content-Type", "multipart/form-data; boundary=" + BOUNDARY) + .POST(HttpRequest.BodyPublishers.ofByteArray(body.toByteArray())).build()); + } + + @Test + void uploadedFileIsStoredIntact() throws Exception { + byte[] content = new byte[300_000]; + for (int i = 0; i < content.length; i++) { + content[i] = (byte) ('a' + i % 26); + } + HttpResponse response = upload("intact.py", content, newClientIp()); + assertEquals(200, response.statusCode()); + assertTrue(response.body().contains("\"code\":0"), response.body()); + assertArrayEquals(content, Files.readAllBytes(uploadRoot().resolve("demo").resolve("intact.py"))); + } + + @Test + void sourceFileWithScriptMarkupIsNotRejected() throws Exception { + // kkFileView previews html, jsp, sql and shell sources, so a file part that looks like an attack must not be refused as one. + byte[] content = "\n-- 1 UNION SELECT username, password FROM users\n" + .getBytes(StandardCharsets.UTF_8); + HttpResponse response = upload("page.html", content, newClientIp()); + assertEquals(200, response.statusCode()); + assertTrue(response.body().contains("\"code\":0"), response.body()); + assertArrayEquals(content, Files.readAllBytes(uploadRoot().resolve("demo").resolve("page.html"))); + } +}