Skip to content

Commit c38471b

Browse files
notSumit25notSumit25
committed
fix(security): guard the remaining two SSRF sinks (JDBC host, presigned URL)
Closes the other two java/ssrf alerts. Address classification moves into a shared OutboundHostGuard -- resolve the host and check every returned address, so a public hostname whose A record points at 10.x or 169.254.169.254 is still refused. SshHostGuard now delegates to it instead of carrying its own copy. DatabaseHostGuard (#136, ConnectionService) screens the JDBC host. The SSH guard never covered this: a direct, non-tunnelled connection does not go through SshTunnelService at all. Applied in buildJdbcUrl and in the Hikari pool path, and skipped when a tunnel port is present since that targets the local forwarded port. Ships disabled, same reasoning as the SSH guard -- databases sit on RFC1918 more often than bastions do. S3LogFetchService (#137) was calling setInstanceFollowRedirects(true), so the JDK chased a 302 with no chance to inspect the target and a presigned URL on a public host could hand off to the metadata endpoint. Redirects are now followed manually with a cap of 5, and every hop is re-checked for https plus a public address. This one is always on: there is no legitimate reason to fetch a slow query log from a private address. Verified: 47 unit tests pass across the new and touched guards; the six pre-existing failing classes show the same 13 failures / 4 errors as the untouched baseline, so no new regressions.
1 parent 563daa1 commit c38471b

9 files changed

Lines changed: 364 additions & 93 deletions

File tree

‎CLAUDE.md‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -350,6 +350,24 @@ points are covered by that single call site (`establishTunnel` and
350350
- Operators who want the protection set `enabled=true` and allowlist their own bastion
351351
via `deepsql.ssh.host-guard.allowed-hosts` (exact host, or a leading-dot suffix like
352352
`.corp.internal`).
353+
- **Two sibling guards cover the other two `java/ssrf` alerts.** Address
354+
classification is shared in `OutboundHostGuard` (resolve the host, check every
355+
returned address, block loopback/link-local/RFC1918/CGNAT/ULA/IPv4-mapped-IPv6);
356+
the three call sites differ only in policy and message.
357+
- `DatabaseHostGuard` (alert #136, `ConnectionService`) screens the **JDBC** host.
358+
The SSH guard never covered this — a direct, non-tunnelled connection does not
359+
pass through `SshTunnelService` at all. Applied in `buildJdbcUrl` *and* the
360+
Hikari pool path, and skipped when `tunnelPort != null` since a tunnelled
361+
connection targets the local forwarded port. Also ships disabled
362+
(`deepsql.database.host-guard.enabled`) — databases sit on RFC1918 even more
363+
often than bastions do.
364+
- `S3LogFetchService.assertFetchableUrl` (alert #137) screens the presigned log
365+
URL. The real hazard was `setInstanceFollowRedirects(true)`: the JDK chases a
366+
302 with no chance to inspect the target, so a presigned URL on a public host
367+
could hand off to the metadata endpoint. Redirects are now followed manually
368+
(max 5), with **every hop** re-checked for https + a public address. Unlike the
369+
other two this is always on — there is no legitimate reason to fetch a slow
370+
query log from a private address.
353371

354372
### CodeQL Remediation (code scanning, 138 alerts on main)
355373

‎backend/src/main/java/com/dbaagent/service/ConnectionService.java‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@ public class ConnectionService {
4545
private final SshTunnelService sshTunnelService;
4646
private final CredentialService credentialService;
4747
private final DatabaseProviderRegistry providerRegistry;
48+
private final DatabaseHostGuard databaseHostGuard;
4849

4950
public boolean testConnection(ConnectionRequest request) {
5051
Connection connection = null;
@@ -263,6 +264,10 @@ private HikariDataSource createConnectionPool(String connectionId, ConnectionReq
263264
log.info("SSH tunnel established on local port {} for connection: {}", tunnelPort, connectionId);
264265
}
265266

267+
if (tunnelPort == null) {
268+
databaseHostGuard.assertAllowed(request.getHost());
269+
}
270+
266271
DatabaseDialect dialect = providerRegistry.getDialect(request.getDbType());
267272
ConnectionProvider connectionProvider = dialect.connection();
268273

@@ -316,6 +321,13 @@ private HikariDataSource createConnectionPool(String connectionId, ConnectionReq
316321
* Delegates to the appropriate database provider.
317322
*/
318323
private String buildJdbcUrl(ConnectionRequest request, Integer tunnelPort) {
324+
// Guard here rather than at each caller: this is the single point every
325+
// JDBC URL is built through (java/ssrf on ConnectionService). A tunnelled
326+
// connection targets the local forwarded port, so only the direct case
327+
// carries a user-supplied host worth screening.
328+
if (tunnelPort == null) {
329+
databaseHostGuard.assertAllowed(request.getHost());
330+
}
319331
DatabaseDialect dialect = providerRegistry.getDialect(request.getDbType());
320332
return dialect.connection().buildJdbcUrl(request, tunnelPort);
321333
}

‎backend/src/main/java/com/dbaagent/service/DatabaseHostGuard.java‎

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
package com.dbaagent.service;
2+
3+
import lombok.extern.slf4j.Slf4j;
4+
import org.springframework.stereotype.Component;
5+
6+
import java.net.InetAddress;
7+
8+
/**
9+
* Screens the database host a user supplies before a JDBC connection is opened.
10+
*
11+
* Same class of exposure as {@link SshHostGuard} but a different code path: the
12+
* SSH guard sees only request.getSshHost(), and a direct (non-tunnelled)
13+
* connection never passes through it.
14+
*/
15+
@Component
16+
@Slf4j
17+
public class DatabaseHostGuard {
18+
19+
private final DatabaseHostGuardProperties properties;
20+
21+
public DatabaseHostGuard(DatabaseHostGuardProperties properties) {
22+
this.properties = properties;
23+
}
24+
25+
public void assertAllowed(String databaseHost) {
26+
if (!properties.isEnabled() || databaseHost == null || databaseHost.isBlank()) {
27+
return;
28+
}
29+
30+
String host = OutboundHostGuard.normalize(databaseHost);
31+
if (OutboundHostGuard.isAllowlisted(host, properties.getAllowedHosts())) {
32+
return;
33+
}
34+
35+
InetAddress blocked;
36+
try {
37+
blocked = OutboundHostGuard.findBlockedAddress(host);
38+
} catch (OutboundHostGuard.BlockedHostException e) {
39+
throw new IllegalArgumentException("Database host could not be resolved: " + databaseHost);
40+
}
41+
42+
if (blocked != null) {
43+
log.warn("Blocked database connection to restricted host {} (resolved to {})",
44+
databaseHost, blocked.getHostAddress());
45+
throw new IllegalArgumentException(
46+
"Database host '" + databaseHost + "' resolves to a restricted address ("
47+
+ blocked.getHostAddress() + "). Add it to "
48+
+ "deepsql.database.host-guard.allowed-hosts if this is intentional.");
49+
}
50+
}
51+
}

‎backend/src/main/java/com/dbaagent/service/DatabaseHostGuardProperties.java‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
package com.dbaagent.service;
2+
3+
import lombok.Data;
4+
import org.springframework.boot.context.properties.ConfigurationProperties;
5+
import org.springframework.stereotype.Component;
6+
7+
import java.util.ArrayList;
8+
import java.util.List;
9+
10+
/**
11+
* Binds the database-host guard (CodeQL java/ssrf on ConnectionService).
12+
*
13+
* Off by default, same reasoning as {@link SshHostGuardProperties}: databases
14+
* legitimately live on RFC1918 networks — more often than bastions do — so
15+
* enabling this would break most existing installs on upgrade.
16+
*/
17+
@Component
18+
@ConfigurationProperties(prefix = "deepsql.database.host-guard")
19+
@Data
20+
public class DatabaseHostGuardProperties {
21+
22+
private boolean enabled = false;
23+
24+
private List<String> allowedHosts = new ArrayList<>();
25+
}

‎backend/src/main/java/com/dbaagent/service/OutboundHostGuard.java‎

Lines changed: 110 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,110 @@
1+
package com.dbaagent.service;
2+
3+
import java.net.Inet4Address;
4+
import java.net.Inet6Address;
5+
import java.net.InetAddress;
6+
import java.net.UnknownHostException;
7+
import java.util.List;
8+
import java.util.Locale;
9+
10+
/**
11+
* Shared address classification for every outbound host the user can name:
12+
* SSH bastions, database hosts, and presigned log URLs.
13+
*
14+
* The host is resolved and every returned address is checked, so a public
15+
* hostname whose A record points at 10.x or 169.254.169.254 is still refused —
16+
* a check against the literal string is defeated by one DNS record.
17+
*/
18+
public final class OutboundHostGuard {
19+
20+
private OutboundHostGuard() {}
21+
22+
/** Thrown when a host resolves to an address outbound traffic must not reach. */
23+
public static class BlockedHostException extends RuntimeException {
24+
public BlockedHostException(String message) {
25+
super(message);
26+
}
27+
}
28+
29+
public static String normalize(String host) {
30+
String trimmed = host.trim().toLowerCase(Locale.ROOT);
31+
if (trimmed.startsWith("[") && trimmed.endsWith("]")) {
32+
trimmed = trimmed.substring(1, trimmed.length() - 1);
33+
}
34+
return trimmed;
35+
}
36+
37+
public static boolean isAllowlisted(String host, List<String> allowedHosts) {
38+
for (String allowed : allowedHosts) {
39+
if (allowed == null || allowed.isBlank()) continue;
40+
String candidate = normalize(allowed);
41+
if (candidate.startsWith(".")) {
42+
if (host.endsWith(candidate)) return true;
43+
} else if (host.equals(candidate)) {
44+
return true;
45+
}
46+
}
47+
return false;
48+
}
49+
50+
/**
51+
* @return the blocked address, or null when every resolved address is allowed.
52+
*/
53+
public static InetAddress findBlockedAddress(String host) {
54+
InetAddress[] addresses;
55+
try {
56+
addresses = InetAddress.getAllByName(host);
57+
} catch (UnknownHostException e) {
58+
throw new BlockedHostException("Host could not be resolved: " + host);
59+
}
60+
for (InetAddress address : addresses) {
61+
if (isBlocked(address)) {
62+
return address;
63+
}
64+
}
65+
return null;
66+
}
67+
68+
public static boolean isBlocked(InetAddress address) {
69+
if (address.isLoopbackAddress()
70+
|| address.isAnyLocalAddress()
71+
|| address.isLinkLocalAddress()
72+
|| address.isSiteLocalAddress()
73+
|| address.isMulticastAddress()) {
74+
return true;
75+
}
76+
if (address instanceof Inet4Address) {
77+
return isBlockedIpv4(address.getAddress());
78+
}
79+
if (address instanceof Inet6Address v6) {
80+
// Unique local addresses (fc00::/7) have no isSiteLocalAddress() mapping in Java.
81+
if ((v6.getAddress()[0] & 0xFE) == 0xFC) return true;
82+
byte[] embedded = embeddedIpv4(v6);
83+
return embedded != null && isBlockedIpv4(embedded);
84+
}
85+
return false;
86+
}
87+
88+
private static boolean isBlockedIpv4(byte[] octets) {
89+
int first = octets[0] & 0xFF;
90+
int second = octets[1] & 0xFF;
91+
// 100.64.0.0/10 carrier-grade NAT, used for cloud-internal routing.
92+
if (first == 100 && second >= 64 && second <= 127) return true;
93+
// 192.0.0.0/24 IETF protocol assignments.
94+
if (first == 192 && second == 0 && (octets[2] & 0xFF) == 0) return true;
95+
// 0.0.0.0/8 "this network".
96+
return first == 0;
97+
}
98+
99+
/** IPv4-mapped/compatible forms smuggle a blocked v4 address through a v6 literal. */
100+
private static byte[] embeddedIpv4(Inet6Address address) {
101+
byte[] bytes = address.getAddress();
102+
for (int i = 0; i < 10; i++) {
103+
if (bytes[i] != 0) return null;
104+
}
105+
boolean mapped = (bytes[10] & 0xFF) == 0xFF && (bytes[11] & 0xFF) == 0xFF;
106+
boolean compat = bytes[10] == 0 && bytes[11] == 0;
107+
if (!mapped && !compat) return null;
108+
return new byte[]{bytes[12], bytes[13], bytes[14], bytes[15]};
109+
}
110+
}

‎backend/src/main/java/com/dbaagent/service/S3LogFetchService.java‎

Lines changed: 61 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -184,6 +184,33 @@ private AwsCredentialsProvider resolveCredentialsProvider(AwsCredentialsInput cr
184184
);
185185
}
186186

187+
private static final int MAX_PRESIGNED_REDIRECTS = 5;
188+
189+
/**
190+
* Every hop of a presigned fetch must be https to a public address. The
191+
* initial URL and each redirect target both go through here.
192+
*/
193+
private URI assertFetchableUrl(URI uri) {
194+
String scheme = uri.getScheme();
195+
if (scheme == null || !scheme.equalsIgnoreCase("https")) {
196+
throw new IllegalArgumentException(
197+
"Presigned log URL must use https, got: " + scheme);
198+
}
199+
String host = uri.getHost();
200+
if (host == null || host.isBlank()) {
201+
throw new IllegalArgumentException("Presigned log URL has no host");
202+
}
203+
java.net.InetAddress blocked =
204+
OutboundHostGuard.findBlockedAddress(OutboundHostGuard.normalize(host));
205+
if (blocked != null) {
206+
log.warn("Blocked presigned log fetch to restricted host {} (resolved to {})",
207+
host, blocked.getHostAddress());
208+
throw new IllegalArgumentException(
209+
"Presigned log URL resolves to a restricted address (" + blocked.getHostAddress() + ")");
210+
}
211+
return uri;
212+
}
213+
187214
boolean isPresignedUrl(String s3Url) {
188215
if (s3Url == null || s3Url.isBlank()) {
189216
return false;
@@ -197,26 +224,52 @@ boolean isPresignedUrl(String s3Url) {
197224

198225
private InputStream downloadPresignedLog(String presignedUrl) {
199226
try {
200-
HttpURLConnection connection = (HttpURLConnection) URI.create(presignedUrl).toURL().openConnection();
201-
connection.setRequestMethod("GET");
202-
connection.setConnectTimeout(15_000);
203-
connection.setReadTimeout(60_000);
204-
connection.setInstanceFollowRedirects(true);
227+
// Redirects are followed manually: with setInstanceFollowRedirects(true)
228+
// the JDK chases a 302 without giving us a chance to screen the target,
229+
// so a presigned URL on a public host could hand off to 169.254.169.254
230+
// or an internal address (java/ssrf).
231+
URI current = assertFetchableUrl(URI.create(presignedUrl));
232+
HttpURLConnection connection = null;
233+
int status;
234+
for (int redirects = 0; ; redirects++) {
235+
if (redirects > MAX_PRESIGNED_REDIRECTS) {
236+
throw new RuntimeException("Too many redirects fetching presigned URL");
237+
}
238+
connection = (HttpURLConnection) current.toURL().openConnection();
239+
connection.setRequestMethod("GET");
240+
connection.setConnectTimeout(15_000);
241+
connection.setReadTimeout(60_000);
242+
connection.setInstanceFollowRedirects(false);
243+
244+
status = connection.getResponseCode();
245+
if (status != HttpURLConnection.HTTP_MOVED_PERM
246+
&& status != HttpURLConnection.HTTP_MOVED_TEMP
247+
&& status != HttpURLConnection.HTTP_SEE_OTHER
248+
&& status != 307 && status != 308) {
249+
break;
250+
}
251+
String location = connection.getHeaderField("Location");
252+
connection.disconnect();
253+
if (location == null || location.isBlank()) {
254+
throw new RuntimeException("Redirect without a Location header");
255+
}
256+
current = assertFetchableUrl(current.resolve(location));
257+
}
205258

206-
int status = connection.getResponseCode();
207259
if (status < 200 || status >= 300) {
208260
throw new RuntimeException("Failed to download presigned URL, status " + status);
209261
}
210262

211263
log.info("Streaming slow query log from presigned URL");
212-
InputStream inputStream = connection.getInputStream();
264+
final HttpURLConnection finalConnection = connection;
265+
InputStream inputStream = finalConnection.getInputStream();
213266
return new java.io.FilterInputStream(inputStream) {
214267
@Override
215268
public void close() throws java.io.IOException {
216269
try {
217270
super.close();
218271
} finally {
219-
connection.disconnect();
272+
finalConnection.disconnect();
220273
}
221274
}
222275
};

0 commit comments

Comments
 (0)