From ffbf8e51fe0b80b39fa76535f96c9179b2fcc0a1 Mon Sep 17 00:00:00 2001 From: Chris Date: Wed, 9 Aug 2017 15:49:07 -0500 Subject: PLT-6358: Server HTTP client improvements (#6980) * restrict untrusted, internal http connections by default * command test fix * more test fixes * change setting from toggle to whitelist * requested ui changes * add isdefault diagnostic * fix tests --- api/command_test.go | 3 + api/post_test.go | 6 + api4/command_test.go | 3 + api4/openGraph_test.go | 3 + api4/post_test.go | 3 + app/command.go | 2 +- app/diagnostics.go | 87 ++++++------ app/notification.go | 2 +- app/oauth.go | 4 +- app/post.go | 2 +- app/webhook.go | 2 +- app/webrtc.go | 17 ++- app/webtrc.go | 28 ---- config/config.json | 1 + model/config.go | 5 + utils/httpclient.go | 156 +++++++++++++++++++-- utils/httpclient_test.go | 86 +++++++++++- .../admin_console/developer_settings.jsx | 27 +++- webapp/i18n/en.json | 3 + 19 files changed, 342 insertions(+), 98 deletions(-) delete mode 100644 app/webtrc.go diff --git a/api/command_test.go b/api/command_test.go index 9e6696d64..dd4180b16 100644 --- a/api/command_test.go +++ b/api/command_test.go @@ -233,10 +233,13 @@ func TestTestCommand(t *testing.T) { channel1 := th.SystemAdminChannel enableCommands := *utils.Cfg.ServiceSettings.EnableCommands + allowedInternalConnections := *utils.Cfg.ServiceSettings.AllowedUntrustedInternalConnections defer func() { utils.Cfg.ServiceSettings.EnableCommands = &enableCommands + utils.Cfg.ServiceSettings.AllowedUntrustedInternalConnections = &allowedInternalConnections }() *utils.Cfg.ServiceSettings.EnableCommands = true + *utils.Cfg.ServiceSettings.AllowedUntrustedInternalConnections = "localhost" cmd1 := &model.Command{ URL: "http://localhost" + utils.Cfg.ServiceSettings.ListenAddress + model.API_URL_SUFFIX_V3 + "/teams/command_test", diff --git a/api/post_test.go b/api/post_test.go index c7bd7a04c..a48ad4f51 100644 --- a/api/post_test.go +++ b/api/post_test.go @@ -188,10 +188,13 @@ func testCreatePostWithOutgoingHook( channel := th.CreateChannel(Client, team) enableOutgoingHooks := utils.Cfg.ServiceSettings.EnableOutgoingWebhooks + allowedInternalConnections := *utils.Cfg.ServiceSettings.AllowedUntrustedInternalConnections defer func() { utils.Cfg.ServiceSettings.EnableOutgoingWebhooks = enableOutgoingHooks + utils.Cfg.ServiceSettings.AllowedUntrustedInternalConnections = &allowedInternalConnections }() utils.Cfg.ServiceSettings.EnableOutgoingWebhooks = true + *utils.Cfg.ServiceSettings.AllowedUntrustedInternalConnections = "localhost 127.0.0.1" var hook *model.OutgoingWebhook var post *model.Post @@ -1359,10 +1362,13 @@ func TestGetOpenGraphMetadata(t *testing.T) { Client := th.BasicClient enableLinkPreviews := *utils.Cfg.ServiceSettings.EnableLinkPreviews + allowedInternalConnections := *utils.Cfg.ServiceSettings.AllowedUntrustedInternalConnections defer func() { *utils.Cfg.ServiceSettings.EnableLinkPreviews = enableLinkPreviews + utils.Cfg.ServiceSettings.AllowedUntrustedInternalConnections = &allowedInternalConnections }() *utils.Cfg.ServiceSettings.EnableLinkPreviews = true + *utils.Cfg.ServiceSettings.AllowedUntrustedInternalConnections = "localhost 127.0.0.1" ogDataCacheMissCount := 0 diff --git a/api4/command_test.go b/api4/command_test.go index 467d45955..b0d5f4baa 100644 --- a/api4/command_test.go +++ b/api4/command_test.go @@ -388,10 +388,13 @@ func TestExecuteCommand(t *testing.T) { channel := th.BasicChannel enableCommands := *utils.Cfg.ServiceSettings.EnableCommands + allowedInternalConnections := *utils.Cfg.ServiceSettings.AllowedUntrustedInternalConnections defer func() { utils.Cfg.ServiceSettings.EnableCommands = &enableCommands + utils.Cfg.ServiceSettings.AllowedUntrustedInternalConnections = &allowedInternalConnections }() *utils.Cfg.ServiceSettings.EnableCommands = true + *utils.Cfg.ServiceSettings.AllowedUntrustedInternalConnections = "localhost" postCmd := &model.Command{ CreatorId: th.BasicUser.Id, diff --git a/api4/openGraph_test.go b/api4/openGraph_test.go index 958abf604..df1af66fc 100644 --- a/api4/openGraph_test.go +++ b/api4/openGraph_test.go @@ -19,10 +19,13 @@ func TestGetOpenGraphMetadata(t *testing.T) { Client := th.Client enableLinkPreviews := *utils.Cfg.ServiceSettings.EnableLinkPreviews + allowedInternalConnections := *utils.Cfg.ServiceSettings.AllowedUntrustedInternalConnections defer func() { *utils.Cfg.ServiceSettings.EnableLinkPreviews = enableLinkPreviews + utils.Cfg.ServiceSettings.AllowedUntrustedInternalConnections = &allowedInternalConnections }() *utils.Cfg.ServiceSettings.EnableLinkPreviews = true + *utils.Cfg.ServiceSettings.AllowedUntrustedInternalConnections = "localhost 127.0.0.1" ogDataCacheMissCount := 0 diff --git a/api4/post_test.go b/api4/post_test.go index f136ba676..b7ed06bd4 100644 --- a/api4/post_test.go +++ b/api4/post_test.go @@ -119,14 +119,17 @@ func testCreatePostWithOutgoingHook( enableOutgoingHooks := utils.Cfg.ServiceSettings.EnableOutgoingWebhooks enableAdminOnlyHooks := utils.Cfg.ServiceSettings.EnableOnlyAdminIntegrations + allowedInternalConnections := *utils.Cfg.ServiceSettings.AllowedUntrustedInternalConnections defer func() { utils.Cfg.ServiceSettings.EnableOutgoingWebhooks = enableOutgoingHooks utils.Cfg.ServiceSettings.EnableOnlyAdminIntegrations = enableAdminOnlyHooks utils.SetDefaultRolesBasedOnConfig() + utils.Cfg.ServiceSettings.AllowedUntrustedInternalConnections = &allowedInternalConnections }() utils.Cfg.ServiceSettings.EnableOutgoingWebhooks = true *utils.Cfg.ServiceSettings.EnableOnlyAdminIntegrations = true utils.SetDefaultRolesBasedOnConfig() + *utils.Cfg.ServiceSettings.AllowedUntrustedInternalConnections = "localhost 127.0.0.1" var hook *model.OutgoingWebhook var post *model.Post diff --git a/app/command.go b/app/command.go index bfb97ae0c..7fe11fffc 100644 --- a/app/command.go +++ b/app/command.go @@ -209,7 +209,7 @@ func ExecuteCommand(args *model.CommandArgs) (*model.CommandResponse, *model.App req.Header.Set("Content-Type", "application/x-www-form-urlencoded") } - if resp, err := utils.HttpClient().Do(req); err != nil { + if resp, err := utils.HttpClient(false).Do(req); err != nil { return nil, model.NewAppError("command", "api.command.execute_command.failed.app_error", map[string]interface{}{"Trigger": trigger}, err.Error(), http.StatusInternalServerError) } else { if resp.StatusCode == http.StatusOK { diff --git a/app/diagnostics.go b/app/diagnostics.go index 983ce8077..aff223855 100644 --- a/app/diagnostics.go +++ b/app/diagnostics.go @@ -174,49 +174,50 @@ func trackActivity() { func trackConfig() { SendDiagnostic(TRACK_CONFIG_SERVICE, map[string]interface{}{ - "web_server_mode": *utils.Cfg.ServiceSettings.WebserverMode, - "enable_security_fix_alert": *utils.Cfg.ServiceSettings.EnableSecurityFixAlert, - "enable_insecure_outgoing_connections": *utils.Cfg.ServiceSettings.EnableInsecureOutgoingConnections, - "enable_incoming_webhooks": utils.Cfg.ServiceSettings.EnableIncomingWebhooks, - "enable_outgoing_webhooks": utils.Cfg.ServiceSettings.EnableOutgoingWebhooks, - "enable_commands": *utils.Cfg.ServiceSettings.EnableCommands, - "enable_only_admin_integrations": *utils.Cfg.ServiceSettings.EnableOnlyAdminIntegrations, - "enable_post_username_override": utils.Cfg.ServiceSettings.EnablePostUsernameOverride, - "enable_post_icon_override": utils.Cfg.ServiceSettings.EnablePostIconOverride, - "enable_apiv3": *utils.Cfg.ServiceSettings.EnableAPIv3, - "enable_user_access_tokens": *utils.Cfg.ServiceSettings.EnableUserAccessTokens, - "enable_custom_emoji": *utils.Cfg.ServiceSettings.EnableCustomEmoji, - "enable_emoji_picker": *utils.Cfg.ServiceSettings.EnableEmojiPicker, - "restrict_custom_emoji_creation": *utils.Cfg.ServiceSettings.RestrictCustomEmojiCreation, - "enable_testing": utils.Cfg.ServiceSettings.EnableTesting, - "enable_developer": *utils.Cfg.ServiceSettings.EnableDeveloper, - "enable_multifactor_authentication": *utils.Cfg.ServiceSettings.EnableMultifactorAuthentication, - "enforce_multifactor_authentication": *utils.Cfg.ServiceSettings.EnforceMultifactorAuthentication, - "enable_oauth_service_provider": utils.Cfg.ServiceSettings.EnableOAuthServiceProvider, - "connection_security": *utils.Cfg.ServiceSettings.ConnectionSecurity, - "uses_letsencrypt": *utils.Cfg.ServiceSettings.UseLetsEncrypt, - "forward_80_to_443": *utils.Cfg.ServiceSettings.Forward80To443, - "maximum_login_attempts": utils.Cfg.ServiceSettings.MaximumLoginAttempts, - "session_length_web_in_days": *utils.Cfg.ServiceSettings.SessionLengthWebInDays, - "session_length_mobile_in_days": *utils.Cfg.ServiceSettings.SessionLengthMobileInDays, - "session_length_sso_in_days": *utils.Cfg.ServiceSettings.SessionLengthSSOInDays, - "session_cache_in_minutes": *utils.Cfg.ServiceSettings.SessionCacheInMinutes, - "isdefault_site_url": isDefault(*utils.Cfg.ServiceSettings.SiteURL, model.SERVICE_SETTINGS_DEFAULT_SITE_URL), - "isdefault_tls_cert_file": isDefault(*utils.Cfg.ServiceSettings.TLSCertFile, model.SERVICE_SETTINGS_DEFAULT_TLS_CERT_FILE), - "isdefault_tls_key_file": isDefault(*utils.Cfg.ServiceSettings.TLSKeyFile, model.SERVICE_SETTINGS_DEFAULT_TLS_KEY_FILE), - "isdefault_read_timeout": isDefault(*utils.Cfg.ServiceSettings.ReadTimeout, model.SERVICE_SETTINGS_DEFAULT_READ_TIMEOUT), - "isdefault_write_timeout": isDefault(*utils.Cfg.ServiceSettings.WriteTimeout, model.SERVICE_SETTINGS_DEFAULT_WRITE_TIMEOUT), - "isdefault_google_developer_key": isDefault(utils.Cfg.ServiceSettings.GoogleDeveloperKey, ""), - "isdefault_allow_cors_from": isDefault(*utils.Cfg.ServiceSettings.AllowCorsFrom, model.SERVICE_SETTINGS_DEFAULT_ALLOW_CORS_FROM), - "restrict_post_delete": *utils.Cfg.ServiceSettings.RestrictPostDelete, - "allow_edit_post": *utils.Cfg.ServiceSettings.AllowEditPost, - "post_edit_time_limit": *utils.Cfg.ServiceSettings.PostEditTimeLimit, - "enable_user_typing_messages": *utils.Cfg.ServiceSettings.EnableUserTypingMessages, - "enable_channel_viewed_messages": *utils.Cfg.ServiceSettings.EnableChannelViewedMessages, - "time_between_user_typing_updates_milliseconds": *utils.Cfg.ServiceSettings.TimeBetweenUserTypingUpdatesMilliseconds, - "cluster_log_timeout_milliseconds": *utils.Cfg.ServiceSettings.ClusterLogTimeoutMilliseconds, - "enable_post_search": *utils.Cfg.ServiceSettings.EnablePostSearch, - "enable_user_statuses": *utils.Cfg.ServiceSettings.EnableUserStatuses, + "web_server_mode": *utils.Cfg.ServiceSettings.WebserverMode, + "enable_security_fix_alert": *utils.Cfg.ServiceSettings.EnableSecurityFixAlert, + "enable_insecure_outgoing_connections": *utils.Cfg.ServiceSettings.EnableInsecureOutgoingConnections, + "enable_incoming_webhooks": utils.Cfg.ServiceSettings.EnableIncomingWebhooks, + "enable_outgoing_webhooks": utils.Cfg.ServiceSettings.EnableOutgoingWebhooks, + "enable_commands": *utils.Cfg.ServiceSettings.EnableCommands, + "enable_only_admin_integrations": *utils.Cfg.ServiceSettings.EnableOnlyAdminIntegrations, + "enable_post_username_override": utils.Cfg.ServiceSettings.EnablePostUsernameOverride, + "enable_post_icon_override": utils.Cfg.ServiceSettings.EnablePostIconOverride, + "enable_apiv3": *utils.Cfg.ServiceSettings.EnableAPIv3, + "enable_user_access_tokens": *utils.Cfg.ServiceSettings.EnableUserAccessTokens, + "enable_custom_emoji": *utils.Cfg.ServiceSettings.EnableCustomEmoji, + "enable_emoji_picker": *utils.Cfg.ServiceSettings.EnableEmojiPicker, + "restrict_custom_emoji_creation": *utils.Cfg.ServiceSettings.RestrictCustomEmojiCreation, + "enable_testing": utils.Cfg.ServiceSettings.EnableTesting, + "enable_developer": *utils.Cfg.ServiceSettings.EnableDeveloper, + "enable_multifactor_authentication": *utils.Cfg.ServiceSettings.EnableMultifactorAuthentication, + "enforce_multifactor_authentication": *utils.Cfg.ServiceSettings.EnforceMultifactorAuthentication, + "enable_oauth_service_provider": utils.Cfg.ServiceSettings.EnableOAuthServiceProvider, + "connection_security": *utils.Cfg.ServiceSettings.ConnectionSecurity, + "uses_letsencrypt": *utils.Cfg.ServiceSettings.UseLetsEncrypt, + "forward_80_to_443": *utils.Cfg.ServiceSettings.Forward80To443, + "maximum_login_attempts": utils.Cfg.ServiceSettings.MaximumLoginAttempts, + "session_length_web_in_days": *utils.Cfg.ServiceSettings.SessionLengthWebInDays, + "session_length_mobile_in_days": *utils.Cfg.ServiceSettings.SessionLengthMobileInDays, + "session_length_sso_in_days": *utils.Cfg.ServiceSettings.SessionLengthSSOInDays, + "session_cache_in_minutes": *utils.Cfg.ServiceSettings.SessionCacheInMinutes, + "isdefault_site_url": isDefault(*utils.Cfg.ServiceSettings.SiteURL, model.SERVICE_SETTINGS_DEFAULT_SITE_URL), + "isdefault_tls_cert_file": isDefault(*utils.Cfg.ServiceSettings.TLSCertFile, model.SERVICE_SETTINGS_DEFAULT_TLS_CERT_FILE), + "isdefault_tls_key_file": isDefault(*utils.Cfg.ServiceSettings.TLSKeyFile, model.SERVICE_SETTINGS_DEFAULT_TLS_KEY_FILE), + "isdefault_read_timeout": isDefault(*utils.Cfg.ServiceSettings.ReadTimeout, model.SERVICE_SETTINGS_DEFAULT_READ_TIMEOUT), + "isdefault_write_timeout": isDefault(*utils.Cfg.ServiceSettings.WriteTimeout, model.SERVICE_SETTINGS_DEFAULT_WRITE_TIMEOUT), + "isdefault_google_developer_key": isDefault(utils.Cfg.ServiceSettings.GoogleDeveloperKey, ""), + "isdefault_allow_cors_from": isDefault(*utils.Cfg.ServiceSettings.AllowCorsFrom, model.SERVICE_SETTINGS_DEFAULT_ALLOW_CORS_FROM), + "isdefault_allowed_untrusted_internal_connections": isDefault(*utils.Cfg.ServiceSettings.AllowedUntrustedInternalConnections, ""), + "restrict_post_delete": *utils.Cfg.ServiceSettings.RestrictPostDelete, + "allow_edit_post": *utils.Cfg.ServiceSettings.AllowEditPost, + "post_edit_time_limit": *utils.Cfg.ServiceSettings.PostEditTimeLimit, + "enable_user_typing_messages": *utils.Cfg.ServiceSettings.EnableUserTypingMessages, + "enable_channel_viewed_messages": *utils.Cfg.ServiceSettings.EnableChannelViewedMessages, + "time_between_user_typing_updates_milliseconds": *utils.Cfg.ServiceSettings.TimeBetweenUserTypingUpdatesMilliseconds, + "cluster_log_timeout_milliseconds": *utils.Cfg.ServiceSettings.ClusterLogTimeoutMilliseconds, + "enable_post_search": *utils.Cfg.ServiceSettings.EnablePostSearch, + "enable_user_statuses": *utils.Cfg.ServiceSettings.EnableUserStatuses, }) SendDiagnostic(TRACK_CONFIG_TEAM, map[string]interface{}{ diff --git a/app/notification.go b/app/notification.go index b9153037e..f8b0dd7ce 100644 --- a/app/notification.go +++ b/app/notification.go @@ -682,7 +682,7 @@ func sendToPushProxy(msg model.PushNotification, session *model.Session) { request, _ := http.NewRequest("POST", *utils.Cfg.EmailSettings.PushNotificationServer+model.API_URL_SUFFIX_V1+"/send_push", strings.NewReader(msg.ToJson())) - if resp, err := utils.HttpClient().Do(request); err != nil { + if resp, err := utils.HttpClient(true).Do(request); err != nil { l4g.Error("Device push reported as error for UserId=%v SessionId=%v message=%v", session.UserId, session.Id, err.Error()) } else { pushResponse := model.PushResponseFromJson(resp.Body) diff --git a/app/oauth.go b/app/oauth.go index 4174f8146..d9129dd6f 100644 --- a/app/oauth.go +++ b/app/oauth.go @@ -673,7 +673,7 @@ func AuthorizeOAuthUser(w http.ResponseWriter, r *http.Request, service, code, s var ar *model.AccessResponse var bodyBytes []byte - if resp, err := utils.HttpClient().Do(req); err != nil { + if resp, err := utils.HttpClient(true).Do(req); err != nil { return nil, "", stateProps, model.NewLocAppError("AuthorizeOAuthUser", "api.user.authorize_oauth_user.token_failed.app_error", nil, err.Error()) } else { bodyBytes, _ = ioutil.ReadAll(resp.Body) @@ -702,7 +702,7 @@ func AuthorizeOAuthUser(w http.ResponseWriter, r *http.Request, service, code, s req.Header.Set("Accept", "application/json") req.Header.Set("Authorization", "Bearer "+ar.AccessToken) - if resp, err := utils.HttpClient().Do(req); err != nil { + if resp, err := utils.HttpClient(true).Do(req); err != nil { return nil, "", stateProps, model.NewLocAppError("AuthorizeOAuthUser", "api.user.authorize_oauth_user.service.app_error", map[string]interface{}{"Service": service}, err.Error()) } else { diff --git a/app/post.go b/app/post.go index 055f98373..a4b177859 100644 --- a/app/post.go +++ b/app/post.go @@ -619,7 +619,7 @@ func GetFileInfosForPost(postId string, readFromMaster bool) ([]*model.FileInfo, func GetOpenGraphMetadata(url string) *opengraph.OpenGraph { og := opengraph.NewOpenGraph() - res, err := utils.HttpClient().Get(url) + res, err := utils.HttpClient(false).Get(url) if err != nil { l4g.Error("GetOpenGraphMetadata request failed for url=%v with err=%v", url, err.Error()) return og diff --git a/app/webhook.go b/app/webhook.go index 29f642ea8..4606c207f 100644 --- a/app/webhook.go +++ b/app/webhook.go @@ -102,7 +102,7 @@ func TriggerWebhook(payload *model.OutgoingWebhookPayload, hook *model.OutgoingW req, _ := http.NewRequest("POST", url, body) req.Header.Set("Content-Type", contentType) req.Header.Set("Accept", "application/json") - if resp, err := utils.HttpClient().Do(req); err != nil { + if resp, err := utils.HttpClient(false).Do(req); err != nil { l4g.Error(utils.T("api.post.handle_webhook_events_and_forget.event_post.error"), err.Error()) } else { defer CloseBody(resp) diff --git a/app/webrtc.go b/app/webrtc.go index 4d44234a8..f8361bbb2 100644 --- a/app/webrtc.go +++ b/app/webrtc.go @@ -59,7 +59,7 @@ func GetWebrtcToken(sessionId string) (string, *model.AppError) { rq, _ := http.NewRequest("POST", *utils.Cfg.WebrtcSettings.GatewayAdminUrl, strings.NewReader(model.MapToJson(data))) rq.Header.Set("Content-Type", "application/json") - if rp, err := utils.HttpClient().Do(rq); err != nil { + if rp, err := utils.HttpClient(true).Do(rq); err != nil { return "", model.NewAppError("WebRTC.Token", "model.client.connecting.app_error", nil, err.Error(), http.StatusInternalServerError) } else if rp.StatusCode >= 300 { defer CloseBody(rp) @@ -80,3 +80,18 @@ func GenerateTurnPassword(username string, secret string) string { h.Write([]byte(username)) return base64.StdEncoding.EncodeToString(h.Sum(nil)) } + +func RevokeWebrtcToken(sessionId string) { + token := base64.StdEncoding.EncodeToString([]byte(sessionId)) + data := make(map[string]string) + data["janus"] = "remove_token" + data["token"] = token + data["transaction"] = model.NewId() + data["admin_secret"] = *utils.Cfg.WebrtcSettings.GatewayAdminSecret + + rq, _ := http.NewRequest("POST", *utils.Cfg.WebrtcSettings.GatewayAdminUrl, strings.NewReader(model.MapToJson(data))) + rq.Header.Set("Content-Type", "application/json") + + // we do not care about the response + utils.HttpClient(true).Do(rq) +} diff --git a/app/webtrc.go b/app/webtrc.go deleted file mode 100644 index 76e173651..000000000 --- a/app/webtrc.go +++ /dev/null @@ -1,28 +0,0 @@ -// Copyright (c) 2017-present Mattermost, Inc. All Rights Reserved. -// See License.txt for license information. - -package app - -import ( - "encoding/base64" - "net/http" - "strings" - - "github.com/mattermost/platform/model" - "github.com/mattermost/platform/utils" -) - -func RevokeWebrtcToken(sessionId string) { - token := base64.StdEncoding.EncodeToString([]byte(sessionId)) - data := make(map[string]string) - data["janus"] = "remove_token" - data["token"] = token - data["transaction"] = model.NewId() - data["admin_secret"] = *utils.Cfg.WebrtcSettings.GatewayAdminSecret - - rq, _ := http.NewRequest("POST", *utils.Cfg.WebrtcSettings.GatewayAdminUrl, strings.NewReader(model.MapToJson(data))) - rq.Header.Set("Content-Type", "application/json") - - // we do not care about the response - utils.HttpClient().Do(rq) -} diff --git a/config/config.json b/config/config.json index 23d1d2ec0..5acd7d177 100644 --- a/config/config.json +++ b/config/config.json @@ -27,6 +27,7 @@ "EnableDeveloper": false, "EnableSecurityFixAlert": true, "EnableInsecureOutgoingConnections": false, + "AllowedUntrustedInternalConnections": "", "EnableMultifactorAuthentication": false, "EnforceMultifactorAuthentication": false, "EnableUserAccessTokens": false, diff --git a/model/config.go b/model/config.go index 1717d61a0..933c643f2 100644 --- a/model/config.go +++ b/model/config.go @@ -158,6 +158,7 @@ type ServiceSettings struct { EnableDeveloper *bool EnableSecurityFixAlert *bool EnableInsecureOutgoingConnections *bool + AllowedUntrustedInternalConnections *string EnableMultifactorAuthentication *bool EnforceMultifactorAuthentication *bool EnableUserAccessTokens *bool @@ -629,6 +630,10 @@ func (o *Config) SetDefaults() { *o.ServiceSettings.EnableInsecureOutgoingConnections = false } + if o.ServiceSettings.AllowedUntrustedInternalConnections == nil { + o.ServiceSettings.AllowedUntrustedInternalConnections = new(string) + } + if o.ServiceSettings.EnableMultifactorAuthentication == nil { o.ServiceSettings.EnableMultifactorAuthentication = new(bool) *o.ServiceSettings.EnableMultifactorAuthentication = false diff --git a/utils/httpclient.go b/utils/httpclient.go index a2355d827..afa717637 100644 --- a/utils/httpclient.go +++ b/utils/httpclient.go @@ -4,9 +4,12 @@ package utils import ( + "context" "crypto/tls" + "errors" "net" "net/http" + "strings" "time" ) @@ -15,6 +18,11 @@ const ( requestTimeout = 30 * time.Second ) +var secureHttpClient *http.Client +var secureUntrustedHttpClient *http.Client +var insecureHttpClient *http.Client +var insecureUntrustedHttpClient *http.Client + // HttpClient returns a variation the default implementation of Client. // It uses a Transport with the same settings as the default Transport // but with the following modifications: @@ -24,27 +32,147 @@ const ( // "requestTimeout") // - skipping server certificate check if specified in "config.json" // via "ServiceSettings.EnableInsecureOutgoingConnections" -func HttpClient() *http.Client { - if Cfg.ServiceSettings.EnableInsecureOutgoingConnections != nil && *Cfg.ServiceSettings.EnableInsecureOutgoingConnections { +func HttpClient(trustURLs bool) *http.Client { + insecure := Cfg.ServiceSettings.EnableInsecureOutgoingConnections != nil && *Cfg.ServiceSettings.EnableInsecureOutgoingConnections + switch { + case insecure && trustURLs: return insecureHttpClient + case insecure: + return insecureUntrustedHttpClient + case trustURLs: + return secureHttpClient + default: + return secureUntrustedHttpClient } - return secureHttpClient } -var ( - secureHttpClient = createHttpClient(false) - insecureHttpClient = createHttpClient(true) -) +var reservedIPRanges []*net.IPNet + +func isReserved(ip net.IP) bool { + for _, ipRange := range reservedIPRanges { + if ipRange.Contains(ip) { + return true + } + } + return false +} + +func init() { + for _, cidr := range []string{ + // See https://tools.ietf.org/html/rfc6890 + "0.0.0.0/8", // This host on this network + "10.0.0.0/8", // Private-Use + "127.0.0.0/8", // Loopback + "169.254.0.0/16", // Link Local + "172.16.0.0/12", // Private-Use Networks + "192.168.0.0/16", // Private-Use Networks + "::/128", // Unspecified Address + "::1/128", // Loopback Address + "fc00::/7", // Unique-Local + "fe80::/10", // Linked-Scoped Unicast + } { + _, parsed, err := net.ParseCIDR(cidr) + if err != nil { + panic(err) + } + reservedIPRanges = append(reservedIPRanges, parsed) + } + + allowHost := func(host string) bool { + if Cfg.ServiceSettings.AllowedUntrustedInternalConnections == nil { + return false + } + for _, allowed := range strings.Fields(*Cfg.ServiceSettings.AllowedUntrustedInternalConnections) { + if host == allowed { + return true + } + } + return false + } + + allowIP := func(ip net.IP) bool { + if !isReserved(ip) { + return true + } + if Cfg.ServiceSettings.AllowedUntrustedInternalConnections == nil { + return false + } + for _, allowed := range strings.Fields(*Cfg.ServiceSettings.AllowedUntrustedInternalConnections) { + if _, ipRange, err := net.ParseCIDR(allowed); err == nil && ipRange.Contains(ip) { + return true + } + } + return false + } + + secureHttpClient = createHttpClient(false, nil, nil) + insecureHttpClient = createHttpClient(true, nil, nil) + + secureUntrustedHttpClient = createHttpClient(false, allowHost, allowIP) + insecureUntrustedHttpClient = createHttpClient(true, allowHost, allowIP) +} + +type DialContextFunction func(ctx context.Context, network, addr string) (net.Conn, error) + +var AddressForbidden error = errors.New("address forbidden") + +func dialContextFilter(dial DialContextFunction, allowHost func(host string) bool, allowIP func(ip net.IP) bool) DialContextFunction { + return func(ctx context.Context, network, addr string) (net.Conn, error) { + host, port, err := net.SplitHostPort(addr) + if err != nil { + return nil, err + } + + if allowHost != nil && allowHost(host) { + return dial(ctx, network, addr) + } + + ips, err := net.LookupIP(host) + if err != nil { + return nil, err + } + + var firstErr error + for _, ip := range ips { + select { + case <-ctx.Done(): + return nil, ctx.Err() + default: + } + + if allowIP == nil || !allowIP(ip) { + continue + } + + conn, err := dial(ctx, network, net.JoinHostPort(ip.String(), port)) + if err == nil { + return conn, nil + } + if firstErr == nil { + firstErr = err + } + } + if firstErr == nil { + return nil, AddressForbidden + } + return nil, firstErr + } +} + +func createHttpClient(enableInsecureConnections bool, allowHost func(host string) bool, allowIP func(ip net.IP) bool) *http.Client { + dialContext := (&net.Dialer{ + Timeout: connectTimeout, + KeepAlive: 30 * time.Second, + }).DialContext + + if allowHost != nil || allowIP != nil { + dialContext = dialContextFilter(dialContext, allowHost, allowIP) + } -func createHttpClient(enableInsecureConnections bool) *http.Client { client := &http.Client{ Transport: &http.Transport{ - Proxy: http.ProxyFromEnvironment, - DialContext: (&net.Dialer{ - Timeout: connectTimeout, - KeepAlive: 30 * time.Second, - DualStack: true, - }).DialContext, + Proxy: http.ProxyFromEnvironment, + DialContext: dialContext, MaxIdleConns: 100, IdleConnTimeout: 90 * time.Second, TLSHandshakeTimeout: connectTimeout, diff --git a/utils/httpclient_test.go b/utils/httpclient_test.go index 17353a4e7..1878b58b4 100644 --- a/utils/httpclient_test.go +++ b/utils/httpclient_test.go @@ -4,21 +4,63 @@ package utils import ( + "context" "fmt" "io/ioutil" + "net" "net/http" "net/http/httptest" - "os" + "net/url" "testing" ) +func TestHttpClient(t *testing.T) { + for _, allowInternal := range []bool{true, false} { + c := HttpClient(allowInternal) + for _, tc := range []struct { + URL string + IsInternal bool + }{ + { + URL: "https://google.com", + IsInternal: false, + }, + { + URL: "https://127.0.0.1", + IsInternal: true, + }, + } { + _, err := c.Get(tc.URL) + if !tc.IsInternal { + if err != nil { + t.Fatal("google is down?") + } + } else { + allowed := !tc.IsInternal || allowInternal + success := err == nil + switch e := err.(type) { + case *net.OpError: + success = e.Err != AddressForbidden + case *url.Error: + success = e.Err != AddressForbidden + } + if success != allowed { + t.Fatalf("failed for %v. allowed: %v, success %v", tc.URL, allowed, success) + } + } + } + } +} + func TestHttpClientWithProxy(t *testing.T) { proxy := createProxyServer() defer proxy.Close() - os.Setenv("HTTP_PROXY", proxy.URL) - client := HttpClient() - resp, err := client.Get("http://acme.com") + c := createHttpClient(true, nil, nil) + purl, _ := url.Parse(proxy.URL) + c.Transport.(*http.Transport).Proxy = http.ProxyURL(purl) + + resp, err := c.Get("http://acme.com") if err != nil { t.Fatal(err) } @@ -40,3 +82,39 @@ func createProxyServer() *httptest.Server { fmt.Fprint(w, "proxy") })) } + +func TestDialContextFilter(t *testing.T) { + for _, tc := range []struct { + Addr string + IsValid bool + }{ + { + Addr: "google.com:80", + IsValid: true, + }, + { + Addr: "8.8.8.8:53", + IsValid: true, + }, + { + Addr: "127.0.0.1:80", + }, + { + Addr: "10.0.0.1:80", + IsValid: true, + }, + } { + didDial := false + filter := dialContextFilter(func(ctx context.Context, network, addr string) (net.Conn, error) { + didDial = true + return nil, nil + }, func(host string) bool { return host == "10.0.0.1" }, func(ip net.IP) bool { return !isReserved(ip) }) + _, err := filter(context.Background(), "", tc.Addr) + switch { + case tc.IsValid == (err == AddressForbidden) || (err != nil && err != AddressForbidden): + t.Errorf("unexpected err for %v (%v)", tc.Addr, err) + case tc.IsValid != didDial: + t.Errorf("unexpected didDial for %v", tc.Addr) + } + } +} diff --git a/webapp/components/admin_console/developer_settings.jsx b/webapp/components/admin_console/developer_settings.jsx index c7ffd18d5..6a8f49dbd 100644 --- a/webapp/components/admin_console/developer_settings.jsx +++ b/webapp/components/admin_console/developer_settings.jsx @@ -3,10 +3,13 @@ import React from 'react'; +import * as Utils from 'utils/utils.jsx'; + import AdminSettings from './admin_settings.jsx'; import BooleanSetting from './boolean_setting.jsx'; -import {FormattedMessage} from 'react-intl'; +import {FormattedMessage, FormattedHTMLMessage} from 'react-intl'; import SettingsGroup from './settings_group.jsx'; +import TextSetting from './text_setting.jsx'; export default class DeveloperSettings extends AdminSettings { constructor(props) { @@ -20,6 +23,7 @@ export default class DeveloperSettings extends AdminSettings { getConfigFromState(config) { config.ServiceSettings.EnableTesting = this.state.enableTesting; config.ServiceSettings.EnableDeveloper = this.state.enableDeveloper; + config.ServiceSettings.AllowedUntrustedInternalConnections = this.state.allowedUntrustedInternalConnections; return config; } @@ -27,7 +31,8 @@ export default class DeveloperSettings extends AdminSettings { getStateFromConfig(config) { return { enableTesting: config.ServiceSettings.EnableTesting, - enableDeveloper: config.ServiceSettings.EnableDeveloper + enableDeveloper: config.ServiceSettings.EnableDeveloper, + allowedUntrustedInternalConnections: config.ServiceSettings.AllowedUntrustedInternalConnections }; } @@ -77,6 +82,24 @@ export default class DeveloperSettings extends AdminSettings { value={this.state.enableDeveloper} onChange={this.handleChange} /> + + } + placeholder={Utils.localizeMessage('admin.service.internalConnectionsEx', 'webhooks.internal.example.com 127.0.0.1 10.0.16.0/28')} + helpText={ + + } + value={this.state.allowedUntrustedInternalConnections} + onChange={this.handleChange} + /> ); } diff --git a/webapp/i18n/en.json b/webapp/i18n/en.json index b74c049b4..1eced0e54 100755 --- a/webapp/i18n/en.json +++ b/webapp/i18n/en.json @@ -805,6 +805,9 @@ "admin.service.iconTitle": "Enable integrations to override profile picture icons:", "admin.service.insecureTlsDesc": "When true, any outgoing HTTPS requests will accept unverified, self-signed certificates. For example, outgoing webhooks to a server with a self-signed TLS certificate, using any domain, will be allowed. Note that this makes these connections susceptible to man-in-the-middle attacks.", "admin.service.insecureTlsTitle": "Enable Insecure Outgoing Connections: ", + "admin.service.internalConnectionsDesc": "In testing environments, such as when developing integrations locally on a development machine, use this setting to specify domains, IP addresses, or CIDR notations to allow internal connections. Not recommended for use in production, since this can allow a user to extract confidential data from your server or internal network.

By default, user-supplied URLs such as those used for Open Graph metadata, webhooks, or slash commands will not be allowed to connect to reserved IP addresses including loopback or link-local addresses used for internal networks. Push notification, OAuth 2.0 and WebRTC server URLs are trusted and not affected by this setting.", + "admin.service.internalConnectionsTitle": "Allow untrusted internal connections to: ", + "admin.service.internalConnectionsEx": "webhooks.internal.example.com 127.0.0.1 10.0.16.0/28", "admin.service.integrationAdmin": "Restrict managing integrations to Admins:", "admin.service.integrationAdminDesc": "When true, webhooks and slash commands can only be created, edited and viewed by Team and System Admins, and OAuth 2.0 applications by System Admins. Integrations are available to all users after they have been created by the Admin.", "admin.service.letsEncryptCertificateCacheFile": "Let's Encrypt Certificate Cache File:", -- cgit v1.2.3-1-g7c22