mirror of
https://github.com/tiennm99/goclaw.git
synced 2026-10-14 20:17:20 +00:00
fix(bitrix): add SSRF-safe self-hosted domain check
- Add validateSelfHostedDomain() with SSRF protection (blocked CIDRs, hostname resolution, localhost/.local/.localhost rejection) - Validate port range 1-65535 for self-hosted domains with ports - Mirror SSRF + port validation in frontend form - Add comprehensive tests: SSRF blocked IPs, invalid ports, valid public domains
This commit is contained in:
1 parent
e4f1d55e0c
commit
56fce5d294
3 files changed
+228
-11
No files matched your search
@@ -4,8 +4,11 @@ import (
|
||||
"context"
|
||||
"encoding/json"
|
||||
"errors"
|
||||
"fmt"
|
||||
"log/slog"
|
||||
"net"
|
||||
"regexp"
|
||||
"strconv"
|
||||
"strings"
|
||||
|
||||
"github.com/google/uuid"
|
||||
@@ -13,6 +16,7 @@ import (
|
||||
"github.com/nextlevelbuilder/goclaw/internal/gateway"
|
||||
"github.com/nextlevelbuilder/goclaw/internal/i18n"
|
||||
"github.com/nextlevelbuilder/goclaw/internal/permissions"
|
||||
"github.com/nextlevelbuilder/goclaw/internal/security"
|
||||
"github.com/nextlevelbuilder/goclaw/internal/store"
|
||||
"github.com/nextlevelbuilder/goclaw/pkg/protocol"
|
||||
)
|
||||
@@ -150,6 +154,14 @@ func (m *BitrixPortalsMethods) handleCreate(ctx context.Context, client *gateway
|
||||
client.SendResponse(protocol.NewErrorResponse(req.ID, protocol.ErrInvalidRequest, i18n.T(locale, i18n.MsgInvalidRequest, "domain: must be a valid hostname (e.g. *.bitrix24.com, *.bitrix.info, or your self-hosted domain)")))
|
||||
return
|
||||
}
|
||||
// SSRF + port validation for self-hosted domains (cloud domains are
|
||||
// Bitrix-operated and implicitly trusted).
|
||||
if !bitrixCloudDomainRegex.MatchString(domain) {
|
||||
if err := validateSelfHostedDomain(domain); err != nil {
|
||||
client.SendResponse(protocol.NewErrorResponse(req.ID, protocol.ErrInvalidRequest, i18n.T(locale, i18n.MsgInvalidRequest, "domain: "+err.Error())))
|
||||
return
|
||||
}
|
||||
}
|
||||
if clientID == "" || clientSecret == "" {
|
||||
client.SendResponse(protocol.NewErrorResponse(req.ID, protocol.ErrInvalidRequest, i18n.T(locale, i18n.MsgRequired, "client_id and client_secret")))
|
||||
return
|
||||
@@ -356,6 +368,67 @@ func portalRowToView(row store.BitrixPortalData) bitrixPortalView {
|
||||
return v
|
||||
}
|
||||
|
||||
// validateSelfHostedDomain checks a self-hosted Bitrix24 domain for SSRF
|
||||
// risks and invalid port ranges. Cloud domains (*.bitrix24.*, *.bitrix.info)
|
||||
// are Bitrix-operated and implicitly trusted — this function is only called
|
||||
// for custom/self-hosted domains.
|
||||
//
|
||||
// Policy:
|
||||
// - Rejects literal private/loopback/metadata IPs (127.x, 10.x, 192.168.x,
|
||||
// 169.254.x, ::1, fc00::, etc.)
|
||||
// - Rejects hostnames that resolve to blocked IPs (defense against
|
||||
// internal service names like "postgres", "redis", "metadata")
|
||||
// - Rejects .localhost and .local TLDs (commonly used for local dev)
|
||||
// - Validates port range 1-65535 when a port is specified
|
||||
func validateSelfHostedDomain(domain string) error {
|
||||
// Extract host and optional port.
|
||||
host := domain
|
||||
portStr := ""
|
||||
if idx := strings.LastIndex(domain, ":"); idx != -1 {
|
||||
host = domain[:idx]
|
||||
portStr = domain[idx+1:]
|
||||
}
|
||||
|
||||
// Validate port range if present.
|
||||
if portStr != "" {
|
||||
port, err := strconv.Atoi(portStr)
|
||||
if err != nil || port < 1 || port > 65535 {
|
||||
return fmt.Errorf("port must be 1-65535")
|
||||
}
|
||||
}
|
||||
|
||||
// Reject .localhost and .local TLDs (commonly used for local development).
|
||||
lowerHost := strings.ToLower(host)
|
||||
if strings.HasSuffix(lowerHost, ".localhost") || strings.HasSuffix(lowerHost, ".local") || lowerHost == "localhost" {
|
||||
return fmt.Errorf("private/internal hostnames (localhost, .local, .localhost) are not allowed")
|
||||
}
|
||||
|
||||
// If the host is a literal IP, check it against blocked CIDRs.
|
||||
if ip := net.ParseIP(host); ip != nil {
|
||||
if security.IsBlocked(ip) {
|
||||
return fmt.Errorf("IP %s is in a blocked range (loopback/private/metadata)", ip)
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
// For hostnames, resolve and check the first returned IP.
|
||||
addrs, err := net.LookupHost(host)
|
||||
if err != nil {
|
||||
return fmt.Errorf("cannot resolve hostname %q", host)
|
||||
}
|
||||
if len(addrs) == 0 {
|
||||
return fmt.Errorf("hostname %q resolved to no addresses", host)
|
||||
}
|
||||
ip := net.ParseIP(addrs[0])
|
||||
if ip == nil {
|
||||
return fmt.Errorf("resolved address %q is not a valid IP", addrs[0])
|
||||
}
|
||||
if security.IsBlocked(ip) {
|
||||
return fmt.Errorf("hostname %q resolved to blocked IP %s (loopback/private/metadata)", host, ip)
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
// isDuplicateKeyErr probes a store error for a UNIQUE violation. Kept as a
|
||||
// string substring match because the store interface doesn't expose typed
|
||||
// duplicate errors and we want consistent behaviour between pg + sqlite
|
||||
|
||||
@@ -366,17 +366,36 @@ func TestBitrixPortals_Create_HappyPath_ReturnsInstallURL(t *testing.T) {
|
||||
func TestBitrixPortals_Create_InvalidDomain(t *testing.T) {
|
||||
tid := uuid.New()
|
||||
m := NewBitrixPortalsMethods(newStubBitrixPortalStore(), newStubChannelInstanceStore(), gatewayURLFn("https://gw.example.com"))
|
||||
client, ch := gateway.NewCapturingTestClient(permissions.RoleAdmin, tid, "u", 4)
|
||||
m.handleCreate(store.WithTenantID(context.Background(), tid), client, buildBitrixReq(t, protocol.MethodBitrixPortalsCreate, map[string]string{
|
||||
"name": "p",
|
||||
"domain": "-invalid.com",
|
||||
"client_id": "x",
|
||||
"client_secret": "y",
|
||||
}))
|
||||
|
||||
resp := readResponse(t, ch)
|
||||
if resp.Error == nil || resp.Error.Code != protocol.ErrInvalidRequest {
|
||||
t.Errorf("expected INVALID_REQUEST, got %+v", resp.Error)
|
||||
cases := []struct {
|
||||
name string
|
||||
domain string
|
||||
}{
|
||||
{"leading hyphen", "-invalid.com"},
|
||||
{"scheme included", "https://example.com"},
|
||||
{"path included", "example.com/path"},
|
||||
{"space in domain", "exam ple.com"},
|
||||
{"invalid port zero", "example.com:0"},
|
||||
{"invalid port overflow", "example.com:99999"},
|
||||
{"localhost", "localhost"},
|
||||
{"localhost subdomain", "bx.localhost"},
|
||||
{"local TLD", "bx.local"},
|
||||
}
|
||||
|
||||
for _, tc := range cases {
|
||||
t.Run(tc.name, func(t *testing.T) {
|
||||
client, ch := gateway.NewCapturingTestClient(permissions.RoleAdmin, tid, "u", 4)
|
||||
m.handleCreate(store.WithTenantID(context.Background(), tid), client, buildBitrixReq(t, protocol.MethodBitrixPortalsCreate, map[string]string{
|
||||
"name": "p",
|
||||
"domain": tc.domain,
|
||||
"client_id": "x",
|
||||
"client_secret": "y",
|
||||
}))
|
||||
resp := readResponse(t, ch)
|
||||
if resp.Error == nil || resp.Error.Code != protocol.ErrInvalidRequest {
|
||||
t.Errorf("expected INVALID_REQUEST for %q, got %+v", tc.domain, resp.Error)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
@@ -630,6 +649,64 @@ func TestPortalNameRegex(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// SSRF + port validation for self-hosted domains
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
func TestValidateSelfHostedDomain_SSRF(t *testing.T) {
|
||||
// These should all be rejected as SSRF risks.
|
||||
blocked := []string{
|
||||
"127.0.0.1",
|
||||
"127.0.0.1:8080",
|
||||
"::1",
|
||||
"10.0.0.1",
|
||||
"10.0.0.1:443",
|
||||
"192.168.1.1",
|
||||
"172.16.0.1",
|
||||
"169.254.169.254", // cloud metadata
|
||||
"0.0.0.0",
|
||||
"localhost",
|
||||
"bx.localhost",
|
||||
"bx.local",
|
||||
"portal.internal.localhost",
|
||||
}
|
||||
for _, d := range blocked {
|
||||
if err := validateSelfHostedDomain(d); err == nil {
|
||||
t.Errorf("should reject %q (SSRF risk)", d)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
func TestValidateSelfHostedDomain_PortRange(t *testing.T) {
|
||||
badPorts := []string{
|
||||
"example.com:0",
|
||||
"example.com:99999",
|
||||
"example.com:-1",
|
||||
"example.com:abc",
|
||||
}
|
||||
for _, d := range badPorts {
|
||||
if err := validateSelfHostedDomain(d); err == nil {
|
||||
t.Errorf("should reject %q (invalid port)", d)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
func TestValidateSelfHostedDomain_ValidPublicDomain(t *testing.T) {
|
||||
// Public domains that resolve to public IPs should pass.
|
||||
// (Note: this does a real DNS lookup — if the test environment has no
|
||||
// internet, this will fail. That's acceptable; the SSRF logic is still
|
||||
// tested via the blocked cases above which use literal IPs.)
|
||||
valid := []string{
|
||||
"google.com",
|
||||
"example.com:443",
|
||||
}
|
||||
for _, d := range valid {
|
||||
if err := validateSelfHostedDomain(d); err != nil {
|
||||
t.Errorf("should accept %q, got error: %v", d, err)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// TestIsDuplicateKeyErr covers the two backend error string shapes we map
|
||||
// to ALREADY_EXISTS.
|
||||
func TestIsDuplicateKeyErr(t *testing.T) {
|
||||
|
||||
@@ -20,6 +20,62 @@ const SELF_HOSTED_DOMAIN_RE =
|
||||
/^[a-z0-9]([a-z0-9-]{0,61}[a-z0-9])?(\.[a-z0-9]([a-z0-9-]{0,61}[a-z0-9])?)*(:\d+)?$/;
|
||||
const PORTAL_NAME_RE = /^[a-z0-9][a-z0-9_-]{0,62}[a-z0-9]$/;
|
||||
|
||||
// validateSelfHostedDomain mirrors the backend SSRF + port validation.
|
||||
// Rejects localhost, .local, .localhost TLDs, literal private/loopback IPs,
|
||||
// and invalid port ranges (0, >65535).
|
||||
function validateSelfHostedDomain(domain: string): string | null {
|
||||
// Extract host and optional port.
|
||||
let host = domain;
|
||||
let portStr: string | undefined;
|
||||
const colonIdx = domain.lastIndexOf(":");
|
||||
if (colonIdx !== -1) {
|
||||
host = domain.slice(0, colonIdx);
|
||||
portStr = domain.slice(colonIdx + 1);
|
||||
}
|
||||
|
||||
// Validate port range.
|
||||
if (portStr !== undefined) {
|
||||
const port = Number(portStr);
|
||||
if (!Number.isInteger(port) || port < 1 || port > 65535) {
|
||||
return "port must be 1-65535";
|
||||
}
|
||||
}
|
||||
|
||||
// Reject localhost and .local/.localhost TLDs.
|
||||
const lowerHost = host.toLowerCase();
|
||||
if (
|
||||
lowerHost === "localhost" ||
|
||||
lowerHost.endsWith(".localhost") ||
|
||||
lowerHost.endsWith(".local")
|
||||
) {
|
||||
return "private/internal hostnames (localhost, .local, .localhost) are not allowed";
|
||||
}
|
||||
|
||||
// Reject literal private/loopback IPs.
|
||||
// Simple check for common patterns — backend does full CIDR validation.
|
||||
if (
|
||||
lowerHost === "127.0.0.1" ||
|
||||
lowerHost.startsWith("127.") ||
|
||||
lowerHost === "10.0.0.0" ||
|
||||
lowerHost.startsWith("10.") ||
|
||||
lowerHost.startsWith("192.168.") ||
|
||||
lowerHost.startsWith("172.16.") ||
|
||||
lowerHost.startsWith("172.17.") ||
|
||||
lowerHost.startsWith("172.18.") ||
|
||||
lowerHost.startsWith("172.19.") ||
|
||||
lowerHost.startsWith("172.2") ||
|
||||
lowerHost.startsWith("172.3") ||
|
||||
lowerHost === "169.254.169.254" ||
|
||||
lowerHost.startsWith("169.254.") ||
|
||||
lowerHost === "::1" ||
|
||||
lowerHost === "0.0.0.0"
|
||||
) {
|
||||
return "IP is in a blocked range (loopback/private/metadata)";
|
||||
}
|
||||
|
||||
return null;
|
||||
}
|
||||
|
||||
interface BitrixPortalFormStepProps {
|
||||
/** Invoked with the server response after bitrix.portals.create succeeds. */
|
||||
onSuccess: (createdName: string, installUrl: string, warning?: string) => void;
|
||||
@@ -56,10 +112,21 @@ export function BitrixPortalFormStep({ onSuccess, onCancel }: BitrixPortalFormSt
|
||||
defaultValue: "Use lowercase letters, digits, hyphens, underscores (2-64 chars).",
|
||||
});
|
||||
}
|
||||
if (!BITRIX_DOMAIN_RE.test(domain.toLowerCase()) && !SELF_HOSTED_DOMAIN_RE.test(domain.toLowerCase())) {
|
||||
const domainLower = domain.toLowerCase();
|
||||
const isCloud = BITRIX_DOMAIN_RE.test(domainLower);
|
||||
const isSelfHostedSyntax = SELF_HOSTED_DOMAIN_RE.test(domainLower);
|
||||
if (!isCloud && !isSelfHostedSyntax) {
|
||||
e.domain = t("bitrix24.create.errors.invalidDomain", {
|
||||
defaultValue: "Must be a valid hostname (e.g. *.bitrix24.com, *.bitrix.info, or your self-hosted domain).",
|
||||
});
|
||||
} else if (!isCloud) {
|
||||
// SSRF + port validation for self-hosted domains.
|
||||
const ssrfErr = validateSelfHostedDomain(domainLower);
|
||||
if (ssrfErr) {
|
||||
e.domain = t("bitrix24.create.errors.invalidDomain", {
|
||||
defaultValue: ssrfErr,
|
||||
});
|
||||
}
|
||||
}
|
||||
if (!clientId.trim()) e.client_id = t("common.required", { defaultValue: "Required" });
|
||||
if (!clientSecret.trim()) e.client_secret = t("common.required", { defaultValue: "Required" });
|
||||
|
||||
Reference in new issue
Block a user