Block host enrollment with empty enroll secrets

VerifyEnrollSecret matched by exact string, so an empty enroll_secret
matched any stored empty secret and issued a valid node key. Guard the
shared chokepoint: reject empty/whitespace secrets before matching,
closing all enrollment paths (osquery, Orbit, Apple MDM, Android). Add a
migration to delete pre-existing empty secrets.
This commit is contained in:
Juan Fernandez
2026-07-24 10:29:46 -04:00
committed by GitHub
parent ade803d6eb
commit aeb56916ee
7 changed files with 118 additions and 3 deletions
+1 -1
View File
@@ -1 +1 @@
- Rejected empty and whitespace-only enroll secrets when creating or updating teams.
- Rejected empty and whitespace-only enroll secrets when creating or updating teams, blocked host enrollment with such secrets across all enrollment paths (osquery, Orbit, Apple MDM, Android), and removed any pre-existing empty enroll secrets.
+4
View File
@@ -135,6 +135,10 @@ func (ds *Datastore) SetAndroidEnabledAndConfigured(ctx context.Context, configu
}
func (ds *Datastore) VerifyEnrollSecret(ctx context.Context, secret string) (*fleet.EnrollSecret, error) {
if strings.TrimSpace(secret) == "" {
return nil, ctxerr.Wrap(ctx, notFound("EnrollSecret"), "no matching secret found")
}
var s fleet.EnrollSecret
err := sqlx.GetContext(ctx, ds.reader(ctx), &s, "SELECT team_id FROM enroll_secrets WHERE secret = ?", secret)
if err != nil {
@@ -157,6 +157,19 @@ func testAppConfigEnrollSecrets(t *testing.T, ds *Datastore) {
assert.Error(t, err)
assert.Nil(t, secret)
// An empty or whitespace-only secret is rejected as not-found before
// matching, even when an empty secret exists in storage (e.g. a row created
// before the create/update validation existed).
require.NoError(t, ds.ApplyEnrollSecrets(ctx, &team1.ID, []*fleet.EnrollSecret{{Secret: "", TeamID: &team1.ID}}))
for _, in := range []string{"", " ", "\t\n"} {
secret, err = ds.VerifyEnrollSecret(ctx, in)
require.Error(t, err)
require.True(t, fleet.IsNotFound(err))
require.Nil(t, secret)
}
// remove the empty secret so the rest of the test starts from a clean slate
require.NoError(t, ds.ApplyEnrollSecrets(ctx, &team1.ID, []*fleet.EnrollSecret{}))
err = ds.ApplyEnrollSecrets(ctx, &team1.ID,
[]*fleet.EnrollSecret{
{Secret: "one_secret", TeamID: &team1.ID},
@@ -0,0 +1,31 @@
package tables
import (
"database/sql"
"fmt"
)
func init() {
MigrationClient.AddMigration(Up_20260724134801, Down_20260724134801)
}
func Up_20260724134801(tx *sql.Tx) error {
// Remove empty or whitespace-only enroll secrets. These can never be used
// to enroll a host (the server now rejects blank secrets), so deleting them
// neutralizes any blank secret that predates the create/update validation.
// A team left without any secret simply falls back to the global enroll
// secret for MDM provisioning, matching the "team has no secret" behavior.
//
// Match the same set of secrets the server rejects: Go's strings.TrimSpace
// treats tabs, newlines, and Unicode whitespace as blank, so we can't rely
// on MySQL's TRIM (which only strips ASCII spaces). MySQL 8's ICU-backed
// [[:space:]] class covers the full Unicode whitespace set.
if _, err := tx.Exec(`DELETE FROM enroll_secrets WHERE secret REGEXP '^[[:space:]]*$'`); err != nil {
return fmt.Errorf("deleting empty enroll secrets: %w", err)
}
return nil
}
func Down_20260724134801(tx *sql.Tx) error {
return nil
}
@@ -0,0 +1,44 @@
package tables
import (
"testing"
"github.com/stretchr/testify/require"
)
func TestUp_20260724134801(t *testing.T) {
db := applyUpToPrev(t)
// Start from a clean enroll_secrets table so the assertions below are exact.
_, err := db.Exec(`DELETE FROM enroll_secrets`)
require.NoError(t, err)
// Seed whitespace-only secrets (must be removed) alongside secrets that
// contain real content (must be kept). CHAR(... USING utf8mb4) keeps the
// exact bytes explicit. The secret column is a PADSPACE primary key, so the
// empty string and a spaces-only secret share the same key; ' ' stands in
// for both. The tab/newline/NBSP cases are the ones MySQL's TRIM would miss.
_, err = db.Exec(`
INSERT INTO enroll_secrets (secret, team_id) VALUES
(CHAR(32, 32, 32 USING utf8mb4), NULL), -- spaces only
(CHAR(9 USING utf8mb4), NULL), -- tab only
(CHAR(10 USING utf8mb4), NULL), -- newline only
(CONCAT(CHAR(13 USING utf8mb4), CHAR(10 USING utf8mb4)), NULL), -- CRLF only
(CONCAT(CHAR(9 USING utf8mb4), CHAR(32 USING utf8mb4), CHAR(10 USING utf8mb4)), NULL), -- mixed tab/space/newline
(_utf8mb4 0xC2A0, NULL), -- non-breaking space (Unicode)
('validSecret', NULL), -- kept
('has spaces inside', NULL), -- kept (inner spaces)
(CONCAT('a', CHAR(9 USING utf8mb4), 'b'), NULL) -- kept (tab between content)
`)
require.NoError(t, err)
applyNext(t, db)
var remaining []string
require.NoError(t, db.Select(&remaining, `SELECT secret FROM enroll_secrets`))
require.ElementsMatch(t, []string{
"validSecret",
"has spaces inside",
"a\tb",
}, remaining)
}
File diff suppressed because one or more lines are too long
+23
View File
@@ -9974,6 +9974,29 @@ func (s *integrationTestSuite) TestEnrollOsquery() {
defer hres.Body.Close()
require.NoError(t, json.NewDecoder(hres.Body).Decode(&resp))
require.NotEmpty(t, resp.NodeKey)
// A team may retain an empty enroll secret created before the create/update
// validation existed. Simulate that by writing an empty secret directly via
// the datastore, bypassing the service-layer validation.
ctx := context.Background()
emptyTeam, err := s.ds.NewTeam(ctx, &fleet.Team{Name: t.Name() + "empty"})
require.NoError(t, err)
require.NoError(t, s.ds.ApplyEnrollSecrets(ctx, &emptyTeam.ID, []*fleet.EnrollSecret{{Secret: "", TeamID: &emptyTeam.ID}}))
// Enrolling with an empty or whitespace-only secret must be rejected as
// node_invalid, even though an empty secret exists in storage.
for _, badSecret := range []string{"", " "} {
j, err = json.Marshal(&contract.EnrollOsqueryAgentRequest{
EnrollSecret: badSecret,
HostIdentifier: t.Name() + "empty-host",
})
require.NoError(t, err)
badRes := s.DoRawNoAuth("POST", "/api/osquery/enroll", j, http.StatusUnauthorized)
var body map[string]any
require.NoError(t, json.NewDecoder(badRes.Body).Decode(&body))
badRes.Body.Close()
require.Equal(t, true, body["node_invalid"])
}
}
func (s *integrationTestSuite) TestReenrollHostCleansPolicies() {