diff --git a/changes/11570-bp-url b/changes/11570-bp-url new file mode 100644 index 0000000000..a88eeb484d --- /dev/null +++ b/changes/11570-bp-url @@ -0,0 +1 @@ +* MDM: try to infer the bootstrap package name from the URL on upload if a content-disposition header is not provided. diff --git a/pkg/file/validation.go b/pkg/file/validation.go new file mode 100644 index 0000000000..b9152cae60 --- /dev/null +++ b/pkg/file/validation.go @@ -0,0 +1,13 @@ +package file + +import "strings" + +var InvalidMacOSChars = []rune{':', '\\', '*', '?', '"', '<', '>', '|', 0} + +func IsValidMacOSName(fileName string) bool { + if fileName == "" { + return false + } + + return !strings.ContainsAny(fileName, string(InvalidMacOSChars)) +} diff --git a/pkg/file/validation_test.go b/pkg/file/validation_test.go new file mode 100644 index 0000000000..91d992f75b --- /dev/null +++ b/pkg/file/validation_test.go @@ -0,0 +1,38 @@ +package file + +import ( + "testing" + + "github.com/stretchr/testify/require" +) + +func TestIsValidMacOSName(t *testing.T) { + testCases := []struct { + name string + input string + output bool + }{ + {"valid", "filename.txt", true}, + {"spaces", "file name with spaces.txt", true}, + {"dashes", "file-name-with-dashes.txt", true}, + {"underscores", "file_underscored.txt", true}, + {"non-ASCII characters", "中文文件名.txt", true}, + + {"colon", "file:name.txt", false}, + {"backslash", "file\\name.txt", false}, + {"asterisk", "file*name.txt", false}, + {"question mark", "file?name.txt", false}, + {"double quote", "file\"name.txt", false}, + {"less than", "filename.txt", false}, + {"pipe", "file|name.txt", false}, + {"null character", "file\x00name.txt", false}, + {"empty", "", false}, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + require.Equal(t, tc.output, IsValidMacOSName(tc.input)) + }) + } +} diff --git a/server/service/apple_mdm.go b/server/service/apple_mdm.go index c4b9d3514b..077a407c5f 100644 --- a/server/service/apple_mdm.go +++ b/server/service/apple_mdm.go @@ -17,6 +17,7 @@ import ( "github.com/VividCortex/mysqlerr" "github.com/docker/go-units" + "github.com/fleetdm/fleet/v4/pkg/file" "github.com/fleetdm/fleet/v4/server/authz" "github.com/fleetdm/fleet/v4/server/contexts/ctxerr" "github.com/fleetdm/fleet/v4/server/contexts/license" @@ -1662,6 +1663,12 @@ func (uploadBootstrapPackageRequest) DecodeRequest(ctx context.Context, r *http. } decoded.Package = r.MultipartForm.File["package"][0] + if !file.IsValidMacOSName(decoded.Package.Filename) { + return nil, &fleet.BadRequestError{ + Message: "package name contains invalid characters", + InternalErr: ctxerr.New(ctx, "package name contains invalid characters"), + } + } // default is no team decoded.TeamID = 0 diff --git a/server/service/client_mdm.go b/server/service/client_mdm.go index 7918eb04bd..7a69c8939d 100644 --- a/server/service/client_mdm.go +++ b/server/service/client_mdm.go @@ -11,7 +11,9 @@ import ( "mime" "mime/multipart" "net/http" + "net/url" "os" + "path" "path/filepath" "strings" @@ -146,8 +148,32 @@ func (c *Client) ValidateBootstrapPackageFromURL(url string) (*fleet.MDMAppleBoo return downloadRemoteMacosBootstrapPackage(url) } -func downloadRemoteMacosBootstrapPackage(url string) (*fleet.MDMAppleBootstrapPackage, error) { - resp, err := http.Get(url) // nolint:gosec // we want this URL to be provided by the user. It will run on their machine. +func extractFilenameFromPath(p string) string { + u, err := url.Parse(p) + if err != nil { + return "" + } + + invalid := map[string]struct{}{ + "": {}, + ".": {}, + "/": {}, + } + + b := path.Base(u.Path) + if _, ok := invalid[b]; ok { + return "" + } + + if _, ok := invalid[path.Ext(b)]; ok { + return b + ".pkg" + } + + return b +} + +func downloadRemoteMacosBootstrapPackage(pkgURL string) (*fleet.MDMAppleBootstrapPackage, error) { + resp, err := http.Get(pkgURL) // nolint:gosec // we want this URL to be provided by the user. It will run on their machine. if err != nil { return nil, fmt.Errorf("downloading bootstrap package: %w", err) } @@ -166,6 +192,13 @@ func downloadRemoteMacosBootstrapPackage(url string) (*fleet.MDMAppleBootstrapPa filename = params["filename"] } } + + // if it fails, try to extract it from the URL + if filename == "" { + filename = extractFilenameFromPath(pkgURL) + } + + // if all else fails, use a default name if filename == "" { filename = "bootstrap-package.pkg" } diff --git a/server/service/client_test.go b/server/service/client_test.go index 136145b889..e5f55a64b5 100644 --- a/server/service/client_test.go +++ b/server/service/client_test.go @@ -202,3 +202,26 @@ spec: }) } } + +func TestExtractFilenameFromPath(t *testing.T) { + cases := []struct { + in string + out string + }{ + {"http://example.com", ""}, + {"http://example.com/", ""}, + {"http://example.com?foo=bar", ""}, + {"http://example.com/foo.pkg", "foo.pkg"}, + {"http://example.com/foo.exe", "foo.exe"}, + {"http://example.com/foo.pkg?bar=baz", "foo.pkg"}, + {"http://example.com/foo.bar.pkg", "foo.bar.pkg"}, + {"http://example.com/foo", "foo.pkg"}, + {"http://example.com/foo/bar/baz", "baz.pkg"}, + {"http://example.com/foo?bar=baz", "foo.pkg"}, + } + + for _, c := range cases { + got := extractFilenameFromPath(c.in) + require.Equalf(t, c.out, got, "for URL %s", c.in) + } +} diff --git a/server/service/integration_mdm_test.go b/server/service/integration_mdm_test.go index 3112a70c37..c6c800f498 100644 --- a/server/service/integration_mdm_test.go +++ b/server/service/integration_mdm_test.go @@ -28,6 +28,7 @@ import ( "testing" "time" + "github.com/fleetdm/fleet/v4/pkg/file" "github.com/fleetdm/fleet/v4/server/config" "github.com/fleetdm/fleet/v4/server/datastore/mysql" "github.com/fleetdm/fleet/v4/server/datastore/redis/redistest" @@ -2608,6 +2609,14 @@ func (s *integrationMDMTestSuite) TestBootstrapPackage() { s.uploadBootstrapPackage(&fleet.MDMAppleBootstrapPackage{Bytes: signedPkg}, http.StatusBadRequest, "package multipart field is required") // invalid s.uploadBootstrapPackage(&fleet.MDMAppleBootstrapPackage{Bytes: invalidPkg, Name: "invalid.tar.gz"}, http.StatusBadRequest, "invalid file type") + // invalid names + for _, char := range file.InvalidMacOSChars { + s.uploadBootstrapPackage( + &fleet.MDMAppleBootstrapPackage{ + Bytes: signedPkg, + Name: fmt.Sprintf("invalid_%c_name.pkg", char), + }, http.StatusBadRequest, "") + } // unsigned s.uploadBootstrapPackage(&fleet.MDMAppleBootstrapPackage{Bytes: unsignedPkg, Name: "pkg.pkg"}, http.StatusBadRequest, "file is not signed") // wrong TOC