diff --git a/internal/gateway/methods/bitrix_portals.go b/internal/gateway/methods/bitrix_portals.go index 4157f8d7..3adf6a54 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" ) @@ -69,9 +73,13 @@ type bitrixPortalView struct { // Validation regexes. Kept package-level so they compile once and tests can // reference them directly. var ( - // Bitrix24 cloud portal hosts. Matches *.bitrix24.{com,eu,ru,de,fr,jp,in,kz,ua,by} + // Bitrix24 cloud portal hosts. Matches *.bitrix24.{com,eu,ru,de,fr,jp,in,kz,ua,by,vn,tr,es,com.br,com.ar} // plus self-hosted *.bitrix.info. Subdomain regex matches DNS label rules. - bitrixDomainRegex = regexp.MustCompile(`^[a-z0-9]([a-z0-9-]{0,61}[a-z0-9])?\.(bitrix24\.(com|eu|ru|de|fr|jp|in|kz|ua|by)|bitrix\.info)$`) + bitrixCloudDomainRegex = regexp.MustCompile(`^[a-z0-9]([a-z0-9-]{0,61}[a-z0-9])?\.(bitrix24\.(com|eu|ru|de|fr|jp|in|kz|ua|by|vn|tr|es|com\.br|com\.ar)|bitrix\.info)$`) + + // Valid hostname regex for self-hosted Bitrix24 instances (custom domains). + // Accepts any valid FQDN or hostname with optional port (e.g. bx.example.com, portal.internal:8443). + selfHostedDomainRegex = regexp.MustCompile(`^[a-z0-9]([a-z0-9-]{0,61}[a-z0-9])?(\.[a-z0-9]([a-z0-9-]{0,61}[a-z0-9])?)*(\:\d+)?$`) // Portal name: lowercase slug used in install state token + channel config // reference. Underscore allowed for legacy CLI-created portals. @@ -142,10 +150,18 @@ func (m *BitrixPortalsMethods) handleCreate(ctx context.Context, client *gateway client.SendResponse(protocol.NewErrorResponse(req.ID, protocol.ErrInvalidRequest, i18n.T(locale, i18n.MsgInvalidRequest, "name: lowercase letters, digits, hyphen, underscore (2-64 chars)"))) return } - if !bitrixDomainRegex.MatchString(domain) { - client.SendResponse(protocol.NewErrorResponse(req.ID, protocol.ErrInvalidRequest, i18n.T(locale, i18n.MsgInvalidRequest, "domain: must be *.bitrix24.{com,eu,ru,…} or *.bitrix.info"))) + if !bitrixCloudDomainRegex.MatchString(domain) && !selfHostedDomainRegex.MatchString(domain) { + 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 @@ -352,6 +368,75 @@ func portalRowToView(row store.BitrixPortalData) bitrixPortalView { return v } +// lookupHost is the DNS resolver used by validateSelfHostedDomain. +// Replaced in tests to avoid real network calls and to exercise multi-IP +// SSRF bypass scenarios. +var lookupHost = net.LookupHost + +// 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 where ANY resolved IP is blocked (not just the first) +// - 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 ALL returned IPs. DNS result ordering + // is not a security boundary — a resolver may return a public IP first + // followed by a private one (e.g. split-horizon, fallback addresses). + addrs, err := 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) + } + for _, addr := range addrs { + ip := net.ParseIP(addr) + if ip == nil { + return fmt.Errorf("resolved address %q is not a valid IP", addr) + } + 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 1405267b..c5cfc76a 100644 --- a/internal/gateway/methods/bitrix_portals_test.go +++ b/internal/gateway/methods/bitrix_portals_test.go @@ -4,6 +4,7 @@ import ( "context" "encoding/json" "errors" + "fmt" "strings" "sync" "testing" @@ -366,17 +367,62 @@ 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) + + 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) + } + }) + } +} + +func TestBitrixPortals_Create_SelfHostedDomain(t *testing.T) { + tid := uuid.MustParse("22222222-2222-2222-2222-222222222222") + pStore := newStubBitrixPortalStore() + m := NewBitrixPortalsMethods(pStore, newStubChannelInstanceStore(), gatewayURLFn("https://goclaw.tamgiac.com")) + + // Use a cloud domain (bitrixCloudDomainRegex) for the happy-path test + // since it bypasses SSRF DNS validation. Self-hosted SSRF validation + // is covered by TestValidateSelfHostedDomain_* tests. + client, ch := gateway.NewCapturingTestClient(permissions.RoleAdmin, tid, "admin", 4) m.handleCreate(store.WithTenantID(context.Background(), tid), client, buildBitrixReq(t, protocol.MethodBitrixPortalsCreate, map[string]string{ - "name": "p", - "domain": "not-a-bitrix-domain.com", - "client_id": "x", - "client_secret": "y", + "name": "myportal", + "domain": "myportal.bitrix24.com", + "client_id": "local.abc", + "client_secret": "secret123", })) resp := readResponse(t, ch) - if resp.Error == nil || resp.Error.Code != protocol.ErrInvalidRequest { - t.Errorf("expected INVALID_REQUEST, got %+v", resp.Error) + if resp.Error != nil { + t.Fatalf("create with self-hosted domain failed: %+v", resp.Error) + } + result := resp.Payload.(map[string]any) + if result["domain"] != "myportal.bitrix24.com" { + t.Errorf("domain = %q, want myportal.bitrix24.com", result["domain"]) } } @@ -540,22 +586,51 @@ func TestBitrixDomainRegex(t *testing.T) { "my-corp.bitrix24.eu", "a.bitrix24.com", "company.bitrix.info", + "mycorp.bitrix24.vn", + "portal.bitrix24.tr", + "miempresa.bitrix24.es", + "empresa.bitrix24.com.br", } bad := []string{ "tamgiac.bitrix24", - "tamgiac.example.com", "tamgiac.bitrix24.xx", "-bad.bitrix24.com", "UPPER.bitrix24.com", // we lowercase before match "a.b.bitrix24.com", // multi-level subdomain not allowed } for _, d := range good { - if !bitrixDomainRegex.MatchString(d) { + if !bitrixCloudDomainRegex.MatchString(d) { t.Errorf("should accept %q", d) } } for _, d := range bad { - if bitrixDomainRegex.MatchString(d) { + if bitrixCloudDomainRegex.MatchString(d) { + t.Errorf("should reject %q", d) + } + } +} + +func TestSelfHostedDomainRegex(t *testing.T) { + good := []string{ + "bx.example.com", + "portal.internal", + "bitrix.mycompany.co.uk", + "portal.example.com:8443", + "bx.corp", + } + bad := []string{ + "-bad.example.com", + "UPPER.example.com", // we lowercase before match + "", + "not a domain", + } + for _, d := range good { + if !selfHostedDomainRegex.MatchString(d) { + t.Errorf("should accept %q", d) + } + } + for _, d := range bad { + if selfHostedDomainRegex.MatchString(d) { t.Errorf("should reject %q", d) } } @@ -578,6 +653,101 @@ 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_StubbedDNS(t *testing.T) { + // Save and restore the real resolver. + orig := lookupHost + defer func() { lookupHost = orig }() + + tests := []struct { + name string + host string + addrs []string + dnsErr error + wantErr bool + }{ + {"single public IP", "public.example.com", []string{"8.8.8.8"}, nil, false}, + {"public IP with port", "public.example.com", []string{"93.184.216.34"}, nil, false}, + {"single private IP", "internal.example.com", []string{"10.0.0.1"}, nil, true}, + {"DNS resolution failure", "unresolvable.example.com", nil, fmt.Errorf("no such host"), true}, + {"no addresses returned", "empty.example.com", []string{}, nil, true}, + {"invalid IP string", "bad.example.com", []string{"not-an-ip"}, nil, true}, + // Regression: multi-IP SSRF bypass — public IP first, private IP second. + // DNS ordering is not a security boundary; must reject if ANY IP is blocked. + {"multi-ip public-then-private", "evil.example.com", []string{"8.8.8.8", "10.0.0.1"}, nil, true}, + {"multi-ip private-then-public", "evil2.example.com", []string{"192.168.1.1", "8.8.4.4"}, nil, true}, + {"multi-ip all public", "safe.example.com", []string{"8.8.8.8", "8.8.4.4"}, nil, false}, + {"multi-ip metadata IP", "meta.example.com", []string{"8.8.8.8", "169.254.169.254"}, nil, true}, + {"multi-ip IPv6 loopback", "ipv6evil.example.com", []string{"2001:db8::1", "::1"}, nil, true}, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + lookupHost = func(host string) ([]string, error) { + if host == tc.host { + return tc.addrs, tc.dnsErr + } + return nil, fmt.Errorf("unexpected host: %s", host) + } + + domain := tc.host + // Add port for the "public IP with port" case. + if tc.name == "public IP with port" { + domain = tc.host + ":443" + } + + err := validateSelfHostedDomain(domain) + if tc.wantErr && err == nil { + t.Errorf("expected error for %q, got nil", tc.name) + } + if !tc.wantErr && err != nil { + t.Errorf("expected no error for %q, got: %v", tc.name, 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 13772dda..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 @@ -10,13 +10,72 @@ import { useBitrixPortalCreate } from "./use-bitrix-portals"; // Validation mirrors the server-side regex in // internal/gateway/methods/bitrix_portals.go. Server is authoritative; // client validation is purely UX so the operator gets feedback before a -// round-trip. Pattern intentionally accepts a wide TLD set (Bitrix24 has -// regional clouds: .com, .eu, .ru, .de, .fr, .jp, .in, .kz, .ua, .by) plus -// .bitrix.info for self-hosted. +// round-trip. Pattern accepts Bitrix24 regional clouds (.com, .eu, .ru, +// .de, .fr, .jp, .in, .kz, .ua, .by, .vn, .tr, .es, .com.br, .com.ar), +// .bitrix.info self-hosted, plus any valid hostname/port for fully +// self-hosted custom domains. const BITRIX_DOMAIN_RE = - /^[a-z0-9]([a-z0-9-]{0,61}[a-z0-9])?\.(bitrix24\.(com|eu|ru|de|fr|jp|in|kz|ua|by)|bitrix\.info)$/; + /^[a-z0-9]([a-z0-9-]{0,61}[a-z0-9])?\.(bitrix24\.(com|eu|ru|de|fr|jp|in|kz|ua|by|vn|tr|es|com\.br|com\.ar)|bitrix\.info)$/; +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; @@ -53,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())) { + 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 Bitrix24 portal domain (e.g. mycorp.bitrix24.com).", + 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" }); @@ -121,7 +191,7 @@ export function BitrixPortalFormStep({ onSuccess, onCancel }: BitrixPortalFormSt value={domain} onChange={(e) => setDomain(e.target.value)} onBlur={handleDomainBlur} - placeholder="tamgiac.bitrix24.com" + placeholder="mycorp.bitrix24.vn or bitrix.example.com" autoComplete="off" autoFocus />