improve profile validation to detect invalid XML (#15365)

for #15361
This commit is contained in:
Roberto Dip 2023-11-29 18:13:39 -03:00 committed by GitHub
parent e3274b9a77
commit 2c337a4f17
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
2 changed files with 293 additions and 89 deletions

View file

@ -2,12 +2,12 @@ package fleet
import (
"bytes"
"encoding/xml"
"errors"
"fmt"
"strings"
"time"
"github.com/beevik/etree"
"github.com/fleetdm/fleet/v4/server/mdm"
microsoft_mdm "github.com/fleetdm/fleet/v4/server/mdm/microsoft"
)
@ -45,38 +45,50 @@ type MDMWindowsConfigProfile struct {
//
// Returns an error if these conditions are not met.
func (m *MDMWindowsConfigProfile) ValidateUserProvided() error {
if len(bytes.TrimSpace(m.SyncML)) == 0 {
return errors.New("The file should include valid XML.")
}
if _, ok := microsoft_mdm.FleetReservedProfileNames()[m.Name]; ok {
return fmt.Errorf("Profile name %q is not allowed.", m.Name)
}
if mdm.GetRawProfilePlatform(m.SyncML) != "windows" {
// it doesn't start with <Replace>, check if it is still valid XML.
if len(bytes.TrimSpace(m.SyncML)) == 0 {
return errors.New("The file should include valid XML.")
}
doc := etree.NewDocument()
if err := doc.ReadFromBytes(m.SyncML); err != nil {
return fmt.Errorf("The file should include valid XML: %w", err)
}
// valid xml, but does not start with <Replace>
return errors.New("Only <Replace> supported as a top level element. Make sure you don't have other top level elements.")
var validator struct {
SyncBody
NonProtocolElements []interface{} `xml:",any,omitempty"`
}
doc := etree.NewDocument()
if err := doc.ReadFromBytes(m.SyncML); err != nil {
wrappedProfile := fmt.Sprintf("<SyncBody>%s</SyncBody>", m.SyncML)
if err := xml.Unmarshal([]byte(wrappedProfile), &validator); err != nil {
return fmt.Errorf("The file should include valid XML: %w", err)
}
for _, element := range doc.ChildElements() {
if element.Tag != CmdReplace {
return errors.New("Only <Replace> supported as a top level element. Make sure you don't have other top level elements.")
}
// might be valid XML, but start with something other than <Replace>
if mdm.GetRawProfilePlatform(m.SyncML) != "windows" {
return errors.New("Only <Replace> supported as a top level element. Make sure you don't have other top level elements.")
}
for _, locURI := range element.FindElements("//Item/Target/LocURI") {
if locURI != nil {
if err := validateFleetProvidedLocURI(locURI.Text()); err != nil {
return err
}
if len(validator.Add) != 0 ||
len(validator.Alert) != 0 ||
len(validator.Atomic) != 0 ||
len(validator.Delete) != 0 ||
len(validator.Exec) != 0 ||
len(validator.Get) != 0 ||
len(validator.Results) != 0 ||
len(validator.Status) != 0 ||
len(validator.NonProtocolElements) != 0 {
return errors.New("Only <Replace> supported as a top level element. Make sure you don't have other top level elements.")
}
for _, cmd := range validator.Replace {
for _, item := range cmd.Items {
// intentionally skipping any further validation if we
// don't get a target per product decision.
if item.Target == nil {
continue
}
if err := validateFleetProvidedLocURI(*item.Target); err != nil {
return err
}
}
}

View file

@ -17,12 +17,12 @@ func TestValidateUserProvided(t *testing.T) {
name: "Valid XML with Replace",
profile: MDMWindowsConfigProfile{
SyncML: []byte(`
<Replace>
<Item>
<Target><LocURI>Custom/URI</LocURI></Target>
</Item>
</Replace>
`),
<Replace>
<Item>
<Target><LocURI>Custom/URI</LocURI></Target>
</Item>
</Replace>
`),
},
wantErr: false,
},
@ -30,14 +30,14 @@ func TestValidateUserProvided(t *testing.T) {
name: "Invalid Platform",
profile: MDMWindowsConfigProfile{
SyncML: []byte(`
<SyncML xmlns="SYNCML:SYNCML1.2">
<Replace>
<Item>
<Target><LocURI>Custom/URI</LocURI></Target>
</Item>
</Replace>
</SyncML>
`),
<SyncML xmlns="SYNCML:SYNCML1.2">
<Replace>
<Item>
<Target><LocURI>Custom/URI</LocURI></Target>
</Item>
</Replace>
</SyncML>
`),
},
wantErr: true,
},
@ -45,12 +45,12 @@ func TestValidateUserProvided(t *testing.T) {
name: "Invalid XML Structure",
profile: MDMWindowsConfigProfile{
SyncML: []byte(`
<Add>
<Item>
<Target><LocURI>Custom/URI</LocURI></Target>
</Item>
</Add>
`),
<Add>
<Item>
<Target><LocURI>Custom/URI</LocURI></Target>
</Item>
</Add>
`),
},
wantErr: true,
},
@ -58,12 +58,12 @@ func TestValidateUserProvided(t *testing.T) {
name: "Reserved LocURI",
profile: MDMWindowsConfigProfile{
SyncML: []byte(`
<Replace>
<Item>
<Target><LocURI>./Device/Vendor/MSFT/BitLocker/Foo</LocURI></Target>
</Item>
</Replace>
`),
<Replace>
<Item>
<Target><LocURI>./Device/Vendor/MSFT/BitLocker/Foo</LocURI></Target>
</Item>
</Replace>
`),
},
wantErr: true,
},
@ -71,12 +71,12 @@ func TestValidateUserProvided(t *testing.T) {
name: "Reserved LocURI with implicit ./Device prefix",
profile: MDMWindowsConfigProfile{
SyncML: []byte(`
<Replace>
<Item>
<Target><LocURI>./Vendor/MSFT/BitLocker/Foo</LocURI></Target>
</Item>
</Replace>
`),
<Replace>
<Item>
<Target><LocURI>./Vendor/MSFT/BitLocker/Foo</LocURI></Target>
</Item>
</Replace>
`),
},
wantErr: true,
},
@ -84,17 +84,17 @@ func TestValidateUserProvided(t *testing.T) {
name: "XML with Multiple Replace Elements",
profile: MDMWindowsConfigProfile{
SyncML: []byte(`
<Replace>
<Item>
<Target><LocURI>Custom/URI1</LocURI></Target>
</Item>
</Replace>
<Replace>
<Item>
<Target><LocURI>Custom/URI2</LocURI></Target>
</Item>
</Replace>
`),
<Replace>
<Item>
<Target><LocURI>Custom/URI1</LocURI></Target>
</Item>
</Replace>
<Replace>
<Item>
<Target><LocURI>Custom/URI2</LocURI></Target>
</Item>
</Replace>
`),
},
wantErr: false,
},
@ -109,17 +109,17 @@ func TestValidateUserProvided(t *testing.T) {
name: "XML with Multiple Replace Elements, One with Reserved LocURI",
profile: MDMWindowsConfigProfile{
SyncML: []byte(`
<Replace>
<Item>
<Target><LocURI>Custom/URI</LocURI></Target>
</Item>
</Replace>
<Replace>
<Item>
<Target><LocURI>./Device/Vendor/MSFT/BitLocker/Bar</LocURI></Target>
</Item>
</Replace>
`),
<Replace>
<Item>
<Target><LocURI>Custom/URI</LocURI></Target>
</Item>
</Replace>
<Replace>
<Item>
<Target><LocURI>./Device/Vendor/MSFT/BitLocker/Bar</LocURI></Target>
</Item>
</Replace>
`),
},
wantErr: true,
},
@ -127,20 +127,212 @@ func TestValidateUserProvided(t *testing.T) {
name: "XML with Mixed Replace and Add",
profile: MDMWindowsConfigProfile{
SyncML: []byte(`
<Replace>
<Item>
<Target><LocURI>Custom/URI</LocURI></Target>
</Item>
</Replace>
<Add>
<Item>
<Target><LocURI>Another/URI</LocURI></Target>
</Item>
</Add>
`),
<Replace>
<Item>
<Target><LocURI>Custom/URI</LocURI></Target>
</Item>
</Replace>
<Add>
<Item>
<Target><LocURI>Another/URI</LocURI></Target>
</Item>
</Add>
`),
},
wantErr: true,
},
{
name: "XML with Replace and Alert",
profile: MDMWindowsConfigProfile{
SyncML: []byte(`
<Replace>
<Item>
<Target><LocURI>Replace/URI</LocURI></Target>
</Item>
</Replace>
<Alert>
<Item>
<Target><LocURI>Alert/URI</LocURI></Target>
</Item>
</Alert>
`),
},
wantErr: true,
},
{
name: "XML with Replace and Atomic",
profile: MDMWindowsConfigProfile{
SyncML: []byte(`
<Replace>
<Item>
<Target><LocURI>Replace/URI</LocURI></Target>
</Item>
</Replace>
<Atomic>
<Item>
<Target><LocURI>Atomic/URI</LocURI></Target>
</Item>
</Atomic>
`),
},
wantErr: true,
},
{
name: "XML with Replace and Delete",
profile: MDMWindowsConfigProfile{
SyncML: []byte(`
<Replace>
<Item>
<Target><LocURI>Replace/URI</LocURI></Target>
</Item>
</Replace>
<Delete>
<Item>
<Target><LocURI>Delete/URI</LocURI></Target>
</Item>
</Delete>
`),
},
wantErr: true,
},
{
name: "XML with Replace and Exec",
profile: MDMWindowsConfigProfile{
SyncML: []byte(`
<Replace>
<Item>
<Target><LocURI>Replace/URI</LocURI></Target>
</Item>
</Replace>
<Exec>
<Item>
<Target><LocURI>Exec/URI</LocURI></Target>
</Item>
</Exec>
`),
},
wantErr: true,
},
{
name: "XML with Replace and Get",
profile: MDMWindowsConfigProfile{
SyncML: []byte(`
<Replace>
<Item>
<Target><LocURI>Replace/URI</LocURI></Target>
</Item>
</Replace>
<Get>
<Item>
<Target><LocURI>Get/URI</LocURI></Target>
</Item>
</Get>
`),
},
wantErr: true,
},
{
name: "XML with Replace and Results",
profile: MDMWindowsConfigProfile{
SyncML: []byte(`
<Replace>
<Item>
<Target><LocURI>Replace/URI</LocURI></Target>
</Item>
</Replace>
<Results>
<Item>
<Target><LocURI>Results/URI</LocURI></Target>
</Item>
</Results>
`),
},
wantErr: true,
},
{
name: "XML with Replace and Status",
profile: MDMWindowsConfigProfile{
SyncML: []byte(`
<Replace>
<Item>
<Target><LocURI>Replace/URI</LocURI></Target>
</Item>
</Replace>
<Status>
<Item>
<Target><LocURI>Status/URI</LocURI></Target>
</Item>
</Status>
`),
},
wantErr: true,
},
{
name: "XML with elements not defined in the protocol",
profile: MDMWindowsConfigProfile{
SyncML: []byte(`
<Replace>
<Item>
<Target><LocURI>Custom/URI</LocURI></Target>
</Item>
</Replace>
<Foo>
<Item>
<Target><LocURI>Another/URI</LocURI></Target>
</Item>
</Foo>
`),
},
wantErr: true,
},
{
name: "invalid XML",
profile: MDMWindowsConfigProfile{
SyncML: []byte(`
<Replace>
<Item>
<Target><LocURI>Custom/URI</LocURI></Target>
</Item>
</Add>
`),
},
wantErr: true,
},
{
name: "empty LocURI",
profile: MDMWindowsConfigProfile{
SyncML: []byte(`
<Replace>
<Item>
<Target><LocURI></LocURI></Target>
</Item>
</Replace>
`),
},
wantErr: false,
},
{
name: "item without target",
profile: MDMWindowsConfigProfile{
SyncML: []byte(`
<Replace>
<Item>
</Item>
</Replace>
`),
},
wantErr: false,
},
{
name: "no items in Replace",
profile: MDMWindowsConfigProfile{
SyncML: []byte(`
<Replace>
</Replace>
`),
},
wantErr: false,
},
{
name: "Valid XML with reserved name",
profile: MDMWindowsConfigProfile{