From e1ed46605a9204c3befb8c9f8b8e4dedb7671bcb Mon Sep 17 00:00:00 2001 From: Jesse Hallam Date: Tue, 26 Feb 2019 12:17:49 -0500 Subject: [PATCH] use hard-coded names for saml certificates (#10341) This preserves the ability to use custom file names -- required for backwards compatibility -- but forces names upon all newly uploaded certificates, avoiding clashes with other configuration files and skipping the need for file safety checks. --- app/saml.go | 34 ++++++++++++++-------------------- i18n/en.json | 2 +- model/client4.go | 3 +++ 3 files changed, 18 insertions(+), 21 deletions(-) diff --git a/app/saml.go b/app/saml.go index f721ccd6ec..0c4a1a3b0f 100644 --- a/app/saml.go +++ b/app/saml.go @@ -15,6 +15,12 @@ import ( "github.com/mattermost/mattermost-server/utils/fileutils" ) +const ( + SamlPublicCertificateName = "saml-public.crt" + SamlPrivateKeyName = "saml-private.key" + SamlIdpCertificateName = "saml-idp.crt" +) + func (a *App) GetSamlMetadata() (string, *model.AppError) { if a.Saml == nil { err := model.NewAppError("GetSamlMetadata", "api.admin.saml.not_available.app_error", nil, "", http.StatusNotImplemented) @@ -28,13 +34,7 @@ func (a *App) GetSamlMetadata() (string, *model.AppError) { return result, nil } -func WriteSamlFile(fileData *multipart.FileHeader) *model.AppError { - filename := filepath.Base(fileData.Filename) - - if filename == "." || filename == string(filepath.Separator) { - return model.NewAppError("AddSamlCertificate", "api.admin.add_certificate.saving.app_error", nil, "", http.StatusBadRequest) - } - +func WriteSamlFile(filename string, fileData *multipart.FileHeader) *model.AppError { file, err := fileData.Open() if err != nil { return model.NewAppError("AddSamlCertificate", "api.admin.add_certificate.open.app_error", nil, err.Error(), http.StatusInternalServerError) @@ -53,12 +53,12 @@ func WriteSamlFile(fileData *multipart.FileHeader) *model.AppError { } func (a *App) AddSamlPublicCertificate(fileData *multipart.FileHeader) *model.AppError { - if err := WriteSamlFile(fileData); err != nil { + if err := WriteSamlFile(SamlPublicCertificateName, fileData); err != nil { return err } cfg := a.Config().Clone() - *cfg.SamlSettings.PublicCertificateFile = fileData.Filename + *cfg.SamlSettings.PublicCertificateFile = SamlPublicCertificateName if err := cfg.IsValid(); err != nil { return err @@ -71,12 +71,12 @@ func (a *App) AddSamlPublicCertificate(fileData *multipart.FileHeader) *model.Ap } func (a *App) AddSamlPrivateCertificate(fileData *multipart.FileHeader) *model.AppError { - if err := WriteSamlFile(fileData); err != nil { + if err := WriteSamlFile(SamlPrivateKeyName, fileData); err != nil { return err } cfg := a.Config().Clone() - *cfg.SamlSettings.PrivateKeyFile = fileData.Filename + *cfg.SamlSettings.PrivateKeyFile = SamlPrivateKeyName if err := cfg.IsValid(); err != nil { return err @@ -89,12 +89,12 @@ func (a *App) AddSamlPrivateCertificate(fileData *multipart.FileHeader) *model.A } func (a *App) AddSamlIdpCertificate(fileData *multipart.FileHeader) *model.AppError { - if err := WriteSamlFile(fileData); err != nil { + if err := WriteSamlFile(SamlIdpCertificateName, fileData); err != nil { return err } cfg := a.Config().Clone() - *cfg.SamlSettings.IdpCertificateFile = fileData.Filename + *cfg.SamlSettings.IdpCertificateFile = SamlIdpCertificateName if err := cfg.IsValid(); err != nil { return err @@ -107,14 +107,8 @@ func (a *App) AddSamlIdpCertificate(fileData *multipart.FileHeader) *model.AppEr } func RemoveSamlFile(filename string) *model.AppError { - filename = filepath.Base(filename) - - if filename == "." || filename == string(filepath.Separator) { - return model.NewAppError("AddSamlCertificate", "api.admin.remove_certificate.delete.app_error", nil, "", http.StatusBadRequest) - } - if err := os.Remove(fileutils.FindConfigFile(filename)); err != nil { - return model.NewAppError("removeCertificate", "api.admin.remove_certificate.delete.app_error", map[string]interface{}{"Filename": filename}, err.Error(), http.StatusInternalServerError) + return model.NewAppError("removeCertificate", "api.admin.remove_certificate.delete.app_error", map[string]interface{}{"Filename": filename}, filename+": "+err.Error(), http.StatusInternalServerError) } return nil diff --git a/i18n/en.json b/i18n/en.json index e0e2b4d501..4468d138d6 100644 --- a/i18n/en.json +++ b/i18n/en.json @@ -105,7 +105,7 @@ }, { "id": "api.admin.remove_certificate.delete.app_error", - "translation": "An error occurred while deleting the certificate. Make sure the file config/{{.Filename}} exists." + "translation": "An error occurred while deleting the certificate." }, { "id": "api.admin.saml.metadata.app_error", diff --git a/model/client4.go b/model/client4.go index 021a4b408d..1cdf50abd9 100644 --- a/model/client4.go +++ b/model/client4.go @@ -2944,6 +2944,7 @@ func samlFileToMultipart(data []byte, filename string) ([]byte, *multipart.Write } // UploadSamlIdpCertificate will upload an IDP certificate for SAML and set the config to use it. +// The filename parameter is deprecated and ignored: the server will pick a hard-coded filename when writing to disk. func (c *Client4) UploadSamlIdpCertificate(data []byte, filename string) (bool, *Response) { body, writer, err := samlFileToMultipart(data, filename) if err != nil { @@ -2955,6 +2956,7 @@ func (c *Client4) UploadSamlIdpCertificate(data []byte, filename string) (bool, } // UploadSamlPublicCertificate will upload a public certificate for SAML and set the config to use it. +// The filename parameter is deprecated and ignored: the server will pick a hard-coded filename when writing to disk. func (c *Client4) UploadSamlPublicCertificate(data []byte, filename string) (bool, *Response) { body, writer, err := samlFileToMultipart(data, filename) if err != nil { @@ -2966,6 +2968,7 @@ func (c *Client4) UploadSamlPublicCertificate(data []byte, filename string) (boo } // UploadSamlPrivateCertificate will upload a private key for SAML and set the config to use it. +// The filename parameter is deprecated and ignored: the server will pick a hard-coded filename when writing to disk. func (c *Client4) UploadSamlPrivateCertificate(data []byte, filename string) (bool, *Response) { body, writer, err := samlFileToMultipart(data, filename) if err != nil {