Skip to content

Commit

Permalink
Prevent all courier HTTP requests from accessing local networks
Browse files Browse the repository at this point in the history
  • Loading branch information
rowanseymour committed Oct 30, 2023
1 parent 8f8ef1b commit eb08b7c
Show file tree
Hide file tree
Showing 76 changed files with 346 additions and 289 deletions.
2 changes: 1 addition & 1 deletion attachments.go
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ func FetchAndStoreAttachment(ctx context.Context, b Backend, channel Channel, at
return nil, errors.Wrap(err, "unable to create attachment request")
}

trace, err := httpx.DoTrace(utils.GetHTTPClient(), attRequest, nil, nil, maxAttBodyReadBytes)
trace, err := httpx.DoTrace(b.HttpClient(true), attRequest, nil, b.HttpAccess(), maxAttBodyReadBytes)
if trace != nil {
clog.HTTP(trace)

Expand Down
6 changes: 6 additions & 0 deletions backend.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,11 @@ package courier
import (
"context"
"fmt"
"net/http"
"strings"

"github.com/gomodule/redigo/redis"
"github.com/nyaruka/gocommon/httpx"
"github.com/nyaruka/gocommon/urns"
)

Expand Down Expand Up @@ -88,6 +90,10 @@ type Backend interface {
// ResolveMedia resolves an outgoing attachment URL to a media object
ResolveMedia(context.Context, string) (Media, error)

// HttpClient returns an HTTP client for making external requests
HttpClient(bool) *http.Client
HttpAccess() *httpx.AccessConfig

// Health returns a string describing any health problems the backend has, or empty string if all is well
Health() string

Expand Down
35 changes: 35 additions & 0 deletions backends/rapidpro/backend.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,12 @@ package rapidpro
import (
"bytes"
"context"
"crypto/tls"
"database/sql"
"encoding/json"
"fmt"
"log/slog"
"net/http"
"net/url"
"path"
"path/filepath"
Expand All @@ -23,6 +25,7 @@ import (
"github.com/nyaruka/courier/queue"
"github.com/nyaruka/gocommon/analytics"
"github.com/nyaruka/gocommon/dbutil"
"github.com/nyaruka/gocommon/httpx"
"github.com/nyaruka/gocommon/jsonx"
"github.com/nyaruka/gocommon/storage"
"github.com/nyaruka/gocommon/syncx"
Expand Down Expand Up @@ -66,6 +69,10 @@ type backend struct {
stopChan chan bool
waitGroup *sync.WaitGroup

httpClient *http.Client
httpClientInsecure *http.Client
httpAccess *httpx.AccessConfig

mediaCache *redisx.IntervalHash
mediaMutexes syncx.HashMutex

Expand All @@ -85,9 +92,26 @@ type backend struct {

// NewBackend creates a new RapidPro backend
func newBackend(cfg *courier.Config) courier.Backend {
transport := http.DefaultTransport.(*http.Transport).Clone()
transport.MaxIdleConns = 64
transport.MaxIdleConnsPerHost = 8
transport.IdleConnTimeout = 15 * time.Second

insecureTransport := http.DefaultTransport.(*http.Transport).Clone()
insecureTransport.MaxIdleConns = 64
insecureTransport.MaxIdleConnsPerHost = 8
insecureTransport.IdleConnTimeout = 15 * time.Second
insecureTransport.TLSClientConfig = &tls.Config{InsecureSkipVerify: true}

disallowedIPs, disallowedNets, _ := cfg.ParseDisallowedNetworks()

return &backend{
config: cfg,

httpClient: &http.Client{Transport: transport, Timeout: 30 * time.Second},
httpClientInsecure: &http.Client{Transport: insecureTransport, Timeout: 30 * time.Second},
httpAccess: httpx.NewAccessConfig(10*time.Second, disallowedIPs, disallowedNets),

stopChan: make(chan bool),
waitGroup: &sync.WaitGroup{},

Expand Down Expand Up @@ -720,6 +744,17 @@ func (b *backend) ResolveMedia(ctx context.Context, mediaUrl string) (courier.Me
return media, nil
}

func (b *backend) HttpClient(secure bool) *http.Client {
if secure {
return b.httpClient
}
return b.httpClientInsecure

Check warning on line 751 in backends/rapidpro/backend.go

View check run for this annotation

Codecov / codecov/patch

backends/rapidpro/backend.go#L747-L751

Added lines #L747 - L751 were not covered by tests
}

func (b *backend) HttpAccess() *httpx.AccessConfig {
return b.httpAccess

Check warning on line 755 in backends/rapidpro/backend.go

View check run for this annotation

Codecov / codecov/patch

backends/rapidpro/backend.go#L754-L755

Added lines #L754 - L755 were not covered by tests
}

// Health returns the health of this backend as a string, returning "" if all is well
func (b *backend) Health() string {
// test redis
Expand Down
56 changes: 44 additions & 12 deletions config.go
Original file line number Diff line number Diff line change
@@ -1,7 +1,15 @@
package courier

import (
"encoding/csv"
"io"
"net"
"strings"

"github.com/nyaruka/courier/utils"
"github.com/nyaruka/ezconf"
"github.com/nyaruka/gocommon/httpx"
"github.com/pkg/errors"
)

// Config is our top level configuration object
Expand Down Expand Up @@ -30,15 +38,16 @@ type Config struct {
FacebookWebhookSecret string `help:"the secret for Facebook webhook URL verification"`
WhatsappAdminSystemUserToken string `help:"the token of the admin system user for WhatsApp"`

MediaDomain string `help:"the domain on which we'll try to resolve outgoing media URLs"`
MaxWorkers int `help:"the maximum number of go routines that will be used for sending (set to 0 to disable sending)"`
LibratoUsername string `help:"the username that will be used to authenticate to Librato"`
LibratoToken string `help:"the token that will be used to authenticate to Librato"`
StatusUsername string `help:"the username that is needed to authenticate against the /status endpoint"`
StatusPassword string `help:"the password that is needed to authenticate against the /status endpoint"`
AuthToken string `help:"the authentication token need to access non-channel endpoints"`
LogLevel string `help:"the logging level courier should use"`
Version string `help:"the version that will be used in request and response headers"`
DisallowedNetworks string `help:"comma separated list of IP addresses and networks which we disallow fetching attachments from"`
MediaDomain string `help:"the domain on which we'll try to resolve outgoing media URLs"`
MaxWorkers int `help:"the maximum number of go routines that will be used for sending (set to 0 to disable sending)"`
LibratoUsername string `help:"the username that will be used to authenticate to Librato"`
LibratoToken string `help:"the token that will be used to authenticate to Librato"`
StatusUsername string `help:"the username that is needed to authenticate against the /status endpoint"`
StatusPassword string `help:"the password that is needed to authenticate against the /status endpoint"`
AuthToken string `help:"the authentication token need to access non-channel endpoints"`
LogLevel string `help:"the logging level courier should use"`
Version string `help:"the version that will be used in request and response headers"`

// IncludeChannels is the list of channels to enable, empty means include all
IncludeChannels []string
Expand Down Expand Up @@ -73,9 +82,10 @@ func NewConfig() *Config {
FacebookWebhookSecret: "missing_facebook_webhook_secret",
WhatsappAdminSystemUserToken: "missing_whatsapp_admin_system_user_token",

MaxWorkers: 32,
LogLevel: "error",
Version: "Dev",
DisallowedNetworks: `127.0.0.1,::1,10.0.0.0/8,172.16.0.0/12,192.168.0.0/16,169.254.0.0/16,fe80::/10`,
MaxWorkers: 32,
LogLevel: "error",
Version: "Dev",
}
}

Expand All @@ -91,3 +101,25 @@ func LoadConfig(filename string) *Config {
loader.MustLoad()
return config
}

// Validate validates the config
func (c *Config) Validate() error {
if err := utils.Validate(c); err != nil {
return err
}

Check warning on line 109 in config.go

View check run for this annotation

Codecov / codecov/patch

config.go#L106-L109

Added lines #L106 - L109 were not covered by tests

if _, _, err := c.ParseDisallowedNetworks(); err != nil {
return errors.Wrap(err, "unable to parse 'DisallowedNetworks'")
}
return nil

Check warning on line 114 in config.go

View check run for this annotation

Codecov / codecov/patch

config.go#L111-L114

Added lines #L111 - L114 were not covered by tests
}

// ParseDisallowedNetworks parses the list of IPs and IP networks (written in CIDR notation)
func (c *Config) ParseDisallowedNetworks() ([]net.IP, []*net.IPNet, error) {
addrs, err := csv.NewReader(strings.NewReader(c.DisallowedNetworks)).Read()
if err != nil && err != io.EOF {
return nil, nil, err
}

Check warning on line 122 in config.go

View check run for this annotation

Codecov / codecov/patch

config.go#L118-L122

Added lines #L118 - L122 were not covered by tests

return httpx.ParseNetworks(addrs...)

Check warning on line 124 in config.go

View check run for this annotation

Codecov / codecov/patch

config.go#L124

Added line #L124 was not covered by tests
}
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ require (
github.com/jmoiron/sqlx v1.3.5
github.com/lib/pq v1.10.9
github.com/nyaruka/ezconf v0.2.1
github.com/nyaruka/gocommon v1.42.1
github.com/nyaruka/gocommon v1.42.2
github.com/nyaruka/null/v3 v3.0.0
github.com/nyaruka/redisx v0.5.0
github.com/patrickmn/go-cache v2.1.0+incompatible
Expand Down
4 changes: 2 additions & 2 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -69,8 +69,8 @@ github.com/naoina/toml v0.1.1 h1:PT/lllxVVN0gzzSqSlHEmP8MJB4MY2U7STGxiouV4X8=
github.com/naoina/toml v0.1.1/go.mod h1:NBIhNtsFMo3G2szEBne+bO4gS192HuIYRqfvOWb4i1E=
github.com/nyaruka/ezconf v0.2.1 h1:TDXWoqjqYya1uhou1mAJZg7rgFYL98EB0Tb3+BWtUh0=
github.com/nyaruka/ezconf v0.2.1/go.mod h1:ey182kYkw2MIi4XiWe1FR/mzI33WCmTWuceDYYxgnQw=
github.com/nyaruka/gocommon v1.42.1 h1:BIS+RpgG06Vl4nzrPLxuFFVF+KTP7PZ+V1xJE1ksLBo=
github.com/nyaruka/gocommon v1.42.1/go.mod h1:JuphjZr/q+GYycaXSQ1WmXzJdbqkbm0iMBlqxxVcF8M=
github.com/nyaruka/gocommon v1.42.2 h1:VGJ/h7WNmCyQ6wNYClJfFkXkU7ZZn+Aiz9xoKJHVRH4=
github.com/nyaruka/gocommon v1.42.2/go.mod h1:JuphjZr/q+GYycaXSQ1WmXzJdbqkbm0iMBlqxxVcF8M=
github.com/nyaruka/librato v1.1.1 h1:0nTYtJLl3Sn7lX3CuHsLf+nXy1k/tGV0OjVxLy3Et4s=
github.com/nyaruka/librato v1.1.1/go.mod h1:fme1Fu1PT2qvkaBZyw8WW+SrnFe2qeeCWpvqmAaKAKE=
github.com/nyaruka/null/v2 v2.0.3 h1:rdmMRQyVzrOF3Jff/gpU/7BDR9mQX0lcLl4yImsA3kw=
Expand Down
2 changes: 1 addition & 1 deletion handlers/africastalking/handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,7 @@ func (h *handler) Send(ctx context.Context, msg courier.MsgOut, clog *courier.Ch
req.Header.Set("Accept", "application/json")
req.Header.Set("apikey", apiKey)

resp, respBody, err := handlers.RequestHTTP(req, clog)
resp, respBody, err := h.RequestHTTP(req, clog)
if err != nil || resp.StatusCode/100 != 2 {
return status, nil
}
Expand Down
2 changes: 1 addition & 1 deletion handlers/arabiacell/handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ func (h *handler) Send(ctx context.Context, msg courier.MsgOut, clog *courier.Ch
req.Header.Set("Content-Type", "application/x-www-form-urlencoded")
req.Header.Set("Accept", "application/xml")

resp, respBody, err := handlers.RequestHTTP(req, clog)
resp, respBody, err := h.RequestHTTP(req, clog)
if err != nil || resp.StatusCode/100 != 2 {
return status, nil
}
Expand Down
2 changes: 1 addition & 1 deletion handlers/bandwidth/handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -227,7 +227,7 @@ func (h *handler) Send(ctx context.Context, msg courier.MsgOut, clog *courier.Ch
req.Header.Set("Accept", "application/json")
req.SetBasicAuth(username, password)

resp, respBody, _ := handlers.RequestHTTP(req, clog)
resp, respBody, _ := h.RequestHTTP(req, clog)

response := &mtResponse{}
err = json.Unmarshal(respBody, response)
Expand Down
32 changes: 32 additions & 0 deletions handlers/base.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,12 @@ package handlers

import (
"context"
"fmt"
"net/http"

"github.com/go-chi/chi"
"github.com/nyaruka/courier"
"github.com/nyaruka/gocommon/httpx"
)

var defaultRedactConfigKeys = []string{courier.ConfigAuthToken, courier.ConfigAPIKey, courier.ConfigSecret, courier.ConfigPassword, courier.ConfigSendAuthorization}
Expand Down Expand Up @@ -98,6 +100,36 @@ func (h *BaseHandler) GetChannel(ctx context.Context, r *http.Request) (courier.
return h.backend.GetChannel(ctx, h.ChannelType(), uuid)
}

// RequestHTTP does the given request, logging the trace, and returns the response
func (h *BaseHandler) RequestHTTP(req *http.Request, clog *courier.ChannelLog) (*http.Response, []byte, error) {
return h.RequestHTTPWithClient(h.backend.HttpClient(true), req, clog)
}

// RequestHTTP does the given request, logging the trace, and returns the response
func (h *BaseHandler) RequestHTTPInsecure(req *http.Request, clog *courier.ChannelLog) (*http.Response, []byte, error) {
return h.RequestHTTPWithClient(h.backend.HttpClient(false), req, clog)

Check warning on line 110 in handlers/base.go

View check run for this annotation

Codecov / codecov/patch

handlers/base.go#L109-L110

Added lines #L109 - L110 were not covered by tests
}

// RequestHTTP does the given request using the given client, logging the trace, and returns the response
func (h *BaseHandler) RequestHTTPWithClient(client *http.Client, req *http.Request, clog *courier.ChannelLog) (*http.Response, []byte, error) {
var resp *http.Response
var body []byte

req.Header.Set("User-Agent", fmt.Sprintf("Courier/%s", h.server.Config().Version))

trace, err := httpx.DoTrace(client, req, nil, h.backend.HttpAccess(), 0)
if trace != nil {
clog.HTTP(trace)
resp = trace.Response
body = trace.ResponseBody
}
if err != nil {
return nil, nil, err
}

Check warning on line 128 in handlers/base.go

View check run for this annotation

Codecov / codecov/patch

handlers/base.go#L127-L128

Added lines #L127 - L128 were not covered by tests

return resp, body, nil
}

// WriteStatusSuccessResponse writes a success response for the statuses
func (h *BaseHandler) WriteStatusSuccessResponse(ctx context.Context, w http.ResponseWriter, statuses []courier.StatusUpdate) error {
return courier.WriteStatusSuccess(w, statuses)
Expand Down
12 changes: 9 additions & 3 deletions handlers/http_test.go → handlers/base_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import (
"github.com/stretchr/testify/assert"
)

func TestDoHTTPRequest(t *testing.T) {
func TestRequestHTTP(t *testing.T) {
httpx.SetRequestor(httpx.NewMockRequestor(map[string][]*httpx.MockResponse{
"https://api.messages.com/send.json": {
httpx.NewMockResponse(200, nil, []byte(`{"status":"success"}`)),
Expand All @@ -26,8 +26,14 @@ func TestDoHTTPRequest(t *testing.T) {
mm := mb.NewOutgoingMsg(mc, 123, urns.URN("tel:+1234"), "Hello World", false, nil, "", "", courier.MsgOriginChat, nil)
clog := courier.NewChannelLogForSend(mm, nil)

config := courier.NewConfig()
server := test.NewMockServer(config, mb)

h := handlers.NewBaseHandler("NX", "Test")
h.SetServer(server)

req, _ := http.NewRequest("POST", "https://api.messages.com/send.json", nil)
resp, respBody, err := handlers.RequestHTTP(req, clog)
resp, respBody, err := h.RequestHTTP(req, clog)
assert.NoError(t, err)
assert.Equal(t, 200, resp.StatusCode)
assert.Equal(t, []byte(`{"status":"success"}`), respBody)
Expand All @@ -38,7 +44,7 @@ func TestDoHTTPRequest(t *testing.T) {
assert.Equal(t, "https://api.messages.com/send.json", hlog1.URL)

req, _ = http.NewRequest("POST", "https://api.messages.com/send.json", nil)
resp, _, err = handlers.RequestHTTP(req, clog)
resp, _, err = h.RequestHTTP(req, clog)
assert.NoError(t, err)
assert.Equal(t, 400, resp.StatusCode)
assert.Len(t, clog.HTTPLogs(), 2)
Expand Down
2 changes: 1 addition & 1 deletion handlers/bongolive/handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,7 @@ func (h *handler) Send(ctx context.Context, msg courier.MsgOut, clog *courier.Ch
}
req.Header.Set("Content-Type", "application/x-www-form-urlencoded")

resp, respBody, err := handlers.RequestHTTPInsecure(req, clog)
resp, respBody, err := h.RequestHTTPInsecure(req, clog)
if err != nil || resp.StatusCode/100 != 2 {
return status, nil
}
Expand Down
2 changes: 1 addition & 1 deletion handlers/burstsms/handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ func (h *handler) Send(ctx context.Context, msg courier.MsgOut, clog *courier.Ch
req.Header.Set("Content-Type", "application/x-www-form-urlencoded")
req.Header.Set("Accept", "application/json")

resp, respBody, err := handlers.RequestHTTP(req, clog)
resp, respBody, err := h.RequestHTTP(req, clog)
if err != nil || resp.StatusCode/100 != 2 {
return status, nil
}
Expand Down
2 changes: 1 addition & 1 deletion handlers/clickatell/handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,7 @@ func (h *handler) Send(ctx context.Context, msg courier.MsgOut, clog *courier.Ch
req.Header.Set("Content-Type", "application/x-www-form-urlencoded")
req.Header.Set("Accept", "application/json")

resp, respBody, err := handlers.RequestHTTP(req, clog)
resp, respBody, err := h.RequestHTTP(req, clog)
if err != nil || resp.StatusCode/100 != 2 {
return status, nil
}
Expand Down
2 changes: 1 addition & 1 deletion handlers/clickmobile/handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,7 @@ func (h *handler) Send(ctx context.Context, msg courier.MsgOut, clog *courier.Ch
req.Header.Set("Content-Type", "application/json")
req.Header.Set("Accept", "application/json")

resp, respBody, err := handlers.RequestHTTP(req, clog)
resp, respBody, err := h.RequestHTTP(req, clog)
if err != nil || resp.StatusCode/100 != 2 {
return status, nil
}
Expand Down
2 changes: 1 addition & 1 deletion handlers/clicksend/handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ func (h *handler) Send(ctx context.Context, msg courier.MsgOut, clog *courier.Ch
req.Header.Set("Accept", "application/json")
req.SetBasicAuth(username, password)

resp, respBody, err := handlers.RequestHTTP(req, clog)
resp, respBody, err := h.RequestHTTP(req, clog)
if err != nil || resp.StatusCode/100 != 2 {
return status, nil
}
Expand Down
2 changes: 1 addition & 1 deletion handlers/dart/handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -186,7 +186,7 @@ func (h *handler) Send(ctx context.Context, msg courier.MsgOut, clog *courier.Ch

req.Header.Set("Content-Type", "application/x-www-form-urlencoded")

resp, respBody, err := handlers.RequestHTTP(req, clog)
resp, respBody, err := h.RequestHTTP(req, clog)
if err != nil || resp.StatusCode/100 != 2 {
return status, nil
}
Expand Down
Loading

0 comments on commit eb08b7c

Please sign in to comment.