feat(xoa): auto open ufw port for livemount (#10468)

only open the specific route from source to xo / xo-proxy

port openned only live in memory : restarting ufw will drop them, thus making all the live-mount disk unavailable

doc updated

@xen-orchestra/mixins/live-mount/_firewall.mjs

@@ -0,0 +1,221 @@
+import { createLogger } from '@xen-orchestra/log'
+import { execFile } from 'node:child_process'
+import { isIPv6 } from 'node:net'
+import { promisify } from 'node:util'
+
+const { debug, warn } = createLogger('xo:mixins:LiveMount')
+
+// every rule is tagged with this prefix and the scope of its process, so the ones a crash left
+// behind can be found again, without touching the ones of another process on the same machine
+const COMMENT_PREFIX = 'xo-live-mount:'
+
+// external command: never let a stuck `iptables` hang a mount
+const EXEC_TIMEOUT = 30e3
+
+// how long `iptables` waits for the xtables lock, held by any other process editing the rules
+const XTABLES_LOCK_WAIT = '5'
+
+const execFileAsync = promisify(execFile)
+const defaultExec = (command, args) => execFileAsync(command, args, { timeout: EXEC_TIMEOUT })
+
+// enabled by default, so it must stay out of the way of the installs it cannot apply to: rather
+// than failing the mounts, no rule is opened, and whatever firewall there is decides
+const NOT_ROOT_RE = /you must be root/i
+// ufw not installed, or not enabled: it deletes its chains when stopped
+const MISSING_CHAIN_RE = /No chain\/target\/match by that name|does not exist/i
+// a rule already gone, dropped by any change made to ufw meanwhile
+const MISSING_RULE_RE = /Bad rule|does a matching rule exist/i
+
+/**
+ * Why the firewall cannot be driven, if that is what `error` says.
+ *
+ * @param {Error & { code?: unknown, stderr?: string }} error - as rejected by `execFile`
+ * @returns {string | undefined}
+ */
+function unavailabilityOf(error) {
+  if (error.code === 'ENOENT') {
+    return 'iptables is not installed'
+  }
+  const stderr = error.stderr ?? ''
+  if (NOT_ROOT_RE.test(stderr)) {
+    return 'not running as root'
+  }
+  if (MISSING_CHAIN_RE.test(stderr)) {
+    return 'ufw is not enabled'
+  }
+  return undefined
+}
+
+/** A failed `iptables` command, telling why when it is not a plain failure. */
+export class FirewallCommandError extends Error {
+  /**
+   * @param {string} message
+   * @param {Error & { code?: unknown, stderr?: string }} cause - as rejected by `execFile`
+   */
+  constructor(message, cause) {
+    super(message, { cause })
+    /** @type {string | undefined} set when it only says there is no ufw to drive */
+    this.unavailable = unavailabilityOf(cause)
+    /** @type {boolean} set when the rule to delete does not exist */
+    this.missingRule = MISSING_RULE_RE.test(cause.stderr ?? '')
+  }
+}
+
+/**
+ * ufw drops anything not explicitly allowed, and the ephemeral port of each target cannot be
+ * declared in advance. Rules are inserted straight into ufw's own chain: a table of our own could
+ * not help, a packet dropped by any chain of the `input` hook is dropped whatever the others say.
+ *
+ * `iptables`, not `ufw allow`: the rule only lives in the kernel, nothing is written to
+ * `/etc/ufw/user.rules`, so a crash cannot leave a permanent hole — at worst one lasting until the
+ * next start, which purges them, or the next reboot.
+ *
+ * Known limit: any change made to ufw (`ufw allow`, `ufw delete`, `ufw reload`, `ufw enable`…)
+ * restores its user chains from `user.rules`, which drops these rules and breaks the mounts alive at
+ * that time. Accepted: XOA and proxies are appliances, their firewall is not meant to be changed by
+ * hand. The mounts made afterwards are not affected.
+ */
+const UFW = {
+  4: { command: 'iptables', chain: 'ufw-user-input' },
+  6: { command: 'ip6tables', chain: 'ufw6-user-input' },
+}
+
+/**
+ * @param {string} scope
+ * @returns {string} the comment prefix of the rules of this scope
+ */
+const commentPrefixOf = scope => `${COMMENT_PREFIX}${scope}:`
+
+/**
+ * @param {string} commentPrefix - see `commentPrefixOf`
+ * @param {{ source: string, port: number, id: string }} rule
+ * @returns {string[]} the rule spec, shared by its insertion and its deletion
+ */
+const ruleSpec = (commentPrefix, { source, port, id }) => [
+  '-s',
+  source,
+  '-p',
+  'tcp',
+  '--dport',
+  String(port),
+  '-m',
+  'comment',
+  '--comment',
+  commentPrefix + id,
+  '-j',
+  'ACCEPT',
+]
+
+// as printed by `iptables -S`, which adds the implicit `-m tcp` and a prefix length to the source:
+// -A ufw-user-input -s 10.1.0.5/32 -p tcp -m tcp --dport 42289 -m comment --comment "xo-live-mount:<scope>:<id>" -j ACCEPT
+const escapeRegExp = string => string.replace(/[.*+?^${}()|[\]\\]/g, '\\$&')
+const listedRuleRe = commentPrefix =>
+  new RegExp(
+    `^-A \\S+ -s (\\S+?)(?:/\\d+)? -p tcp -m tcp --dport (\\d+) -m comment --comment "?${escapeRegExp(commentPrefix)}([^"\\s]+)"? -j ACCEPT$`
+  )
+
+/**
+ * The rules this module inserted for `scope`, from the `iptables -S` output of a chain.
+ *
+ * @param {string} listing
+ * @param {string} scope
+ * @returns {{ source: string, port: number, id: string }[]}
+ */
+export function parseListedRules(listing, scope) {
+  const listedRule = listedRuleRe(commentPrefixOf(scope))
+  const rules = []
+  for (const line of listing.split('\n')) {
+    const match = listedRule.exec(line.trim())
+    if (match !== null) {
+      rules.push({ source: match[1], port: Number(match[2]), id: match[3] })
+    }
+  }
+  return rules
+}
+
+/**
+ * Open the port of each live mount to the host it is attached to, in ufw.
+ *
+ * @param {object} [options]
+ * @param {string} options.scope - the process the rules belong to (e.g. `xo-server`): only its own are purged
+ * @param {(command: string, args: string[]) => Promise<{ stdout: string }>} [options.exec] - injectable for tests only
+ */
+export function createUfwFirewall({ scope, exec = defaultExec }) {
+  const commentPrefix = commentPrefixOf(scope)
+  const familyOf = source => UFW[isIPv6(source) ? 6 : 4]
+
+  /** @throws {FirewallCommandError} */
+  const run = async (command, args) => {
+    try {
+      return await exec(command, ['-w', XTABLES_LOCK_WAIT, ...args])
+    } catch (error) {
+      throw new FirewallCommandError(`${command} ${args.join(' ')} failed`, error)
+    }
+  }
+
+  return {
+    /**
+     * @param {{ source: string, port: number, id: string }} rule - `source` is the address of the host
+     * @returns {Promise<boolean>} whether a rule was opened, and must be closed: not when there is no ufw to drive
+     */
+    async open(rule) {
+      const { command, chain } = familyOf(rule.source)
+      try {
+        // inserted first: nothing below may drop it
+        await run(command, ['-I', chain, ...ruleSpec(commentPrefix, rule)])
+        return true
+      } catch (error) {
+        if (error.unavailable === undefined) {
+          throw error
+        }
+        // not root: ufw may well be enabled, and would then block the host
+        const log = error.unavailable === 'not running as root' ? warn : debug
+        log('no firewall rule opened for this live mount', { reason: error.unavailable, ...rule })
+        return false
+      }
+    },
+
+    /** @param {{ source: string, port: number, id: string }} rule - as passed to a successful `open` */
+    async close(rule) {
+      const { command, chain } = familyOf(rule.source)
+      try {
+        await run(command, ['-D', chain, ...ruleSpec(commentPrefix, rule)])
+      } catch (error) {
+        // a change made to ufw dropped it already: what closing it was for
+        if (error.unavailable === undefined && !error.missingRule) {
+          throw error
+        }
+        // gone while mounted: the host was locked out, which is what broke this mount if it did
+        const log = error.missingRule ? warn : debug
+        log('the firewall rule of this live mount was already gone', { reason: error.unavailable, ...rule })
+      }
+    },
+
+    /**
+     * Remove the rules left behind by a previous process, which died without unmounting.
+     *
+     * @returns {Promise<{ source: string, port: number, id: string }[]>} the removed rules
+     */
+    async purge() {
+      const removed = []
+      for (const { command, chain } of Object.values(UFW)) {
+        let stdout
+        try {
+          ;({ stdout } = await run(command, ['-S', chain]))
+        } catch (error) {
+          if (error.unavailable === undefined) {
+            throw error
+          }
+          // no chain, no rule of ours in it
+          debug('no stale firewall rule to look for', { command, reason: error.unavailable })
+          continue
+        }
+        for (const rule of parseListedRules(stdout, scope)) {
+          await run(command, ['-D', chain, ...ruleSpec(commentPrefix, rule)])
+          removed.push(rule)
+        }
+      }
+      return removed
+    },
+  }
+}

@xen-orchestra/mixins/live-mount/_firewall.test.mjs

@@ -0,0 +1,214 @@
+import assert from 'node:assert/strict'
+import { describe, it } from 'node:test'
+
+import { createUfwFirewall, FirewallCommandError, parseListedRules } from './_firewall.mjs'
+
+const ID = 'a4ab6191df76c942459491122ee61fa7'
+const SCOPE = 'xo-server'
+const RULE_SPEC = [
+  '-p',
+  'tcp',
+  '--dport',
+  '42289',
+  '-m',
+  'comment',
+  '--comment',
+  `xo-live-mount:${SCOPE}:${ID}`,
+  '-j',
+  'ACCEPT',
+]
+
+const makeExec = (listings = {}) => {
+  const calls = []
+  const exec = async (command, args) => {
+    calls.push([command, ...args])
+    if (args.includes('-S')) {
+      const listing = listings[command]
+      if (listing instanceof Error) {
+        throw listing
+      }
+      return { stdout: listing ?? '' }
+    }
+    return { stdout: '' }
+  }
+  return { calls, exec }
+}
+
+describe('createUfwFirewall', () => {
+  it('inserts the rule on top of ufw-user-input, scoped to the host', async () => {
+    const { calls, exec } = makeExec()
+
+    await createUfwFirewall({ scope: SCOPE, exec }).open({ source: '10.1.0.5', port: 42289, id: ID })
+
+    assert.deepEqual(calls, [['iptables', '-w', '5', '-I', 'ufw-user-input', '-s', '10.1.0.5', ...RULE_SPEC]])
+  })
+
+  it('deletes the very same rule', async () => {
+    const { calls, exec } = makeExec()
+
+    await createUfwFirewall({ scope: SCOPE, exec }).close({ source: '10.1.0.5', port: 42289, id: ID })
+
+    assert.deepEqual(calls, [['iptables', '-w', '5', '-D', 'ufw-user-input', '-s', '10.1.0.5', ...RULE_SPEC]])
+  })
+
+  it('uses ip6tables and ufw6-user-input for an IPv6 host', async () => {
+    const { calls, exec } = makeExec()
+
+    await createUfwFirewall({ scope: SCOPE, exec }).open({ source: 'fd00::5', port: 42289, id: ID })
+
+    assert.deepEqual(calls, [['ip6tables', '-w', '5', '-I', 'ufw6-user-input', '-s', 'fd00::5', ...RULE_SPEC]])
+  })
+
+  it('reports the command which failed, with its cause', async () => {
+    const cause = Object.assign(new Error('Command failed'), {
+      code: 4,
+      stderr: 'Another app is currently holding the xtables lock',
+    })
+    const exec = async () => {
+      throw cause
+    }
+
+    await assert.rejects(
+      createUfwFirewall({ scope: SCOPE, exec }).open({ source: '10.1.0.5', port: 42289, id: ID }),
+      error => {
+        assert.ok(error instanceof FirewallCommandError)
+        assert.match(error.message, /^iptables -I ufw-user-input .* failed$/)
+        assert.equal(error.cause, cause)
+        assert.equal(error.unavailable, undefined)
+        assert.equal(error.missingRule, false)
+        return true
+      }
+    )
+  })
+
+  // enabled by default: an install it cannot apply to must still mount
+  for (const [what, failure] of [
+    ['iptables is not installed', Object.assign(new Error('spawn iptables ENOENT'), { code: 'ENOENT' })],
+    [
+      'ufw is not enabled',
+      Object.assign(new Error('Command failed'), {
+        code: 1,
+        stderr: 'iptables: No chain/target/match by that name.\n',
+      }),
+    ],
+    [
+      'not running as root',
+      Object.assign(new Error('Command failed'), {
+        code: 4,
+        stderr:
+          'iptables v1.8.9 (nf_tables): Could not fetch rule set generation id: Permission denied (you must be root)\n',
+      }),
+    ],
+  ]) {
+    it(`opens nothing, without failing, when ${what}`, async () => {
+      const exec = async () => {
+        throw failure
+      }
+      const firewall = createUfwFirewall({ scope: SCOPE, exec })
+
+      assert.equal(await firewall.open({ source: '10.1.0.5', port: 42289, id: ID }), false)
+      await firewall.close({ source: '10.1.0.5', port: 42289, id: ID })
+      assert.deepEqual(await firewall.purge(), [])
+    })
+  }
+
+  it('reports whether a rule was opened', async () => {
+    const { exec } = makeExec()
+
+    assert.equal(
+      await createUfwFirewall({ scope: SCOPE, exec }).open({ source: '10.1.0.5', port: 42289, id: ID }),
+      true
+    )
+  })
+
+  it('considers a rule already dropped, e.g. by a change made to ufw, as closed', async () => {
+    const exec = async () => {
+      throw Object.assign(new Error('Command failed'), {
+        code: 1,
+        stderr: 'iptables: Bad rule (does a matching rule exist in that chain?).\n',
+      })
+    }
+
+    await createUfwFirewall({ scope: SCOPE, exec }).close({ source: '10.1.0.5', port: 42289, id: ID })
+  })
+
+  it('reports any other failure to close a rule', async () => {
+    const cause = Object.assign(new Error('Command failed'), { code: 4, stderr: 'xtables lock timeout' })
+    const exec = async () => {
+      throw cause
+    }
+
+    await assert.rejects(createUfwFirewall({ scope: SCOPE, exec }).close({ source: '10.1.0.5', port: 42289, id: ID }), {
+      cause,
+    })
+  })
+
+  it('purges the families it can, skipping one without ufw', async () => {
+    const calls = []
+    const exec = async (command, args) => {
+      calls.push([command, ...args])
+      if (command === 'ip6tables') {
+        throw Object.assign(new Error('Command failed'), {
+          code: 1,
+          stderr: 'ip6tables: No chain/target/match by that name.\n',
+        })
+      }
+      return {
+        stdout: `-A ufw-user-input -s 10.1.0.5/32 -p tcp -m tcp --dport 42289 -m comment --comment "xo-live-mount:${SCOPE}:${ID}" -j ACCEPT\n`,
+      }
+    }
+
+    assert.deepEqual(await createUfwFirewall({ scope: SCOPE, exec }).purge(), [
+      { source: '10.1.0.5', port: 42289, id: ID },
+    ])
+    assert.deepEqual(
+      calls.map(([command, , , action]) => [command, action]),
+      [
+        ['iptables', '-S'],
+        ['iptables', '-D'],
+        ['ip6tables', '-S'],
+      ]
+    )
+  })
+
+  it('purges only its own rules, not those of another process, in both families', async () => {
+    const { calls, exec } = makeExec({
+      iptables: [
+        '-N ufw-user-input',
+        '-A ufw-user-input -p tcp -m tcp --dport 22 -j ACCEPT',
+        `-A ufw-user-input -s 10.1.0.5/32 -p tcp -m tcp --dport 42289 -m comment --comment "xo-live-mount:${SCOPE}:${ID}" -j ACCEPT`,
+        // another process on the same machine: its mounts are alive
+        `-A ufw-user-input -s 10.1.0.6/32 -p tcp -m tcp --dport 42290 -m comment --comment "xo-live-mount:xo-proxy:${ID}" -j ACCEPT`,
+      ].join('\n'),
+      ip6tables: `-A ufw6-user-input -s fd00::5/128 -p tcp -m tcp --dport 42289 -m comment --comment xo-live-mount:${SCOPE}:${ID} -j ACCEPT\n`,
+    })
+
+    const removed = await createUfwFirewall({ scope: SCOPE, exec }).purge()
+
+    assert.deepEqual(removed, [
+      { source: '10.1.0.5', port: 42289, id: ID },
+      { source: 'fd00::5', port: 42289, id: ID },
+    ])
+    assert.deepEqual(calls, [
+      ['iptables', '-w', '5', '-S', 'ufw-user-input'],
+      ['iptables', '-w', '5', '-D', 'ufw-user-input', '-s', '10.1.0.5', ...RULE_SPEC],
+      ['ip6tables', '-w', '5', '-S', 'ufw6-user-input'],
+      ['ip6tables', '-w', '5', '-D', 'ufw6-user-input', '-s', 'fd00::5', ...RULE_SPEC],
+    ])
+  })
+})
+
+describe('parseListedRules', () => {
+  it('ignores the rules it did not insert', () => {
+    assert.deepEqual(
+      parseListedRules(
+        [
+          '-A ufw-user-input -p tcp -m tcp --dport 443 -j ACCEPT',
+          '-A ufw-user-input -s 10.0.0.0/8 -p tcp -m tcp --dport 3260 -m comment --comment "someone else" -j ACCEPT',
+        ].join('\n'),
+        SCOPE
+      ),
+      []
+    )
+  })
+})

@xen-orchestra/mixins/live-mount/_target.mjs

@@ -48,7 +48,9 @@ export async function probeScsiId({ xapi, hostRef, deviceConfig, address }) {
     throw new Error('SR.probe should have reported the LUN list')
   } catch (error) {
     if (error.code === PROBE_NO_TARGET_ERROR) {
-      const wrapped = new Error(`the host cannot reach the iSCSI target at ${address}, check iscsi.advertisedAddress`)
+      const wrapped = new Error(
+        `the host cannot reach the iSCSI target at ${address}:${deviceConfig.port}, check iscsi.advertisedAddress and the firewalls between them`
+      )
       wrapped.cause = error
       throw wrapped
     }

@xen-orchestra/mixins/live-mount/index.mjs

@@ -8,6 +8,7 @@ import { noSuchObject } from 'xo-common/api-errors.js'
 import { randomBytes } from 'node:crypto'
 
 import { detectLocalAddress } from './_address.mjs'
+import { createUfwFirewall } from './_firewall.mjs'
 import { createChapCredentials, probeScsiId } from './_target.mjs'
 import { forgetSr, introduceSr, introduceVdi } from './_sr.mjs'
 
@@ -30,9 +31,15 @@ const { info, warn } = createLogger('xo:mixins:LiveMount')
  * @property {import('@vates/iscsi').IscsiTarget} target
  * @property {Xapi} xapi - replaced by any newer connection to the same pool, see {@link LiveMount#watchConnection}
  * @property {string} [poolUuid] - pool of `xapi`, unknown if it was not connected
+ * @property {{ source: string, port: number, id: string }} [firewallRule] - opened by `iscsi.manageFirewall`
  * @property {() => Promise<void>} [release]
  */
 
+// the firewalls `iscsi.manageFirewall` can drive
+const FIREWALLS = {
+  ufw: createUfwFirewall,
+}
+
 /**
  * Serve a disk as a read-only iSCSI LUN and attach it, as an SR, to a host —
  * so its content is usable without copying it first.
@@ -72,6 +79,14 @@ export default class LiveMount extends EventEmitter {
   #app
   #createTarget
   #detectAddress
+  #firewall
+  #firewallName
+  // a mount waits for the stale rules to be removed: listed before its own is inserted, its rule
+  // would be removed with them
+  /** @type {Promise<void> | undefined} */
+  #firewallPurge
+  // an invalid `iscsi.manageFirewall` fails the mounts, not the whole process
+  #firewallError
   #openDisk
 
   // mount id -> mount record
@@ -82,14 +97,18 @@ export default class LiveMount extends EventEmitter {
   /** @type {Map<string, string>} */
   #mountIdsByVdiUuid = new Map()
 
-  // `openDisk`/`createTarget`/`detectAddress` are injectable for tests only,
+  // `appName` scopes the firewall rules, so that xo-server and xo-proxy on the same machine do not
+  // purge each other's
+  // `openDisk`/`createTarget`/`detectAddress`/`createFirewall` are injectable for tests only,
   // like xo-server's crypto-credentials mixin does with xenStore/fsPromises
   constructor(
     app,
     {
+      appName,
       openDisk = openDiskChain,
       createTarget = options => new IscsiTarget(options),
       detectAddress = detectLocalAddress,
+      createFirewall = name => FIREWALLS[name]?.({ scope: appName }),
     } = {}
   ) {
     super()
@@ -99,6 +118,25 @@ export default class LiveMount extends EventEmitter {
     this.#detectAddress = detectAddress
     this.#openDisk = openDisk
 
+    // the target of each mount listens on an ephemeral port, which a firewall cannot allow in
+    // advance: when told to, the port is opened to the host of the mount, for its lifetime only
+    // `false` turns off the default of the packaged configuration
+    const configuredFirewall = app.config.getOptional('iscsi.manageFirewall')
+    const firewallName = configuredFirewall === false ? undefined : configuredFirewall
+    const firewall = firewallName === undefined ? undefined : createFirewall(firewallName)
+    if (firewallName !== undefined && firewall === undefined) {
+      this.#firewallError = new Error(
+        `unsupported iscsi.manageFirewall: ${firewallName}, expected one of ${Object.keys(FIREWALLS).join(', ')}`
+      )
+      warn('live mounts are disabled', { error: this.#firewallError })
+    }
+    if (firewall !== undefined) {
+      this.#firewall = firewall
+      this.#firewallName = firewallName
+      // a process which died without unmounting left its rules behind, nothing serves their ports anymore
+      app.hooks.on('start', () => this.#purgeFirewall())
+    }
+
     app.hooks.on('stop', () =>
       asyncEach(
         [...this.#mounts.keys()],
@@ -111,6 +149,27 @@ export default class LiveMount extends EventEmitter {
     )
   }
 
+  /**
+   * Remove the stale rules once, on start or before the first mount, whichever comes first: the
+   * API may be served before the start hooks run.
+   *
+   * @returns {Promise<void>} never rejects: a failure is logged, and does not prevent the mounts
+   */
+  #purgeFirewall() {
+    const firewallName = this.#firewallName
+    this.#firewallPurge ??= (async () => {
+      try {
+        const removed = await this.#firewall.purge()
+        if (removed.length !== 0) {
+          info('removed stale firewall rules', { firewall: firewallName, removed })
+        }
+      } catch (error) {
+        warn('failed to remove stale firewall rules', { error, firewall: firewallName })
+      }
+    })()
+    return this.#firewallPurge
+  }
+
   /**
    * @param {object} params
    * @param {import('@xen-orchestra/fs').RemoteHandlerAbstract} params.handler - passed to `openDisk` to open the source disk
@@ -136,15 +195,20 @@ export default class LiveMount extends EventEmitter {
   }
 
   #createDiskMount = defer(async ($defer, { handler, diskPath, xapi, hostRef, nameLabel, release }) => {
+    if (this.#firewallError !== undefined) {
+      throw this.#firewallError
+    }
     const config = this.#app.config
     // `iscsi.advertisedAddress` overrides auto-detection; unset, the address
     // reachable *from* the target host is guessed by asking the OS which
     // local address it would route through to reach it — usually right, but
     // not guaranteed to be reachable *back* from the host (NAT, asymmetric
     // routing), which is what the override is for.
+    const firewall = this.#firewall
     let address = config.getOptional('iscsi.advertisedAddress')
+    const hostAddress =
+      address === undefined || firewall !== undefined ? await xapi.getField('host', hostRef, 'address') : undefined
     if (address === undefined) {
-      const hostAddress = await xapi.getField('host', hostRef, 'address')
       address = await this.#detectAddress(hostAddress)
     }
 
@@ -171,6 +235,19 @@ export default class LiveMount extends EventEmitter {
     $defer.onFailure(() => target.close())
     const { port } = target.address()
 
+    // scoped to the host's management address: the one the advertised address is detected from,
+    // so the one it connects from — unless `iscsi.advertisedAddress` points at another network
+    let firewallRule
+    if (firewall !== undefined) {
+      await this.#purgeFirewall()
+      const rule = { source: hostAddress, port, id }
+      // none opened when there is no firewall to drive on this install: nothing to close then
+      if (await firewall.open(rule)) {
+        firewallRule = rule
+        $defer.onFailure(() => firewall.close(rule))
+      }
+    }
+
     const deviceConfig = {
       chapuser: chap.user,
       chappassword: chap.secret,
@@ -196,7 +273,22 @@ export default class LiveMount extends EventEmitter {
     info('mounted', { id, address, port, srUuid, vdiUuid, diskPath })
 
     const poolUuid = xapi.pool?.uuid
-    return { address, disk, diskPath, id, iqn, poolUuid, port, release, srRef, srUuid, target, vdiUuid, xapi }
+    return {
+      address,
+      disk,
+      diskPath,
+      firewallRule,
+      id,
+      iqn,
+      poolUuid,
+      port,
+      release,
+      srRef,
+      srUuid,
+      target,
+      vdiUuid,
+      xapi,
+    }
   })
 
   /**
@@ -312,7 +404,7 @@ export default class LiveMount extends EventEmitter {
     // not the deletion this mixin reacts to
     this.#unwatchVdi(mount)
 
-    const { xapi, srRef, target, release } = mount
+    const { xapi, srRef, target, firewallRule, release } = mount
 
     const errors = []
     const step = async (what, fn) => {
@@ -327,6 +419,9 @@ export default class LiveMount extends EventEmitter {
     await step('forget the SR', () => forgetSr(xapi, srRef))
     // stop serving first, so no I/O is left in flight
     await step('close the target', () => target.close())
+    if (firewallRule !== undefined) {
+      await step('close the firewall rule', () => this.#firewall.close(firewallRule))
+    }
     await step('release the caller resources', () => release?.())
 
     // the mount is gone whatever happened above, so callers tracking it must hear about it even

@xen-orchestra/mixins/live-mount/index.test.mjs

@@ -98,9 +98,38 @@ const makeXapi = ({ poolUuid = 'pool-uuid', probeError, vdiSmConfig } = {}) => {
   return xapi
 }
 
-const makeMixin = ({ diskOpenError, listenError, advertisedAddress = '192.168.1.8' } = {}) => {
+// records what the mixin asks of the firewall, instead of running iptables
+const makeFirewall = ({ openError, available = true } = {}) => {
+  const firewall = {
+    calls: [],
+    async open(rule) {
+      firewall.calls.push(['open', rule])
+      if (openError !== undefined) {
+        throw openError
+      }
+      return available
+    },
+    async close(rule) {
+      firewall.calls.push(['close', rule])
+    },
+    async purge() {
+      firewall.calls.push(['purge'])
+      return []
+    },
+  }
+  return firewall
+}
+
+const makeMixin = ({
+  diskOpenError,
+  listenError,
+  advertisedAddress = '192.168.1.8',
+  manageFirewall,
+  firewall = makeFirewall(),
+} = {}) => {
   const hooks = new EventEmitter()
   const detectAddressCalls = []
+  const createFirewallCalls = []
   const app = {
     config: {
       getOptional: path => {
@@ -108,6 +137,9 @@ const makeMixin = ({ diskOpenError, listenError, advertisedAddress = '192.168.1.
           // `null` (as opposed to the default) simulates an unset config key
           return advertisedAddress === null ? undefined : advertisedAddress
         }
+        if (path === 'iscsi.manageFirewall') {
+          return manageFirewall
+        }
         assert.equal(path, 'iscsi.bindAddress')
         return undefined
       },
@@ -136,6 +168,7 @@ const makeMixin = ({ diskOpenError, listenError, advertisedAddress = '192.168.1.
   }
 
   const mixin = new LiveMount(app, {
+    appName: 'xo-server',
     openDisk: async params => {
       if (diskOpenError !== undefined) {
         throw diskOpenError
@@ -151,9 +184,13 @@ const makeMixin = ({ diskOpenError, listenError, advertisedAddress = '192.168.1.
       detectAddressCalls.push(hostAddress)
       return '203.0.113.7'
     },
+    createFirewall: name => {
+      createFirewallCalls.push(name)
+      return name === 'ufw' ? firewall : undefined
+    },
   })
 
-  return { app, detectAddressCalls, disk, hooks, mixin, target }
+  return { app, createFirewallCalls, detectAddressCalls, disk, firewall, hooks, mixin, target }
 }
 
 const mountDisk = (mixin, xapi, params) =>
@@ -246,7 +283,7 @@ describe('mountDisk', () => {
     const { mixin } = makeMixin()
     const xapi = makeXapi({ probeError: new XapiError('SR_BACKEND_FAILURE_141', []) })
 
-    await assert.rejects(mountDisk(mixin, xapi), /cannot reach the iSCSI target at 192\.168\.1\.8/)
+    await assert.rejects(mountDisk(mixin, xapi), /cannot reach the iSCSI target at 192\.168\.1\.8:34567, .* firewalls/)
   })
 
   it('closes the target and the disk when the probe fails', async () => {
@@ -504,6 +541,150 @@ describe('after a reconnection', () => {
   })
 })
 
+describe('iscsi.manageFirewall', () => {
+  it('drives no firewall when unset', async () => {
+    const { mixin, createFirewallCalls, hooks } = makeMixin()
+
+    await mountDisk(mixin, makeXapi())
+
+    assert.deepEqual(createFirewallCalls, [])
+    assert.deepEqual(hooks.listeners('start'), [])
+  })
+
+  it('opens the port of the target to the host, even with an advertised address', async () => {
+    const { mixin, firewall } = makeMixin({ manageFirewall: 'ufw' })
+    const xapi = makeXapi()
+
+    const { id } = await mountDisk(mixin, xapi)
+
+    assert.deepEqual(firewall.calls, [['purge'], ['open', { source: '10.20.30.40', port: 34567, id }]])
+    // opened before the host first connects, which is the probe
+    const probeIndex = xapi.calls.findIndex(([method]) => method === 'SR.probe')
+    const hostAddressIndex = xapi.calls.findIndex(([method, , , field]) => method === 'getField' && field === 'address')
+    assert.ok(hostAddressIndex !== -1 && hostAddressIndex < probeIndex)
+  })
+
+  it('closes the port on unmount, after the target', async () => {
+    const { mixin, firewall, target } = makeMixin({ manageFirewall: 'ufw' })
+    const { id } = await mountDisk(mixin, makeXapi())
+    firewall.calls.length = 0
+    let targetClosedFirst
+    const { close } = firewall
+    firewall.close = async rule => {
+      targetClosedFirst = target.closed
+      return close(rule)
+    }
+
+    await mixin.unmountDisk(id)
+
+    assert.deepEqual(firewall.calls, [['close', { source: '10.20.30.40', port: 34567, id }]])
+    assert.equal(targetClosedFirst, true)
+  })
+
+  it('closes the port when the mount fails afterwards', async () => {
+    const { mixin, firewall, target } = makeMixin({ manageFirewall: 'ufw' })
+
+    await assert.rejects(
+      mountDisk(mixin, makeXapi({ probeError: new XapiError('SR_BACKEND_FAILURE_141', []) })),
+      /cannot reach the iSCSI target/
+    )
+
+    assert.deepEqual(
+      firewall.calls.map(([action]) => action),
+      ['purge', 'open', 'close']
+    )
+    assert.equal(target.closed, true)
+  })
+
+  it('closes the target when the port cannot be opened', async () => {
+    const { mixin, disk, target } = makeMixin({
+      manageFirewall: 'ufw',
+      firewall: makeFirewall({ openError: new Error('iptables failed') }),
+    })
+    const xapi = makeXapi()
+
+    await assert.rejects(mountDisk(mixin, xapi), /iptables failed/)
+
+    assert.equal(target.closed, true)
+    assert.equal(disk.closed, true)
+    assert.ok(!xapi.calls.some(([method]) => method === 'SR.probe'))
+  })
+
+  it('neither records nor closes a rule when there is no firewall to drive', async () => {
+    const { mixin, firewall } = makeMixin({ manageFirewall: 'ufw', firewall: makeFirewall({ available: false }) })
+    const { id } = await mountDisk(mixin, makeXapi())
+
+    await mixin.unmountDisk(id)
+
+    assert.deepEqual(
+      firewall.calls.map(([action]) => action),
+      ['purge', 'open']
+    )
+  })
+
+  it('does not close a rule it could not open when the mount fails', async () => {
+    const { mixin, firewall } = makeMixin({ manageFirewall: 'ufw', firewall: makeFirewall({ available: false }) })
+
+    await assert.rejects(mountDisk(mixin, makeXapi({ probeError: new XapiError('SR_BACKEND_FAILURE_141', []) })))
+
+    assert.deepEqual(
+      firewall.calls.map(([action]) => action),
+      ['purge', 'open']
+    )
+  })
+
+  it('drives no firewall when set to false, overriding the packaged default', async () => {
+    const { mixin, createFirewallCalls, hooks } = makeMixin({ manageFirewall: false })
+
+    await mountDisk(mixin, makeXapi())
+
+    assert.deepEqual(createFirewallCalls, [])
+    assert.deepEqual(hooks.listeners('start'), [])
+  })
+
+  it('removes the stale rules on start', async () => {
+    const { firewall, hooks } = makeMixin({ manageFirewall: 'ufw' })
+
+    await Promise.all(hooks.listeners('start').map(listener => listener()))
+
+    assert.deepEqual(firewall.calls, [['purge']])
+  })
+
+  it('removes the stale rules before the first rule, even when mounting before start, and only once', async () => {
+    const { mixin, firewall, hooks } = makeMixin({ manageFirewall: 'ufw' })
+
+    const { id } = await mountDisk(mixin, makeXapi())
+    await Promise.all(hooks.listeners('start').map(listener => listener()))
+    await mountDisk(mixin, makeXapi())
+
+    assert.deepEqual(
+      firewall.calls.map(([action]) => action),
+      ['purge', 'open', 'open']
+    )
+    assert.equal(firewall.calls[1][1].id, id)
+  })
+
+  it('does not fail the start when the stale rules cannot be removed', async () => {
+    const firewall = makeFirewall()
+    firewall.purge = async () => {
+      throw new Error('ufw is not enabled')
+    }
+    const { hooks } = makeMixin({ manageFirewall: 'ufw', firewall })
+
+    await Promise.all(hooks.listeners('start').map(listener => listener()))
+  })
+
+  it('fails the mounts, not the process, when unsupported', async () => {
+    const { mixin, hooks } = makeMixin({ manageFirewall: 'firewalld' })
+    const xapi = makeXapi()
+
+    await assert.rejects(mountDisk(mixin, xapi), /unsupported iscsi.manageFirewall: firewalld, expected one of ufw/)
+
+    assert.deepEqual(xapi.calls, [])
+    assert.deepEqual(hooks.listeners('start'), [])
+  })
+})
+
 describe('the stop hook', () => {
   it('tears down every live mount', async () => {
     const { mixin, hooks, target } = makeMixin()

@xen-orchestra/proxy/config.toml

@@ -58,6 +58,22 @@ port = 443
 # Address the iSCSI targets bind to. Unset means all interfaces.
 #bindAddress = '0.0.0.0'
 
+# Firewall to open the port of each mount in, to the host it is attached to and
+# for the lifetime of the mount only: every mount listens on its own ephemeral
+# port, which cannot be allowed in advance.
+#
+# Only 'ufw' is supported: rules are inserted in `ufw-user-input` (and
+# `ufw6-user-input`) with `iptables`, so they are never persisted, and the ones a
+# crash left behind are removed at the next start. Any change made to ufw
+# (`ufw allow`, `ufw delete`, `ufw reload`…) drops them, which breaks the mounts
+# alive at that time: the firewall of an appliance is not meant to be changed by
+# hand.
+#
+# Without iptables, without ufw enabled, or when not running as root, no rule
+# is opened and the mounts go on: whatever firewall there is must let the hosts
+# in. Set to `false` to leave the firewall of this proxy alone.
+manageFirewall = 'ufw'
+
 [logs]
 # Display all logs matching this filter, regardless of their level
 #filter = 'xo:backups:*'

CHANGELOG.unreleased.md

@@ -15,6 +15,7 @@
 - [XO6/StateHero] Update StateHero illustrations SVG to match current design system (PR [#10380](https://github.com/vatesfr/xen-orchestra/pull/10380))
 - [i18n] Update Chinese (Simplified Han script), Czech, Dutch, Finnish, Italian, Norwegian, Persian, Portuguese, Russian, Slovak, Spanish and Turkish translations (PR [#10396](https://github.com/vatesfr/xen-orchestra/pull/10396))
 - [Backup/Restore] A live mounted disk is released on its own once it is deleted, or the VM holding it is: its SR is forgotten and the backup is no longer served (PR [#10432](https://github.com/vatesfr/xen-orchestra/pull/10432))
+- [Backup/Restore] When ufw is enabled, as on XOA and proxies, live mount opens its iSCSI port in it, for the host the disk is attached to and while it is mounted (`iscsi.manageFirewall = false` to turn it off) (PR [#10468](https://github.com/vatesfr/xen-orchestra/pull/10468))
 
 ### Bug fixes
 

docs/docs/backups-and-dr/backup-features-and-settings.md

@@ -476,7 +476,7 @@ With a slow or distant BR, typically an S3 or Azure BR outside your network, rea
 - **Incremental backups only.** Live mount is not available for full backups, nor in [backup health checks](#backup-health-check).
 - **Read-only.** The VM sees a read-only disk. It is best suited for data disks: most operating systems need a writable system disk.
 - **One host per restore.** All the disks live mounted by one restore use the same host, and that host must be able to reach the main SR: its own host for a local SR, any host of its pool for a shared SR. The host selector only offers these hosts, and the most likely one is pre-selected.
-- **Network.** The hosts connect to Xen Orchestra (or the proxy) over iSCSI, on a TCP port picked for each mount. A firewall between the hosts and Xen Orchestra must let the hosts open TCP connections to it. The address given to the hosts is auto-detected; if they cannot reach it (NAT, several networks), set it in the [configuration file](../getting-started/configuration.md), for example `/etc/xo-server/config.iscsi.toml`:
+- **Network.** The hosts connect to Xen Orchestra (or the proxy) over iSCSI, on a TCP port picked for each mount. A firewall between the hosts and Xen Orchestra must let the hosts open TCP connections to it. For the firewall of XOA itself, see [Firewall of Xen Orchestra](#live-mount-firewall). The address given to the hosts is auto-detected; if they cannot reach it (NAT, several networks), set it in the [configuration file](../getting-started/configuration.md), for example `/etc/xo-server/config.iscsi.toml`:
 
   ```toml
   [iscsi]
@@ -488,6 +488,50 @@ With a slow or distant BR, typically an S3 or Azure BR outside your network, rea
 
 - **Temporary.** A live mount only lasts as long as the Xen Orchestra process serving it: restarting or updating XOA ends it, and the disk becomes unavailable to the VM. Stop using the disk before restarting or updating XOA. If Xen Orchestra stopped unexpectedly, the `[XO backup] <VM name>` SR stays behind with nothing serving it: detach the disk from the VM and forget the SR.
 
+### Firewall of Xen Orchestra {#live-mount-firewall}
+
+Each live mounted disk is served on its own TCP port, picked when the disk is mounted, so it cannot be allowed in the firewall in advance. So when ufw is enabled on Xen Orchestra, as on XOA, **Xen Orchestra opens these ports in ufw itself, only when needed**. This is enabled by default.
+
+To turn it off, and manage the firewall yourself, add to the [configuration file](../getting-started/configuration.md), for example `/etc/xo-server/config.iscsi.toml`, then restart Xen Orchestra:
+
+```toml
+[iscsi]
+manageFirewall = false
+```
+
+If a firewall still blocks the hosts, the restore fails with `the host cannot reach the iSCSI target at <address>:<port>, check iscsi.advertisedAddress and the firewalls between them`.
+
+How it works:
+
+- **One rule per mount.** When a disk is mounted, Xen Orchestra adds a rule allowing **only the host it is mounted on** to connect, **only on the port of that mount**. The rule is removed when the mount is released, or right away if the mount fails.
+- **Host address.** The rule uses the management address of the host. If `advertisedAddress` points to another network, such as a storage network, the host connects from another address and the rule does not match: allow that network in ufw yourself.
+- **Not saved.** The rules exist only in the running firewall: they are not written to the ufw configuration and do not appear in `ufw status`. To list them:
+
+  ```sh
+  iptables -S ufw-user-input | grep xo-live-mount
+  # hosts with an IPv6 management address
+  ip6tables -S ufw6-user-input | grep xo-live-mount
+  ```
+
+- **Without ufw.** If iptables is not installed, if ufw is not enabled, or if Xen Orchestra does not run as root (XOA and the proxies do), no rule is added and the live mount goes on as usual: any other firewall must let the hosts in. When not running as root, a warning is logged for each mount, since ufw may be enabled and block the hosts.
+- **Only ufw** is supported: any other value than `'ufw'` or `false` makes live mounts fail with `unsupported iscsi.manageFirewall`.
+
+**What happens if ufw is changed** (`ufw allow`, `ufw delete`, `ufw reload`, `ufw disable` then `ufw enable`, restarting the ufw service):
+
+- ufw rebuilds its rules and **drops those added by Xen Orchestra**. XOA and the proxies are appliances: their firewall is not meant to be changed by hand while disks are live mounted.
+- A disk in use keeps working, since ufw still accepts established connections. But the next time the host has to connect again (network outage, host reboot, SR unplugged then plugged again), the connection is blocked and the VM gets I/O errors on that disk.
+- The rules are not added back automatically. To be safe, [release](#release-live-mount) the mounts and live mount the disks again. Releasing a mount whose rule was already dropped works as usual, and logs a warning.
+- Disks live mounted after the change are not affected: their rules are added as usual.
+
+**What happens if Xen Orchestra crashes** (process killed, appliance lost power):
+
+- Its rules can stay in the firewall. Nothing listens on their ports anymore, so a host trying to connect gets refused.
+- They are removed the **next time Xen Orchestra starts**, or when the appliance reboots, whichever comes first. Xen Orchestra and a proxy running on the same machine only remove their own rules.
+- If you set `manageFirewall = false` before restarting, Xen Orchestra no longer removes them. List them with the command above, then delete each one with `iptables -D`, followed by the rule as listed without its leading `-A`.
+- As with any crash, the `[XO backup] <VM name>` SR also stays behind (see **Temporary** above).
+
+A normal stop or restart of Xen Orchestra releases every mount, which removes their rules.
+
 ### Live mount a disk during a restore
 
 1. Go to **Backup → Restore**, and click the **Restore** icon of the VM.

docs/docs/backups-and-dr/scale-and-security/proxy.md

@@ -116,6 +116,7 @@ A disk can be [live mounted](../backup-features-and-settings.md#live-mount) from
 
 - The hosts connect to the proxy over iSCSI, on a TCP port picked for each mount. If a firewall sits between the hosts and the proxy, that firewall must allow TCP connections from the hosts to the proxy.
 - The address provided to the hosts is the proxy's automatically detected address. If the hosts cannot reach it, set the address in the `[iscsi]` section of the proxy configuration (`/etc/xo-proxy/config.toml`), using the same `advertisedAddress` and `bindAddress` settings as those configured for XOA.
+- If ufw is enabled on the proxy, the proxy opens the port of each mount itself, only for the host it is mounted on. This is enabled by default: set `manageFirewall = false` in the same `[iscsi]` section to turn it off. See [Firewall of Xen Orchestra](../backup-features-and-settings.md#live-mount-firewall) for how the rules behave, including when ufw is changed or the proxy crashes.
 - The mount lasts as long as the proxy process: restarting or updating the proxy ends it.
 - Deleting the live mounted disk, or the VM with its disks, releases the mount, as without a proxy.
 

packages/xo-server/config.toml

@@ -219,6 +219,22 @@ requestTimeout = 0
 # Address the iSCSI targets bind to. Unset means all interfaces.
 #bindAddress = '0.0.0.0'
 
+# Firewall to open the port of each mount in, to the host it is attached to and
+# for the lifetime of the mount only: every mount listens on its own ephemeral
+# port, which cannot be allowed in advance.
+#
+# Only 'ufw' is supported: rules are inserted in `ufw-user-input` (and
+# `ufw6-user-input`) with `iptables`, so they are never persisted, and the ones a
+# crash left behind are removed at the next start. Any change made to ufw
+# (`ufw allow`, `ufw delete`, `ufw reload`…) drops them, which breaks the mounts
+# alive at that time: the firewall of an appliance is not meant to be changed by
+# hand.
+#
+# Without iptables, without ufw enabled, or when not running as root, no rule
+# is opened and the mounts go on: whatever firewall there is must let the hosts
+# in. Set to `false` to leave the firewall of this appliance alone.
+manageFirewall = 'ufw'
+
 [logs]
 # Display all logs matching this filter, regardless of their level
 #filter = 'xo:load-balancer'