diff --git a/internal/gateway/methods/bitrix_portals.go b/internal/gateway/methods/bitrix_portals.go index 28db90dc..8175dd41 100644 --- a/internal/gateway/methods/bitrix_portals.go +++ b/internal/gateway/methods/bitrix_portals.go @@ -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 diff --git a/internal/gateway/methods/bitrix_portals_test.go b/internal/gateway/methods/bitrix_portals_test.go index 2ed4568f..57d59cda 100644 --- a/internal/gateway/methods/bitrix_portals_test.go +++ b/internal/gateway/methods/bitrix_portals_test.go @@ -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) { diff --git a/ui/web/src/pages/channels/bitrix24/bitrix-portal-form-step.tsx b/ui/web/src/pages/channels/bitrix24/bitrix-portal-form-step.tsx index aa0a15ea..a0addf0c 100644 --- a/ui/web/src/pages/channels/bitrix24/bitrix-portal-form-step.tsx +++ b/ui/web/src/pages/channels/bitrix24/bitrix-portal-form-step.tsx @@ -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" });