diff --git a/changes/43959-duplicate-installers b/changes/43959-duplicate-installers new file mode 100644 index 0000000000..681efc3604 --- /dev/null +++ b/changes/43959-duplicate-installers @@ -0,0 +1 @@ +- Fixed a bug where duplicate software installers for linux could be added. diff --git a/server/datastore/mysql/in_house_apps.go b/server/datastore/mysql/in_house_apps.go index 4342f6e09c..31e4f8525b 100644 --- a/server/datastore/mysql/in_house_apps.go +++ b/server/datastore/mysql/in_house_apps.go @@ -1548,6 +1548,28 @@ WHERE return exists == 1, nil } +func (ds *Datastore) checkInstallerExistsByName(ctx context.Context, q sqlx.QueryerContext, teamID *uint, name, source, platform string) (bool, error) { + const stmt = ` +SELECT 1 +FROM + software_titles st + INNER JOIN software_installers ON st.id = software_installers.title_id + AND software_installers.global_or_team_id = ? +WHERE + st.name = ? + AND st.source = ? + AND st.extension_for = '' + AND software_installers.platform = ? +` + + var exists int + err := sqlx.GetContext(ctx, q, &exists, stmt, ptr.ValOrZero(teamID), name, source, platform) + if err != nil && !errors.Is(err, sql.ErrNoRows) { + return false, ctxerr.Wrap(ctx, err, "check installer exists by name") + } + return exists == 1, nil +} + func (ds *Datastore) checkInHouseAppExistsForAdamID(ctx context.Context, q sqlx.QueryerContext, teamID *uint, appID fleet.VPPAppID) (exists bool, title string, err error) { const stmt = ` SELECT st.name diff --git a/server/datastore/mysql/software_installers.go b/server/datastore/mysql/software_installers.go index 9985aa1076..b9b47d8a85 100644 --- a/server/datastore/mysql/software_installers.go +++ b/server/datastore/mysql/software_installers.go @@ -3699,68 +3699,63 @@ LIMIT 1` } func (ds *Datastore) checkSoftwareConflictsByIdentifier(ctx context.Context, payload *fleet.UploadSoftwareInstallerPayload) error { - // if this is an in-house app, check if an installer exists - if payload.Extension == "ipa" { + switch payload.Platform { + // currently, the platform will always be ios for .ipa files + case string(fleet.IOSPlatform), string(fleet.IPadOSPlatform): // at the point where this method is called, we attempt to create both iOS and iPadOS entries // for ipa apps, so check for conflicts on either platform. for platform, source := range map[string]string{ string(fleet.IOSPlatform): "ios_apps", string(fleet.IPadOSPlatform): "ipados_apps", } { - exists, err := ds.checkInstallerOrInHouseAppExists(ctx, ds.reader(ctx), payload.TeamID, payload.BundleIdentifier, platform, softwareTypeInstaller) + exists, err := ds.checkVPPAppExistsForTitleIdentifier(ctx, ds.reader(ctx), payload.TeamID, platform, payload.BundleIdentifier, source, "") + if err != nil { + return ctxerr.Wrap(ctx, err, "check if VPP app exists for title identifier") + } + if exists { + return alreadyExists("VPP app", payload.Title) + } + + // check if equivalent installers exist, duplicate in-house apps are checked in insertInHouseApp + exists, err = ds.checkInstallerOrInHouseAppExists(ctx, ds.reader(ctx), payload.TeamID, payload.BundleIdentifier, platform, softwareTypeInstaller) if err != nil { return ctxerr.Wrap(ctx, err, "check if software installer exists for title identifier") } if exists { return alreadyExists("software installer", payload.Title) } - - exists, err = ds.checkVPPAppExistsForTitleIdentifier(ctx, ds.reader(ctx), payload.TeamID, platform, payload.BundleIdentifier, source, "") - if err != nil { - return ctxerr.Wrap(ctx, err, "check if VPP app exists for title identifier") - } - if exists { - return alreadyExists("VPP app", payload.Title) - } } - } else { - // check if a VPP app already exists for that software title in the same - // platform and team. - if payload.Platform == string(fleet.MacOSPlatform) || payload.Platform == string(fleet.IOSPlatform) || payload.Platform == string(fleet.IPadOSPlatform) { - exists, err := ds.checkVPPAppExistsForTitleIdentifier(ctx, ds.reader(ctx), payload.TeamID, payload.Platform, payload.BundleIdentifier, payload.Source, "") - if err != nil { - return ctxerr.Wrap(ctx, err, "check if VPP app exists for title identifier") - } - if exists { - return alreadyExists("VPP app", payload.Title) - } + case string(fleet.MacOSPlatform): + exists, err := ds.checkVPPAppExistsForTitleIdentifier(ctx, ds.reader(ctx), payload.TeamID, payload.Platform, payload.BundleIdentifier, payload.Source, "") + if err != nil { + return ctxerr.Wrap(ctx, err, "check if VPP app exists for title identifier") + } + if exists { + return alreadyExists("VPP app", payload.Title) } - // Check if an in-house app with the same bundle id already exists. - // Also check if equivalent installers exist, since we relaxed the uniqueness constraints to allow - // multiple FMA installer versions. - if payload.BundleIdentifier != "" { - exists, err := ds.checkInstallerOrInHouseAppExists(ctx, ds.reader(ctx), payload.TeamID, payload.BundleIdentifier, payload.Platform, softwareTypeInHouseApp) - if err != nil { - return ctxerr.Wrap(ctx, err, "check if in-house app exists for title identifier") - } - if exists { - return alreadyExists("in-house app", payload.Title) - } - - exists, err = ds.checkInstallerOrInHouseAppExists(ctx, ds.reader(ctx), payload.TeamID, payload.BundleIdentifier, payload.Platform, softwareTypeInstaller) - if err != nil { - return ctxerr.Wrap(ctx, err, "check if installer exists for title identifier") - } - if exists { - return alreadyExists("installer", payload.Title) - } + // check only for installers, since in-house apps target iOS/iPadOS so they won't conflict + exists, err = ds.checkInstallerOrInHouseAppExists(ctx, ds.reader(ctx), payload.TeamID, payload.BundleIdentifier, payload.Platform, softwareTypeInstaller) + if err != nil { + return ctxerr.Wrap(ctx, err, "check if installer exists for title identifier") + } + if exists { + return alreadyExists("installer", payload.Title) + } + case "windows", "linux": + // check by name before any software title renaming side effects can happen + exists, err := ds.checkInstallerExistsByName(ctx, ds.reader(ctx), payload.TeamID, payload.Title, payload.Source, payload.Platform) + if err != nil { + return ctxerr.Wrap(ctx, err, "check if installer exists by name") + } + if exists { + return alreadyExists("installer", payload.Title) } - if payload.Platform == "windows" { - exists, err := ds.checkInstallerOrInHouseAppExists(ctx, ds.reader(ctx), payload.TeamID, payload.Title, payload.Platform, softwareTypeInstaller) + if payload.UpgradeCode != "" { + exists, err := ds.checkInstallerOrInHouseAppExists(ctx, ds.reader(ctx), payload.TeamID, payload.UpgradeCode, payload.Platform, softwareTypeInstaller) if err != nil { - return ctxerr.Wrap(ctx, err, "check if installer exists for title identifier") + return ctxerr.Wrap(ctx, err, "check if installer exists for upgrade code") } if exists { return alreadyExists("installer", payload.Title) diff --git a/server/datastore/mysql/software_installers_test.go b/server/datastore/mysql/software_installers_test.go index 5412b94c1d..57de5dc72f 100644 --- a/server/datastore/mysql/software_installers_test.go +++ b/server/datastore/mysql/software_installers_test.go @@ -61,6 +61,7 @@ func TestSoftwareInstallers(t *testing.T) { {"CustomToFMAInstallerReplacement", testCustomToFMAInstallerReplacement}, {"GetInstallerByTeamAndURL", testGetInstallerByTeamAndURL}, {"BatchSetFMACancelsPendingOnActiveRow", testBatchSetFMACancelsPendingOnActiveRow}, + {"MatchOrCreateSoftwareInstallerDuplicateConflicts", testMatchOrCreateSoftwareInstallerDuplicateConflicts}, } for _, c := range cases { @@ -5048,3 +5049,252 @@ func testBatchSetFMACancelsPendingOnActiveRow(t *testing.T, ds *Datastore) { }) require.Zero(t, pending, "re-submitting the active FMA version must cancel its pending installs") } + +func testMatchOrCreateSoftwareInstallerDuplicateConflicts(t *testing.T, ds *Datastore) { + ctx := context.Background() + + user := test.NewUser(t, ds, "Alice", "alice@example.com", true) + team, err := ds.NewTeam(ctx, &fleet.Team{Name: t.Name()}) + require.NoError(t, err) + + const conflictMsg = "already has an installer available for" + + // macOS installer conflicting with a VPP app on the same bundle id. + test.CreateInsertGlobalVPPToken(t, ds) + _, err = ds.InsertVPPAppWithTeam(ctx, &fleet.VPPApp{ + VPPAppTeam: fleet.VPPAppTeam{VPPAppID: fleet.VPPAppID{AdamID: "adam_vpp_mac", Platform: fleet.MacOSPlatform}}, + Name: "Mac VPP", + BundleIdentifier: "com.example.vpp", + }, &team.ID) + require.NoError(t, err) + + _, _, err = ds.MatchOrCreateSoftwareInstaller(ctx, &fleet.UploadSoftwareInstallerPayload{ + StorageID: "mac-vpp-clash-storage", + Filename: "vpp-clash.pkg", + Title: "Mac VPP Clash", + BundleIdentifier: "com.example.vpp", + Extension: "pkg", + Source: "apps", + Platform: "darwin", + Version: "1.0", + UserID: user.ID, + ValidatedLabels: &fleet.LabelIdentsWithScope{}, + TeamID: &team.ID, + }) + require.ErrorContains(t, err, conflictMsg) + + // macOS installer sharing a bundle id with an in-house app is allowed: + // in-house apps only target iOS/iPadOS, so they don't conflict with macOS. + _, _, err = ds.MatchOrCreateSoftwareInstaller(ctx, &fleet.UploadSoftwareInstallerPayload{ + StorageID: "iha-storage", + Filename: "iha.ipa", + Title: "iOS App", + BundleIdentifier: "com.example.iha", + Extension: "ipa", + Source: "ios_apps", + Platform: "ios", + Version: "1.0", + UserID: user.ID, + ValidatedLabels: &fleet.LabelIdentsWithScope{}, + TeamID: &team.ID, + }) + require.NoError(t, err) + + _, _, err = ds.MatchOrCreateSoftwareInstaller(ctx, &fleet.UploadSoftwareInstallerPayload{ + StorageID: "mac-iha-coexist-storage", + Filename: "mac-iha-coexist.pkg", + Title: "Mac IHA Coexist", + BundleIdentifier: "com.example.iha", + Extension: "pkg", + Source: "apps", + Platform: "darwin", + Version: "1.0", + UserID: user.ID, + ValidatedLabels: &fleet.LabelIdentsWithScope{}, + TeamID: &team.ID, + }) + require.NoError(t, err) + + // macOS installer conflicting with the same installer at a newer version. + _, _, err = ds.MatchOrCreateSoftwareInstaller(ctx, &fleet.UploadSoftwareInstallerPayload{ + StorageID: "mac-base-storage", + Filename: "mac-app.pkg", + Title: "Mac App", + BundleIdentifier: "com.example.mac", + Extension: "pkg", + Source: "apps", + Platform: "darwin", + Version: "1.0", + UserID: user.ID, + ValidatedLabels: &fleet.LabelIdentsWithScope{}, + TeamID: &team.ID, + }) + require.NoError(t, err) + + _, _, err = ds.MatchOrCreateSoftwareInstaller(ctx, &fleet.UploadSoftwareInstallerPayload{ + StorageID: "mac-v2-storage", + Filename: "mac-app-v2.pkg", + Title: "Mac App", + BundleIdentifier: "com.example.mac", + Extension: "pkg", + Source: "apps", + Platform: "darwin", + Version: "2.0", + UserID: user.ID, + ValidatedLabels: &fleet.LabelIdentsWithScope{}, + TeamID: &team.ID, + }) + require.ErrorContains(t, err, conflictMsg) + + // Windows installer conflicting with the same Title at a newer version. + _, _, err = ds.MatchOrCreateSoftwareInstaller(ctx, &fleet.UploadSoftwareInstallerPayload{ + StorageID: "win-base-storage", + Filename: "win-app.msi", + Title: "Win App", + Extension: "msi", + Source: "programs", + Platform: "windows", + Version: "1.0", + UserID: user.ID, + ValidatedLabels: &fleet.LabelIdentsWithScope{}, + TeamID: &team.ID, + }) + require.NoError(t, err) + + _, _, err = ds.MatchOrCreateSoftwareInstaller(ctx, &fleet.UploadSoftwareInstallerPayload{ + StorageID: "win-v2-storage", + Filename: "win-app-v2.msi", + Title: "Win App", + Extension: "msi", + Source: "programs", + Platform: "windows", + Version: "2.0", + UserID: user.ID, + ValidatedLabels: &fleet.LabelIdentsWithScope{}, + TeamID: &team.ID, + }) + require.ErrorContains(t, err, conflictMsg) + + // Windows installer conflicting on the upgrade code with a different Title. + const winUpgradeCode = "{ABCDEF12-3456-7890-ABCD-EF1234567890}" + _, _, err = ds.MatchOrCreateSoftwareInstaller(ctx, &fleet.UploadSoftwareInstallerPayload{ + StorageID: "win-uc-base-storage", + Filename: "win-uc.msi", + Title: "Win UC App", + Extension: "msi", + Source: "programs", + Platform: "windows", + Version: "1.0", + UpgradeCode: winUpgradeCode, + UserID: user.ID, + ValidatedLabels: &fleet.LabelIdentsWithScope{}, + TeamID: &team.ID, + }) + require.NoError(t, err) + + _, _, err = ds.MatchOrCreateSoftwareInstaller(ctx, &fleet.UploadSoftwareInstallerPayload{ + StorageID: "win-uc-v2-storage", + Filename: "win-uc-other.msi", + Title: "Win UC App Renamed", + Extension: "msi", + Source: "programs", + Platform: "windows", + Version: "2.0", + UpgradeCode: winUpgradeCode, + UserID: user.ID, + ValidatedLabels: &fleet.LabelIdentsWithScope{}, + TeamID: &team.ID, + }) + require.ErrorContains(t, err, conflictMsg) + + // Windows: existing installer has an upgrade code, new upload has the same + // Title but no upgrade code. + _, _, err = ds.MatchOrCreateSoftwareInstaller(ctx, &fleet.UploadSoftwareInstallerPayload{ + StorageID: "win-uc-existing-storage", + Filename: "win-uc-existing.msi", + Title: "Win UC Same Name", + Extension: "msi", + Source: "programs", + Platform: "windows", + Version: "1.0", + UpgradeCode: "{11111111-1111-1111-1111-111111111111}", + UserID: user.ID, + ValidatedLabels: &fleet.LabelIdentsWithScope{}, + TeamID: &team.ID, + }) + require.NoError(t, err) + + _, _, err = ds.MatchOrCreateSoftwareInstaller(ctx, &fleet.UploadSoftwareInstallerPayload{ + StorageID: "win-uc-noupgrade-storage", + Filename: "win-uc-custom.msi", + Title: "Win UC Same Name", + Extension: "msi", + Source: "programs", + Platform: "windows", + Version: "2.0", + UserID: user.ID, + ValidatedLabels: &fleet.LabelIdentsWithScope{}, + TeamID: &team.ID, + }) + require.ErrorContains(t, err, conflictMsg) + + // Reverse: existing installer has no upgrade code, new upload has the same + // Title with an upgrade code. + _, _, err = ds.MatchOrCreateSoftwareInstaller(ctx, &fleet.UploadSoftwareInstallerPayload{ + StorageID: "win-plain-base-storage", + Filename: "win-plain.msi", + Title: "Win Plain App", + Extension: "msi", + Source: "programs", + Platform: "windows", + Version: "1.0", + UserID: user.ID, + ValidatedLabels: &fleet.LabelIdentsWithScope{}, + TeamID: &team.ID, + }) + require.NoError(t, err) + + _, _, err = ds.MatchOrCreateSoftwareInstaller(ctx, &fleet.UploadSoftwareInstallerPayload{ + StorageID: "win-plain-uc-storage", + Filename: "win-plain-uc.msi", + Title: "Win Plain App", + Extension: "msi", + Source: "programs", + Platform: "windows", + Version: "2.0", + UpgradeCode: "{22222222-2222-2222-2222-222222222222}", + UserID: user.ID, + ValidatedLabels: &fleet.LabelIdentsWithScope{}, + TeamID: &team.ID, + }) + require.ErrorContains(t, err, conflictMsg) + + // Linux installer conflicting with the same Title at a newer version. + _, _, err = ds.MatchOrCreateSoftwareInstaller(ctx, &fleet.UploadSoftwareInstallerPayload{ + StorageID: "linux-base-storage", + Filename: "linux-app.deb", + Title: "Linux App", + Extension: "deb", + Source: "deb_packages", + Platform: "linux", + Version: "1.0", + UserID: user.ID, + ValidatedLabels: &fleet.LabelIdentsWithScope{}, + TeamID: &team.ID, + }) + require.NoError(t, err) + + _, _, err = ds.MatchOrCreateSoftwareInstaller(ctx, &fleet.UploadSoftwareInstallerPayload{ + StorageID: "linux-v2-storage", + Filename: "linux-app-v2.deb", + Title: "Linux App", + Extension: "deb", + Source: "deb_packages", + Platform: "linux", + Version: "2.0", + UserID: user.ID, + ValidatedLabels: &fleet.LabelIdentsWithScope{}, + TeamID: &team.ID, + }) + require.ErrorContains(t, err, conflictMsg) +} diff --git a/server/service/integration_enterprise_test.go b/server/service/integration_enterprise_test.go index 47fe136c4c..5dc0ae1db1 100644 --- a/server/service/integration_enterprise_test.go +++ b/server/service/integration_enterprise_test.go @@ -12633,7 +12633,7 @@ func (s *integrationEnterpriseTestSuite) TestSoftwareInstallerUploadDownloadAndD s.lastActivityMatches(fleet.ActivityTypeAddedSoftware{}.ActivityName(), activityData, 0) // upload again fails - s.uploadSoftwareInstaller(t, payload, http.StatusConflict, "already exists") + s.uploadSoftwareInstaller(t, payload, http.StatusConflict, "already has an installer available") // update should succeed s.updateSoftwareInstaller(t, &fleet.UpdateSoftwareInstallerPayload{ @@ -12838,7 +12838,7 @@ func (s *integrationEnterpriseTestSuite) TestSoftwareInstallerUploadDownloadAndD s.lastActivityOfTypeMatches(fleet.ActivityTypeAddedSoftware{}.ActivityName(), activityData, 0) // upload again fails - s.uploadSoftwareInstaller(t, payload, http.StatusConflict, "already exists") + s.uploadSoftwareInstaller(t, payload, http.StatusConflict, "already has an installer available") // download the installer r := s.Do("GET", fmt.Sprintf("/api/latest/fleet/software/titles/%d/package?alt=media", titleID), nil, http.StatusOK, "team_id", fmt.Sprintf("%d", *payload.TeamID)) @@ -12951,7 +12951,7 @@ func (s *integrationEnterpriseTestSuite) TestSoftwareInstallerUploadDownloadAndD fmt.Sprintf(`{"software_title": "ruby", "software_package": "ruby.deb", "team_name": null, "team_id": 0, "fleet_name": null, "fleet_id": 0, "self_service": true, "software_title_id": %d}`, titleID), 0) // upload again fails - s.uploadSoftwareInstaller(t, payload, http.StatusConflict, "already exists") + s.uploadSoftwareInstaller(t, payload, http.StatusConflict, "already has an installer available") // download the installer r := s.Do("GET", fmt.Sprintf("/api/latest/fleet/software/titles/%d/package?alt=media", titleID), nil, http.StatusOK, "team_id", fmt.Sprintf("%d", 0))