dumpon: use underlying device if encrypted swap is in use

[emaste]

Summary

/etc/rc.d/dumpon runs before /etc/rc.d/swap.  When encrypted swap is in
use the .eli device will not exist at the time dumpon runs.  Even if
this is addressed it does not make sense to dump core to encrypted swap,
as the encryption key will not be available after reboot.  Thus, for the
case that dumpdev=AUTO and encrypted swap is in use, strip .eli and use
the underlying device.  Emit a warning in this case so that the user is
aware that the dump will not be encrypted.

PR:             238301
Reported by:    Ivan Rozhuk
MFC after:      1 week
Sponsored by:   The FreeBSD Foundation

[jrtc27]

libexec/rc/rc.d/dumpon

67

Is there a reason not to do this?

[emaste]

libexec/rc/rc.d/dumpon

67

No, I guess I just forgot about warn() (and the echo just below here didn't remind me).

75

So perhaps also this

[jhb]

libexec/rc/rc.d/dumpon

67

I think this is non-optional as otherwise you can never read the dump. IIRC, encrypted swap uses a randomly-generated key on each boot. There's no persistent key that would let you recover the dump. I'm not sure this is worth warning about as there's nothing the user can really do to mitigate the warning? Possibly you could only warn if EKCD isn't enabled, but those are only tangentially related. (EKCD uses a persistent key I think?)

[emaste]

use warn for warning (also for one existing case)

[jrtc27]

libexec/rc/rc.d/dumpon

67

My question was whether there was a reason not to use warn instead of echo, not about whether to strip the .eli or not.

[emaste]

@jhb I added the warning to ensure the user is aware - they chose to encrypt their swap, and (with dumpdev=AUTO) by default we will write unencrypted data to that swap partition. The user could specify the parent device explicitly to avoid the warning.

[kevans]

In D34474#781097, @emaste wrote:

@jhb I added the warning to ensure the user is aware - they chose to encrypt their swap, and (with dumpdev=AUTO) by default we will write unencrypted data to that swap partition. The user could specify the parent device explicitly to avoid the warning.

This is the default, too, isn't it? IMO being noisy about it is good, it may be the signal someone needs to reconfigure the system for netdump or avoiding dump.

[emaste]

Omit warning if the user provided an encryption key

[emaste]

This is the default, too, isn't it?

Swap is not encrypted by default in the installer at present.

[emaste]

drop change to existing message, it may not warrant warn()

[kevans]

In D34474#781105, @emaste wrote:

This is the default, too, isn't it?

Swap is not encrypted by default in the installer at present.

I meant dumpdev=auto

[emaste]

I meant dumpdev=auto

Ah, yes it is indeed.

[emaste]

Update expr quoting

[jamie_catflap.org]

libexec/rc/rc.d/dumpon

67

@jhb The point is to warn someone who may be using encrypted swap that if they have dumps

67

I think Ed added that in response to my comment on the bug:

"As for dumping to swap, doesn't that negate the purpose of using encrypted swap in the first place? All someone would need to do is force a panic, and the memory contents would be dumped in the clear." ( https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=238301 )

I could quite easily see someone not realising that if dump was enabled, their swap may not be completely secure.. Though I can see your point that some people may get annoyed seeing that message all the time even when they know what they are doing!

Cheers, Jamie

[jamie_catflap.org]

libexec/rc/rc.d/dumpon

68

The manual page for fstab also mentions the GBDE swap option, with prefix .bde

[jamie_catflap.org]

In D34474#781108, @kevans wrote:

I meant dumpdev=auto

But before this patch, dumping failed when encrypted swap was enabled (I assumed this was intentional, as you normally wouldn't want unecrypted swap data written to disk under any circumstances)

For anyone in a similar situation, this patch would enabled swapping with no warning.

The warning message could be suppressed by specifically setting the dumpdev (In my earlier comment, I missed the fact that Ed had pointed this out)

[jilles]

libexec/rc/rc.d/dumpon

66

This can be checked using shell builtins: case $dev in *.eli) ...

This avoids that it does not work for certain values for $dev like - which are admittedly unlikely, and therefore sets a better example for other code.

[emaste]

libexec/rc/rc.d/dumpon

66

It does work for $dev starting with -:

expr -- -foo.eli : '.*\.eli$' >/dev/null && echo true
true

but as Jamie points out we should handle .bde also, so perhaps something like

case $dev in
*.eli)
        dumpon_warn_unencrypted
        dev=${dev%.eli}
       ;;
*.bde)
        dumpon_warn_unencrypted
        dev=${dev%.bde}
        ;;
esac

[emaste]

libexec/rc/rc.d/dumpon

66

Ah, but indeed not exactly -.

[jrtc27]

libexec/rc/rc.d/dumpon

66

Or less repetitively:

case $dev in
*.bde|*.eli)
        warn "..."
        dev=${dev%.*}
        ;;
esac

(single % is the right one to leave foo.bar.eli as foo.bar not foo)

[emaste]

Use case as suggested by @jilles and handle .bde as well

[emaste]

Incorporate @jrtc27 suggestion

[emaste]

Any other feedback?

[jamie_catflap.org]

In D34474#781829, @emaste wrote:

Any other feedback?

Looks perfect!