Skip to content

Commit

Permalink
fix(ds-host): ssrf protection allows all ports
Browse files Browse the repository at this point in the history
- ssrf protection allows all ports (due to limitations in library in use)
- config: change all port value types from int16 to uint16
- rename config InternalNetwork to LocalNetwork
  • Loading branch information
teleclimber committed Jan 23, 2024
1 parent da5db3e commit fb7799b
Show file tree
Hide file tree
Showing 5 changed files with 18 additions and 19 deletions.
4 changes: 2 additions & 2 deletions cmd/ds-host/appops/remote.go
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ func checkRedirect(req *http.Request, via []*http.Request) error {
func (r *RemoteAppGetter) getSSRF() *ssrf.Guardian {
prefixes4 := make([]netip.Prefix, 0)
prefixes6 := make([]netip.Prefix, 0)
for _, a := range r.Config.InternalNetwork.AllowedIPs {
for _, a := range r.Config.LocalNetwork.AllowedIPs {
p := getPrefix(a)
if p.Addr().Is4() {
prefixes4 = append(prefixes4, p)
Expand All @@ -109,7 +109,7 @@ func (r *RemoteAppGetter) getSSRF() *ssrf.Guardian {
}
}
return ssrf.New(
ssrf.WithPorts(443), // HTTPS only
ssrf.WithAnyPort(),
ssrf.WithAllowedV4Prefixes(prefixes4...),
ssrf.WithAllowedV6Prefixes(prefixes6...))
}
Expand Down
12 changes: 6 additions & 6 deletions cmd/ds-host/appops/remote_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,27 +42,27 @@ func TestGetPrefix6Single(t *testing.T) {

func TestGetSSRF(t *testing.T) {
cfg := domain.RuntimeConfig{}
cfg.InternalNetwork.AllowedIPs = make([]string, 0)
cfg.LocalNetwork.AllowedIPs = make([]string, 0)
r := &RemoteAppGetter{
Config: &cfg,
}

s := r.getSSRF()

err := s.Safe("tcp4", "54.84.236.175:443", nil)
err := s.Safe("tcp4", "54.84.236.175:443", nil) // 54.84.236.175 is a public IP
if err != nil {
t.Error(err)
}
err = s.Safe("tcp4", "54.84.236.175:80", nil)
if err == nil {
t.Error("expected error (prohibited port)")
err = s.Safe("tcp4", "54.84.236.175:5000", nil)
if err != nil {
t.Error(err)
}
err = s.Safe("tcp4", "192.168.1.10:443", nil)
if err == nil {
t.Error("expected error (prohibited ip)")
}

cfg.InternalNetwork.AllowedIPs = []string{"192.168.1.10"}
cfg.LocalNetwork.AllowedIPs = []string{"192.168.1.10"}
s = r.getSSRF()
err = s.Safe("tcp4", "192.168.1.10:443", nil)
if err != nil {
Expand Down
14 changes: 7 additions & 7 deletions cmd/ds-host/domain/domain.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,9 +32,9 @@ import (
type RuntimeConfig struct {
DataDir string `json:"data-dir"`
Server struct {
TLSPort int16 `json:"tls-port"` // defaults to 443.
HTTPPort int16 `json:"http-port"` // defaults to 80.
NoTLS bool `json:"no-tls"` // do not start HTTPS server
TLSPort uint16 `json:"tls-port"` // defaults to 443.
HTTPPort uint16 `json:"http-port"` // defaults to 80.
NoTLS bool `json:"no-tls"` // do not start HTTPS server
// TLS cert and key for the HTTPS server (if any).
// Leave empty if using ManageTLSCertificates
TLSCert string `json:"tls-cert"`
Expand All @@ -44,13 +44,13 @@ type RuntimeConfig struct {
Scheme string `json:"scheme"` // http or https // default to https
Subdomain string `json:"subdomain"` // for users login // default to dropid
Domain string `json:"domain"`
Port int16 `json:"port"` // default to 443
Port uint16 `json:"port"` // default to 443
} `json:"external-access"`
// TrustCert is used in ds2ds
TrustCert string `json:"trust-cert"`
InternalNetwork struct {
TrustCert string `json:"trust-cert"`
LocalNetwork struct {
AllowedIPs []string `json:"allowed-ips"` // Allowed IP addresses, or CIDR ranges.
} `json:"internal-network"`
} `json:"local-network"`
ManageTLSCertificates struct {
Enable bool `json:"enable"`
Email string `json:"acme-account-email"`
Expand Down
4 changes: 2 additions & 2 deletions cmd/ds-host/runtimeconfig/runtimeconfig.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ var configDefault = []byte(`{
"manage-certificates": {
"issuer-endpoint": "https://acme-v02.api.letsencrypt.org/directory"
},
"internal-network": {
"local-network": {
"allowed-ips": []
},
"sandbox": {
Expand Down Expand Up @@ -156,7 +156,7 @@ func validateConfig(rtc *domain.RuntimeConfig) {
}

// AllowedIPs must be either an IP address or an IP range
for _, s := range rtc.InternalNetwork.AllowedIPs {
for _, s := range rtc.LocalNetwork.AllowedIPs {
_, errA := netip.ParseAddr(s)
_, errP := netip.ParsePrefix(s)
if errA != nil && errP != nil {
Expand Down
3 changes: 1 addition & 2 deletions cmd/ds-host/runtimeconfig/runtimeconfig_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -95,10 +95,9 @@ func TestValidateServerTLS(t *testing.T) {
}
}

// TODO test allowed IP stuff
func TestDefaultAllowedIPs(t *testing.T) {
rtc := loadDefault()
if rtc.InternalNetwork.AllowedIPs == nil {
if rtc.LocalNetwork.AllowedIPs == nil {
t.Error("default config should have an empty array for allowed IPs")
}
}
Expand Down

0 comments on commit fb7799b

Please sign in to comment.