mirror of
https://github.com/netbirdio/netbird.git
synced 2026-10-26 14:29:08 +01:00
[client] Address review: redact and clear the MDM upload URL
- Never log the MDM-provided upload URL (it can embed credentials or signed query tokens): mark the key secret so it is redacted, and drop the raw value from the invalid-URL warning. - Clear DebugBundleUploadURL when a replacement policy no longer carries the key, so a removed override can never keep directing uploads to a previously-enforced host; covered by a policy-replacement test. - macOS docs: state "https URL with a host" consistently, and make the managed-plist helper fail closed on an invalid allowRemoteJobs value (emit false rather than dropping the key).
This commit is contained in:
@@ -770,13 +770,18 @@ func (config *Config) applyMDMPolicy(policy *mdm.Policy) {
|
|||||||
if v, ok := policy.GetString(mdm.KeyBundleUploadURL); ok {
|
if v, ok := policy.GetString(mdm.KeyBundleUploadURL); ok {
|
||||||
// Must be a well-formed https URL with a host, matching the client's
|
// Must be a well-formed https URL with a host, matching the client's
|
||||||
// remote-job upload-URL validation. Invalid values are skipped so a
|
// remote-job upload-URL validation. Invalid values are skipped so a
|
||||||
// bad policy cannot break bundle uploads.
|
// bad policy cannot break bundle uploads. The URL is not logged: it
|
||||||
|
// can embed credentials or signed query tokens.
|
||||||
if u, err := url.Parse(v); err != nil || u.Scheme != "https" || u.Host == "" {
|
if u, err := url.Parse(v); err != nil || u.Scheme != "https" || u.Host == "" {
|
||||||
log.Warnf("MDM debug bundle upload URL %q invalid (must be an https URL with a host); keeping previous value", v)
|
log.Warnf("MDM debug bundle upload URL is invalid (must be an https URL with a host); keeping previous value")
|
||||||
} else {
|
} else {
|
||||||
config.DebugBundleUploadURL = v
|
config.DebugBundleUploadURL = v
|
||||||
logApplied(mdm.KeyBundleUploadURL, v)
|
logApplied(mdm.KeyBundleUploadURL, "")
|
||||||
}
|
}
|
||||||
|
} else {
|
||||||
|
// The key was dropped from the policy: clear any stale override so it
|
||||||
|
// can never keep directing uploads to a previously-enforced host.
|
||||||
|
config.DebugBundleUploadURL = ""
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -324,6 +324,14 @@ func TestApplyMDMPolicyRemoteJobs(t *testing.T) {
|
|||||||
}))
|
}))
|
||||||
assert.Empty(t, cfg.DebugBundleUploadURL, "a non-https upload URL must be skipped")
|
assert.Empty(t, cfg.DebugBundleUploadURL, "a non-https upload URL must be skipped")
|
||||||
})
|
})
|
||||||
|
|
||||||
|
t.Run("dropping the key clears a previously-applied override", func(t *testing.T) {
|
||||||
|
cfg := &Config{DebugBundleUploadURL: "https://old.example.com"}
|
||||||
|
// A replacement policy that no longer carries the key must not leave
|
||||||
|
// the old upload target directing bundles.
|
||||||
|
cfg.applyMDMPolicy(mdm.NewPolicy(map[string]any{mdm.KeyRemoteJobsAllowed: true}))
|
||||||
|
assert.Empty(t, cfg.DebugBundleUploadURL, "the stale upload URL override must be cleared")
|
||||||
|
})
|
||||||
}
|
}
|
||||||
|
|
||||||
func TestUpdateOldManagementURL(t *testing.T) {
|
func TestUpdateOldManagementURL(t *testing.T) {
|
||||||
|
|||||||
@@ -82,6 +82,8 @@ const (
|
|||||||
// SecretKeys lists keys whose values must be redacted in logs.
|
// SecretKeys lists keys whose values must be redacted in logs.
|
||||||
var SecretKeys = map[string]struct{}{
|
var SecretKeys = map[string]struct{}{
|
||||||
KeyPreSharedKey: {},
|
KeyPreSharedKey: {},
|
||||||
|
// The upload URL can embed credentials or signed query tokens.
|
||||||
|
KeyBundleUploadURL: {},
|
||||||
}
|
}
|
||||||
|
|
||||||
// boolStringLiterals enumerates the textual boolean encodings the
|
// boolStringLiterals enumerates the textual boolean encodings the
|
||||||
|
|||||||
@@ -125,8 +125,8 @@
|
|||||||
allowRemoteJobs : opt into management-requested
|
allowRemoteJobs : opt into management-requested
|
||||||
remote jobs. Off by default.
|
remote jobs. Off by default.
|
||||||
debugBundleUploadURL : override the debug-bundle upload
|
debugBundleUploadURL : override the debug-bundle upload
|
||||||
service (https URL); precedence
|
service (https URL with a host);
|
||||||
over the management value. -->
|
precedence over the management value. -->
|
||||||
<!--
|
<!--
|
||||||
<key>allowRemoteJobs</key>
|
<key>allowRemoteJobs</key>
|
||||||
<true/>
|
<true/>
|
||||||
|
|||||||
@@ -57,7 +57,7 @@ managementURL='https://api.netbird.io:443'
|
|||||||
preSharedKey="$NULL" # secret; redacted in log
|
preSharedKey="$NULL" # secret; redacted in log
|
||||||
allowServerSSH='true'
|
allowServerSSH='true'
|
||||||
allowRemoteJobs="$NULL"
|
allowRemoteJobs="$NULL"
|
||||||
debugBundleUploadURL="$NULL" # https URL; overrides management
|
debugBundleUploadURL="$NULL" # HTTPS URL with a host; overrides management
|
||||||
blockInbound="$NULL"
|
blockInbound="$NULL"
|
||||||
disableAutoConnect="$NULL"
|
disableAutoConnect="$NULL"
|
||||||
disableAutostart="$NULL"
|
disableAutostart="$NULL"
|
||||||
@@ -130,7 +130,7 @@ emit_bool() {
|
|||||||
case "$value" in
|
case "$value" in
|
||||||
true|True|TRUE|1|yes) xml_bool='<true/>' ; value='true' ;;
|
true|True|TRUE|1|yes) xml_bool='<true/>' ; value='true' ;;
|
||||||
false|False|FALSE|0|no) xml_bool='<false/>' ; value='false' ;;
|
false|False|FALSE|0|no) xml_bool='<false/>' ; value='false' ;;
|
||||||
*) log "invalid boolean for $key: $value (must be true/false); skipping"; return ;;
|
*) log "invalid boolean for $key: $value (must be true/false)"; return 1 ;;
|
||||||
esac
|
esac
|
||||||
printf ' <key>%s</key>\n %s\n' "$key" "$xml_bool" >> "$PLIST_PATH.tmp"
|
printf ' <key>%s</key>\n %s\n' "$key" "$xml_bool" >> "$PLIST_PATH.tmp"
|
||||||
log "set $key = $value"
|
log "set $key = $value"
|
||||||
@@ -156,7 +156,11 @@ main() {
|
|||||||
is_set "$managementURL" && emit_string managementURL "$managementURL"
|
is_set "$managementURL" && emit_string managementURL "$managementURL"
|
||||||
is_set "$preSharedKey" && emit_string preSharedKey "$preSharedKey"
|
is_set "$preSharedKey" && emit_string preSharedKey "$preSharedKey"
|
||||||
is_set "$allowServerSSH" && emit_bool allowServerSSH "$allowServerSSH"
|
is_set "$allowServerSSH" && emit_bool allowServerSSH "$allowServerSSH"
|
||||||
is_set "$allowRemoteJobs" && emit_bool allowRemoteJobs "$allowRemoteJobs"
|
# Fail closed: an invalid allowRemoteJobs value must not drop the key and
|
||||||
|
# leave a conflicting local opt-in active — enforce the safe default (false).
|
||||||
|
if is_set "$allowRemoteJobs"; then
|
||||||
|
emit_bool allowRemoteJobs "$allowRemoteJobs" || emit_bool allowRemoteJobs false
|
||||||
|
fi
|
||||||
is_set "$debugBundleUploadURL" && emit_string debugBundleUploadURL "$debugBundleUploadURL"
|
is_set "$debugBundleUploadURL" && emit_string debugBundleUploadURL "$debugBundleUploadURL"
|
||||||
is_set "$blockInbound" && emit_bool blockInbound "$blockInbound"
|
is_set "$blockInbound" && emit_bool blockInbound "$blockInbound"
|
||||||
is_set "$disableAutoConnect" && emit_bool disableAutoConnect "$disableAutoConnect"
|
is_set "$disableAutoConnect" && emit_bool disableAutoConnect "$disableAutoConnect"
|
||||||
|
|||||||
Reference in New Issue
Block a user