From eff65aa05c74e93533c2504b8141b0474011e68c Mon Sep 17 00:00:00 2001 From: Chris Date: Wed, 7 Feb 2018 11:05:46 -0600 Subject: ABC-132: sign error page parameters (#8197) * sign error page parameters * add comments --- api/api.go | 6 +++- api/context.go | 6 ++-- api/file.go | 4 +-- api4/file.go | 4 +-- api4/oauth.go | 24 +++++++++------ app/app.go | 22 +++++++++----- app/config.go | 88 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ app/config_test.go | 9 ++++++ model/system.go | 23 ++++++++++---- utils/api.go | 25 ++++++++++++---- utils/api_test.go | 49 ++++++++++++++++++++++++++++++ web/web.go | 2 +- 12 files changed, 226 insertions(+), 36 deletions(-) create mode 100644 utils/api_test.go diff --git a/api/api.go b/api/api.go index 2d65bb216..70f36db85 100644 --- a/api/api.go +++ b/api/api.go @@ -109,7 +109,7 @@ func Init(a *app.App, root *mux.Router) *API { api.InitReaction() // 404 on any api route before web.go has a chance to serve it - root.Handle("/api/{anything:.*}", http.HandlerFunc(Handle404)) + root.Handle("/api/{anything:.*}", http.HandlerFunc(api.Handle404)) a.InitEmailBatching() @@ -120,6 +120,10 @@ func Init(a *app.App, root *mux.Router) *API { return api } +func (api *API) Handle404(w http.ResponseWriter, r *http.Request) { + Handle404(api.App, w, r) +} + func ReturnStatusOK(w http.ResponseWriter) { m := make(map[string]string) m[model.STATUS] = model.STATUS_OK diff --git a/api/context.go b/api/context.go index b28a24731..a8ff2b694 100644 --- a/api/context.go +++ b/api/context.go @@ -229,7 +229,7 @@ func (h handler) ServeHTTP(w http.ResponseWriter, r *http.Request) { if c.Err.StatusCode == http.StatusUnauthorized { http.Redirect(w, r, c.GetTeamURL()+"/?redirect="+url.QueryEscape(r.URL.Path), http.StatusTemporaryRedirect) } else { - utils.RenderWebError(c.Err, w, r) + utils.RenderWebAppError(w, r, c.Err, c.App.AsymmetricSigningKey()) } } @@ -434,7 +434,7 @@ func IsApiCall(r *http.Request) bool { return strings.Index(r.URL.Path, "/api/") == 0 } -func Handle404(w http.ResponseWriter, r *http.Request) { +func Handle404(a *app.App, w http.ResponseWriter, r *http.Request) { err := model.NewAppError("Handle404", "api.context.404.app_error", nil, "", http.StatusNotFound) l4g.Debug("%v: code=404 ip=%v", r.URL.Path, utils.GetIpAddress(r)) @@ -444,7 +444,7 @@ func Handle404(w http.ResponseWriter, r *http.Request) { err.DetailedError = "There doesn't appear to be an api call for the url='" + r.URL.Path + "'. Typo? are you missing a team_id or user_id as part of the url?" w.Write([]byte(err.ToJson())) } else { - utils.RenderWebError(err, w, r) + utils.RenderWebAppError(w, r, err, a.AsymmetricSigningKey()) } } diff --git a/api/file.go b/api/file.go index 2d626304e..3b8984816 100644 --- a/api/file.go +++ b/api/file.go @@ -174,12 +174,12 @@ func getPublicFile(c *Context, w http.ResponseWriter, r *http.Request) { if hash != correctHash { c.Err = model.NewAppError("getPublicFile", "api.file.get_file.public_invalid.app_error", nil, "", http.StatusBadRequest) - http.Redirect(w, r, c.GetSiteURLHeader()+"/error?message="+utils.T(c.Err.Message), http.StatusTemporaryRedirect) + utils.RenderWebAppError(w, r, c.Err, c.App.AsymmetricSigningKey()) return } } else { c.Err = model.NewAppError("getPublicFile", "api.file.get_file.public_invalid.app_error", nil, "", http.StatusBadRequest) - http.Redirect(w, r, c.GetSiteURLHeader()+"/error?message="+utils.T(c.Err.Message), http.StatusTemporaryRedirect) + utils.RenderWebAppError(w, r, c.Err, c.App.AsymmetricSigningKey()) return } diff --git a/api4/file.go b/api4/file.go index 48ee281fe..acc4c78e5 100644 --- a/api4/file.go +++ b/api4/file.go @@ -281,13 +281,13 @@ func getPublicFile(c *Context, w http.ResponseWriter, r *http.Request) { if len(hash) == 0 { c.Err = model.NewAppError("getPublicFile", "api.file.get_file.public_invalid.app_error", nil, "", http.StatusBadRequest) - http.Redirect(w, r, c.GetSiteURLHeader()+"/error?message="+utils.T(c.Err.Message), http.StatusTemporaryRedirect) + utils.RenderWebAppError(w, r, c.Err, c.App.AsymmetricSigningKey()) return } if hash != app.GeneratePublicLinkHash(info.Id, *c.App.Config().FileSettings.PublicLinkSalt) { c.Err = model.NewAppError("getPublicFile", "api.file.get_file.public_invalid.app_error", nil, "", http.StatusBadRequest) - http.Redirect(w, r, c.GetSiteURLHeader()+"/error?message="+utils.T(c.Err.Message), http.StatusTemporaryRedirect) + utils.RenderWebAppError(w, r, c.Err, c.App.AsymmetricSigningKey()) return } diff --git a/api4/oauth.go b/api4/oauth.go index 655adaaee..d0f43256a 100644 --- a/api4/oauth.go +++ b/api4/oauth.go @@ -313,7 +313,7 @@ func deauthorizeOAuthApp(c *Context, w http.ResponseWriter, r *http.Request) { func authorizeOAuthPage(c *Context, w http.ResponseWriter, r *http.Request) { if !c.App.Config().ServiceSettings.EnableOAuthServiceProvider { err := model.NewAppError("authorizeOAuth", "api.oauth.authorize_oauth.disabled.app_error", nil, "", http.StatusNotImplemented) - utils.RenderWebError(err, w, r) + utils.RenderWebAppError(w, r, err, c.App.AsymmetricSigningKey()) return } @@ -326,13 +326,13 @@ func authorizeOAuthPage(c *Context, w http.ResponseWriter, r *http.Request) { } if err := authRequest.IsValid(); err != nil { - utils.RenderWebError(err, w, r) + utils.RenderWebAppError(w, r, err, c.App.AsymmetricSigningKey()) return } oauthApp, err := c.App.GetOAuthApp(authRequest.ClientId) if err != nil { - utils.RenderWebError(err, w, r) + utils.RenderWebAppError(w, r, err, c.App.AsymmetricSigningKey()) return } @@ -343,7 +343,8 @@ func authorizeOAuthPage(c *Context, w http.ResponseWriter, r *http.Request) { } if !oauthApp.IsValidRedirectURL(authRequest.RedirectUri) { - utils.RenderWebError(model.NewAppError("authorizeOAuthPage", "api.oauth.allow_oauth.redirect_callback.app_error", nil, "", http.StatusBadRequest), w, r) + err := model.NewAppError("authorizeOAuthPage", "api.oauth.allow_oauth.redirect_callback.app_error", nil, "", http.StatusBadRequest) + utils.RenderWebAppError(w, r, err, c.App.AsymmetricSigningKey()) return } @@ -360,7 +361,7 @@ func authorizeOAuthPage(c *Context, w http.ResponseWriter, r *http.Request) { redirectUrl, err := c.App.AllowOAuthAppAccessToUser(c.Session.UserId, authRequest) if err != nil { - utils.RenderWebError(err, w, r) + utils.RenderWebAppError(w, r, err, c.App.AsymmetricSigningKey()) return } @@ -441,7 +442,10 @@ func completeOAuth(c *Context, w http.ResponseWriter, r *http.Request) { code := r.URL.Query().Get("code") if len(code) == 0 { - http.Redirect(w, r, c.GetSiteURLHeader()+"/error?type=oauth_missing_code&service="+strings.Title(service), http.StatusTemporaryRedirect) + utils.RenderWebError(w, r, http.StatusTemporaryRedirect, url.Values{ + "type": []string{"oauth_missing_code"}, + "service": []string{strings.Title(service)}, + }, c.App.AsymmetricSigningKey()) return } @@ -462,7 +466,7 @@ func completeOAuth(c *Context, w http.ResponseWriter, r *http.Request) { if action == model.OAUTH_ACTION_MOBILE { w.Write([]byte(err.ToJson())) } else { - http.Redirect(w, r, c.GetSiteURLHeader()+"/error?message="+url.QueryEscape(err.Message), http.StatusTemporaryRedirect) + utils.RenderWebAppError(w, r, err, c.App.AsymmetricSigningKey()) } return } @@ -474,7 +478,7 @@ func completeOAuth(c *Context, w http.ResponseWriter, r *http.Request) { if action == model.OAUTH_ACTION_MOBILE { w.Write([]byte(err.ToJson())) } else { - http.Redirect(w, r, c.GetSiteURLHeader()+"/error?message="+url.QueryEscape(err.Message), http.StatusTemporaryRedirect) + utils.RenderWebAppError(w, r, err, c.App.AsymmetricSigningKey()) } return } @@ -559,7 +563,9 @@ func signupWithOAuth(c *Context, w http.ResponseWriter, r *http.Request) { } if !c.App.Config().TeamSettings.EnableUserCreation { - http.Redirect(w, r, c.GetSiteURLHeader()+"/error?message="+url.QueryEscape(utils.T("api.oauth.singup_with_oauth.disabled.app_error")), http.StatusTemporaryRedirect) + utils.RenderWebError(w, r, http.StatusBadRequest, url.Values{ + "message": []string{utils.T("api.oauth.singup_with_oauth.disabled.app_error")}, + }, c.App.AsymmetricSigningKey()) return } diff --git a/app/app.go b/app/app.go index 1e46d29d0..0b5efa76b 100644 --- a/app/app.go +++ b/app/app.go @@ -4,6 +4,7 @@ package app import ( + "crypto/ecdsa" "html/template" "net" "net/http" @@ -60,13 +61,14 @@ type App struct { newStore func() store.Store - htmlTemplateWatcher *utils.HTMLTemplateWatcher - sessionCache *utils.Cache - roles map[string]*model.Role - configListenerId string - licenseListenerId string - disableConfigWatch bool - configWatcher *utils.ConfigWatcher + htmlTemplateWatcher *utils.HTMLTemplateWatcher + sessionCache *utils.Cache + roles map[string]*model.Role + configListenerId string + licenseListenerId string + disableConfigWatch bool + configWatcher *utils.ConfigWatcher + asymmetricSigningKey *ecdsa.PrivateKey pluginCommands []*PluginCommand pluginCommandsLock sync.RWMutex @@ -139,6 +141,10 @@ func New(options ...Option) (*App, error) { } app.Srv.Store = app.newStore() + if err := app.ensureAsymmetricSigningKey(); err != nil { + return nil, errors.Wrapf(err, "unable to ensure asymmetric signing key") + } + app.initJobs() app.initBuiltInPlugins() @@ -448,5 +454,5 @@ func (a *App) Handle404(w http.ResponseWriter, r *http.Request) { l4g.Debug("%v: code=404 ip=%v", r.URL.Path, utils.GetIpAddress(r)) - utils.RenderWebError(err, w, r) + utils.RenderWebAppError(w, r, err, a.AsymmetricSigningKey()) } diff --git a/app/config.go b/app/config.go index a2398f9e9..46426c442 100644 --- a/app/config.go +++ b/app/config.go @@ -4,7 +4,12 @@ package app import ( + "crypto/ecdsa" + "crypto/elliptic" "crypto/md5" + "crypto/rand" + "crypto/x509" + "encoding/base64" "encoding/json" "fmt" "runtime/debug" @@ -116,8 +121,91 @@ func (a *App) InvokeConfigListeners(old, current *model.Config) { } } +// EnsureAsymmetricSigningKey ensures that an asymmetric signing key exists and future calls to +// AsymmetricSigningKey will always return a valid signing key. +func (a *App) ensureAsymmetricSigningKey() error { + if a.asymmetricSigningKey != nil { + return nil + } + + var key *model.SystemAsymmetricSigningKey + + result := <-a.Srv.Store.System().GetByName(model.SYSTEM_ASYMMETRIC_SIGNING_KEY) + if result.Err == nil { + if err := json.Unmarshal([]byte(result.Data.(*model.System).Value), &key); err != nil { + return err + } + } + + // If we don't already have a key, try to generate one. + if key == nil { + newECDSAKey, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) + if err != nil { + return err + } + newKey := &model.SystemAsymmetricSigningKey{ + ECDSAKey: &model.SystemECDSAKey{ + Curve: "P-256", + X: newECDSAKey.X, + Y: newECDSAKey.Y, + D: newECDSAKey.D, + }, + } + system := &model.System{ + Name: model.SYSTEM_ASYMMETRIC_SIGNING_KEY, + } + v, err := json.Marshal(newKey) + if err != nil { + return err + } + system.Value = string(v) + if result = <-a.Srv.Store.System().Save(system); result.Err == nil { + // If we were able to save the key, use it, otherwise ignore the error. + key = newKey + } + } + + // If we weren't able to save a new key above, another server must have beat us to it. Get the + // key from the database, and if that fails, error out. + if key == nil { + result := <-a.Srv.Store.System().GetByName(model.SYSTEM_ASYMMETRIC_SIGNING_KEY) + if result.Err != nil { + return result.Err + } else if err := json.Unmarshal([]byte(result.Data.(*model.System).Value), &key); err != nil { + return err + } + } + + var curve elliptic.Curve + switch key.ECDSAKey.Curve { + case "P-256": + curve = elliptic.P256() + default: + return fmt.Errorf("unknown curve: " + key.ECDSAKey.Curve) + } + a.asymmetricSigningKey = &ecdsa.PrivateKey{ + PublicKey: ecdsa.PublicKey{ + Curve: curve, + X: key.ECDSAKey.X, + Y: key.ECDSAKey.Y, + }, + D: key.ECDSAKey.D, + } + a.regenerateClientConfig() + return nil +} + +// AsymmetricSigningKey will return a private key that can be used for asymmetric signing. +func (a *App) AsymmetricSigningKey() *ecdsa.PrivateKey { + return a.asymmetricSigningKey +} + func (a *App) regenerateClientConfig() { a.clientConfig = utils.GenerateClientConfig(a.Config(), a.DiagnosticId()) + if key := a.AsymmetricSigningKey(); key != nil { + der, _ := x509.MarshalPKIXPublicKey(&key.PublicKey) + a.clientConfig["AsymmetricSigningPublicKey"] = base64.StdEncoding.EncodeToString(der) + } clientConfigJSON, _ := json.Marshal(a.clientConfig) a.clientConfigHash = fmt.Sprintf("%x", md5.Sum(clientConfigJSON)) } diff --git a/app/config_test.go b/app/config_test.go index e3d50b958..5ee999f0f 100644 --- a/app/config_test.go +++ b/app/config_test.go @@ -6,6 +6,8 @@ package app import ( "testing" + "github.com/stretchr/testify/assert" + "github.com/mattermost/mattermost-server/model" ) @@ -54,3 +56,10 @@ func TestConfigListener(t *testing.T) { t.Fatal("listener 2 should've been called") } } + +func TestAsymmetricSigningKey(t *testing.T) { + th := Setup().InitBasic() + defer th.TearDown() + assert.NotNil(t, th.App.AsymmetricSigningKey()) + assert.NotEmpty(t, th.App.ClientConfig()["AsymmetricSigningPublicKey"]) +} diff --git a/model/system.go b/model/system.go index 020c50858..2a636b14f 100644 --- a/model/system.go +++ b/model/system.go @@ -6,14 +6,16 @@ package model import ( "encoding/json" "io" + "math/big" ) const ( - SYSTEM_DIAGNOSTIC_ID = "DiagnosticId" - SYSTEM_RAN_UNIT_TESTS = "RanUnitTests" - SYSTEM_LAST_SECURITY_TIME = "LastSecurityTime" - SYSTEM_ACTIVE_LICENSE_ID = "ActiveLicenseId" - SYSTEM_LAST_COMPLIANCE_TIME = "LastComplianceTime" + SYSTEM_DIAGNOSTIC_ID = "DiagnosticId" + SYSTEM_RAN_UNIT_TESTS = "RanUnitTests" + SYSTEM_LAST_SECURITY_TIME = "LastSecurityTime" + SYSTEM_ACTIVE_LICENSE_ID = "ActiveLicenseId" + SYSTEM_LAST_COMPLIANCE_TIME = "LastComplianceTime" + SYSTEM_ASYMMETRIC_SIGNING_KEY = "AsymmetricSigningKey" ) type System struct { @@ -31,3 +33,14 @@ func SystemFromJson(data io.Reader) *System { json.NewDecoder(data).Decode(&o) return o } + +type SystemAsymmetricSigningKey struct { + ECDSAKey *SystemECDSAKey `json:"ecdsa_key,omitempty"` +} + +type SystemECDSAKey struct { + Curve string `json:"curve"` + X *big.Int `json:"x"` + Y *big.Int `json:"y"` + D *big.Int `json:"d,omitempty"` +} diff --git a/utils/api.go b/utils/api.go index 005c3284b..51524074d 100644 --- a/utils/api.go +++ b/utils/api.go @@ -4,6 +4,9 @@ package utils import ( + "crypto" + "crypto/rand" + "encoding/base64" "fmt" "html/template" "net/http" @@ -32,13 +35,25 @@ func OriginChecker(allowedOrigins string) func(*http.Request) bool { } } -func RenderWebError(err *model.AppError, w http.ResponseWriter, r *http.Request) { - status := http.StatusTemporaryRedirect - if err.StatusCode != http.StatusInternalServerError { - status = err.StatusCode +func RenderWebAppError(w http.ResponseWriter, r *http.Request, err *model.AppError, s crypto.Signer) { + RenderWebError(w, r, err.StatusCode, url.Values{ + "message": []string{err.Message}, + }, s) +} + +func RenderWebError(w http.ResponseWriter, r *http.Request, status int, params url.Values, s crypto.Signer) { + queryString := params.Encode() + + h := crypto.SHA256 + sum := h.New() + sum.Write([]byte("/error?" + queryString)) + signature, err := s.Sign(rand.Reader, sum.Sum(nil), h) + if err != nil { + http.Error(w, "", http.StatusInternalServerError) + return } + destination := strings.TrimRight(GetSiteURL(), "/") + "/error?" + queryString + "&s=" + base64.URLEncoding.EncodeToString(signature) - destination := strings.TrimRight(GetSiteURL(), "/") + "/error?message=" + url.QueryEscape(err.Message) if status >= 300 && status < 400 { http.Redirect(w, r, destination, status) return diff --git a/utils/api_test.go b/utils/api_test.go new file mode 100644 index 000000000..5e41c7bfe --- /dev/null +++ b/utils/api_test.go @@ -0,0 +1,49 @@ +// Copyright (c) 2017-present Mattermost, Inc. All Rights Reserved. +// See License.txt for license information. + +package utils + +import ( + "crypto/ecdsa" + "crypto/elliptic" + "crypto/rand" + "crypto/sha256" + "encoding/asn1" + "encoding/base64" + "math/big" + "net/http" + "net/http/httptest" + "net/url" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestRenderWebError(t *testing.T) { + r := httptest.NewRequest("GET", "http://foo", nil) + w := httptest.NewRecorder() + key, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) + require.NoError(t, err) + RenderWebError(w, r, http.StatusTemporaryRedirect, url.Values{ + "foo": []string{"bar"}, + }, key) + + resp := w.Result() + location, err := url.Parse(resp.Header.Get("Location")) + require.NoError(t, err) + require.NotEmpty(t, location.Query().Get("s")) + + type ecdsaSignature struct { + R, S *big.Int + } + var rs ecdsaSignature + s, err := base64.URLEncoding.DecodeString(location.Query().Get("s")) + require.NoError(t, err) + _, err = asn1.Unmarshal(s, &rs) + require.NoError(t, err) + + assert.Equal(t, "bar", location.Query().Get("foo")) + h := sha256.Sum256([]byte("/error?foo=bar")) + assert.True(t, ecdsa.Verify(&key.PublicKey, h[:], rs.R, rs.S)) +} diff --git a/web/web.go b/web/web.go index 321d83a75..e0edd1b7a 100644 --- a/web/web.go +++ b/web/web.go @@ -94,7 +94,7 @@ func root(c *api.Context, w http.ResponseWriter, r *http.Request) { } if api.IsApiCall(r) { - api.Handle404(w, r) + api.Handle404(c.App, w, r) return } -- cgit v1.2.3-1-g7c22