dumpon(8): Provide seatbelt against weak RSA keys

[cem]

Summary

The premise of dumpon -k foo.pem is that dump contents will be confidential
except to anyone holding the corresponding RSA private key.

This guarantee breaks down when weak RSA keys are used. Small RSA keys
(e.g. 512 bits) can be broken on a single personal computer in tractible
time. Marginal RSA keys (768 bits) can be broken by EC2 and a few dollars.
Even 1024 bit keys can probably be broken by sophisticated and wealthy
attackers.

It would also be good to protect users from weak values of 'e' (i.e., 3) and
perhaps sanity check that their public key .pem does not accidentally
contain their private key as well. These considerations are left as future
exercises.

Test Plan

$ for i in 1024 768 512 ; do \
    openssl genrsa -out testpriv${i}.pem $i ; echo $? ;\
    openssl rsa -in testpriv${i}.pem -out testPUB${i}.pem -pubout ; echo $? ; \
    sudo dumpon off; echo "dumpon ${i}:"; \
    sudo $(make -V .OBJDIR)/dumpon -k ./testPUB${i}.pem gpt/freebsd-swap; echo $? ; \
    echo ; \
  done

Generating RSA private key, 1024 bit long modulus
..++++++
...........++++++
e is 65537 (0x010001)
0
writing RSA key
0
dumpon 1024:
0

Generating RSA private key, 768 bit long modulus
.....++++++++
....++++++++
e is 65537 (0x010001)
0
writing RSA key
0
dumpon 768:
dumpon: RSA keys smaller than 1024b (you provided: 768b) are small enough to factor cheaply.  Please generate a larger key.
1

Generating RSA private key, 512 bit long modulus
.++++++++++++
....................................++++++++++++
e is 65537 (0x010001)
0
writing RSA key
0
dumpon 512:
dumpon: RSA keys smaller than 1024b (you provided: 512b) are small enough to factor cheaply.  Please generate a larger key.
1

[darius-dons.net.au]

Ship it!

[markj]

sbin/dumpon/dumpon.c

256 ↗

(On Diff #49537)

Doesn't this kind of policy belong in the kernel?

[cem]

sbin/dumpon/dumpon.c

256 ↗

(On Diff #49537)

It might, but the kernel doesn't get any information about the RSA key -- this is all done in userspace. The kernel just gets the plaintext symmetric key data and the encrypted plaintext symmetric key data as blobs. All public key crypto for EKCD is done in userspace.

[markj]

sbin/dumpon/dumpon.c

256 ↗

(On Diff #49537)

A couple of lines below we set kda_encryptedkeysize = RSA_size(pubkey), and that is indeed passed to the kernel.

[cem]

sbin/dumpon/dumpon.c

256 ↗

(On Diff #49537)

Sure, that's true. However, that ABI is just the size of the encrypted symmetric key data blob in bytes — there's no reason the asymmetric algorithm used in userspace has to be RSA or that it must match the key size for asymmetric algorithms other than RSA. (In libsodium terms, we're doing a crypto_box_seal operation.)

Other choices (like elliptic curve publickey encryption schemes) might be a good idea going forward. And we can't validate, e.g., sane 'e' (not trivially small) or lack of 'd' (not a public key!) from the kernel, as we lose all of the key data.

I think if anywhere this sort of policy belongs in OpenSSL, actually. But I am unaware of any such routine for enforcing non-weak RSA keys in OpenSSL. Maybe Ben Kaduk (OpenSSL committer) has a better idea. I've tagged him as a reviewer.

[cem]

sbin/dumpon/dumpon.c

256 ↗

(On Diff #49537)

"must match the key size" -> "must match the asymmetric key size"

[markj]

sbin/dumpon/dumpon.c

255 ↗

(On Diff #49537)

The parens are redundant.

256 ↗

(On Diff #49537)

Ok. And if we were to add support for other algorithms, I guess the decryptor can figure out the key encryption algorithm used by examining the key passed by the user. That is, there would be no need to encode that information in the kernel dump header.

[cem]

sbin/dumpon/dumpon.c

255 ↗

(On Diff #49537)

I didn't remember the order of operations when I added them :-). Will remove.

256 ↗

(On Diff #49537)

I haven't thought through it very well yet. :-) Here's a rough sketch:

I guess the decryptor can figure out the key encryption algorithm used by examining the key passed by the user.

Perhaps. It's worth remembering that the current scheme assumes that the user on the savecore/netdumpd end knows exactly which public key was used to encrypt a dump — there's nothing to identify the specific public key used in the dump header nor encrypted key block today. So in that sense, having to know exactly which algorithm and key was used to encrypt a dump is just status quo.

That is, there would be no need to encode that information in the kernel dump header.

It might be worth adding a public key identifier (and asymmetric cipher mode enum while we're at it) independent of any change in the specific asymmetric algorithm.

As far as actual mechanics of the asym. cipher mode, we could borrow a few bits from 'kdc_encryption' (uint8_t). Or there are plenty of bits available in kdk_encryptedkeysize — it's a uint32_t representing numbers on the order of 4096 — to borrow some.

As far as identifying the pubkey used, we could put a truncated digest of the public key (or whatever the literature suggests is a good idea) in the unused back half of the IV array, which is excessively sized for most (?all) symmetric ciphers, as a way to help administrators identify the public key used for the dump.

I intend to leave this for future work, if that's alright.

[cem]

Remove excess parens and reword preceding comment slightly.

No functional change.

[markj]

sbin/dumpon/dumpon.c

256 ↗

(On Diff #49537)

There's no need to worry about changing the layout of struct diocskerneldump_arg: it's allowed to change on -CURRENT, and we can't easily MFC any changes along the lines you suggested (even if the layout is preserved) because the struct is not versioned.

Thanks for the explanation.

[cem]

sbin/dumpon/dumpon.c

256 ↗

(On Diff #49537)

Yeah, that said we should probably steal a few bits we know are zero in stable/12 to start a version field. So that current can dump leftover cores from stable/12.

[bjk]

sbin/dumpon/dumpon.c

256 ↗

(On Diff #49537)

The OpenSSL "security level" functionality (commonly seen in cipher strings as, e.g., "HIGH@SECLEVEL=1") is the natural way to get a high-level strength check, but it's only implemented at the X.509 verification level. Since we're using raw RSA objects here, the best option I can offer is to check against RSA_security_bits(pubkey) with a desired bit-strength.

[cem]

Use RSA_security_bits() equivalent function when available (OpenSSL 1.1.0, I
think) or fallback to the same NIST recommendation (2048 bits) when unavailable
(openssl 1.0.2).

Thanks Ben Kaduk for the pointer.