Page MenuHomeFreeBSD

dumpon(8): support OpenSSL 3
Needs ReviewPublic

Authored by ngie on May 27 2023, 2:56 AM.
Tags
None
Referenced Files
Unknown Object (File)
Mar 14 2024, 7:04 AM
Unknown Object (File)
Dec 25 2023, 10:55 PM
Unknown Object (File)
Dec 20 2023, 7:42 AM
Unknown Object (File)
Dec 3 2023, 7:47 AM
Unknown Object (File)
Nov 30 2023, 5:38 AM
Unknown Object (File)
Nov 13 2023, 4:25 PM
Unknown Object (File)
Nov 10 2023, 4:07 PM
Unknown Object (File)
Nov 9 2023, 12:32 AM
Subscribers

Details

Summary

This change moves away from the 3.0 deprecated APIs such that the code
no longer needs to explicitly request OpenSSL 1.1 API support to compile
cleanly with -Werror.

While here, axe pre-OpenSSL 1.1 ifdefs: FreeBSD has not "shipped"
OpenSSL 1.0 for some time, so there's no need to keep the ifdef soup
around.

MFC after: 1 month

Test Plan

Tested with both OpenSSL 1.1 from base and 3.0 (from ports). Both instances fail when trying to set the kernelsdump ioctl because the device I was using is md(4) based:

# rsa.public generated with the following 2 commands:
# - openssl genrsa -out rsa.private 2048
# - openssl rsa -in rsa.priv -out rsa.public -pubout -outform PEM
$ cat rsa.public 
-----BEGIN PUBLIC KEY-----
MIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEAz/Kc/3eSS4uCoEBhzsI7
P3Vy7+Ar/JcKHUL6nlrKwOECOTaQf63KhIxvHh3wsBIpiqIDoN0MgumECCdhco8G
mdwcFzf5zavq7LsYPsfY79nEUgrtqNPxS0hekNhg49RVjRBFdm++CL4c1HHoj/QO
V2+SDDPO+2/0ZlfZdtbglYNTB+jh3YpgXlh0ap5axpY/s47fcJVhHGDFBChwaGPd
0Kb16XCqmHyCAx5q2W+haBDwEXhXSBSbFMCB5luItteoG/66gtUK1fpps9Jmx0e8
MHxnkFYctVAbDw/p1boyQQWQ7o9FyLyKWf8/hx6RPyA9kV3oFPYhE0Pf/YZOzRcU
WwIDAQAB
-----END PUBLIC KEY-----
$ sudo dumpon -vvv -k ./rsa.public /dev/md0 
dumpon: ioctl(DIOCSKERNELDUMP): Operation not supported
$ sudo `make -V.OBJDIR`/dumpon -vvv -k ./rsa.public /dev/md0 
dumpon: ioctl(DIOCSKERNELDUMP): Operation not supported

The above public key was

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Skipped
Unit
Tests Skipped
Build Status
Buildable 51742
Build 48633: arc lint + arc unit

Event Timeline

ngie requested review of this revision.May 27 2023, 2:56 AM
ngie edited the test plan for this revision. (Show Details)

I think these OPENSSL_VERSION_NUMBER >= 0x30000000L code will actually work with OpenSSL 1.1?

Could you please try if the code will still work after stripping the old code with unifdef -DOPENSSL_VERSION_NUMBER=0x30000000L, and remove the old code if that's the case? Assuming it worked, it would also allow us to remove some intermediate variables and make the diff smaller.

ngie edited the test plan for this revision. (Show Details)

Remove all pre-3.0 related ifdefs.

All but EVP_PKEY_get_size (which is just an alias to EVP_PKEY_size in 3.0)
can be found/used in 1.1.

Eliminate all possible intermediate variables.

sbin/dumpon/dumpon.c
292–293

I think EVP_PKEY_bits(pubkey) would be more appropriate here (it's the actual bits of the RSA key). The security bits is the estimate of security strength equivalent and is not linear to key size.

Update values and messages used when testing pubkey limits

Make error messages associated with the required minimum and maximum
number of RSA pubkeys more direct.

Use functions and computed values that match the desired end-state without
having to convert too much between bits/size.

Make the minimum required RSA bits 256, not 112.

The previous code stated that the minimum number of bits must be 2048 for
pre-1.1 builds. Honor the 256 bits requirement, as the 112 minimum (which
results in 896 bytes) -- a number which seems strange to use as a lower
bound for RSA algorithmic complexity.

Consistently use bytes or bits in calculations

Use bytes when talking about RSA pubkey length.

Include appropriate headers for new APIs in use

I've been going back and forth a lot over the wording and APIs in use because I've been refining my understanding upon reading the manpages and tried to get things consistent across the board to aid with program UX 😅.
The existing code was also misleading/incorrect in terms of how the limits were calculated and enforced, too: it should have been 256, not 112 -- I have no idea why someone chose the number 112.

sbin/dumpon/dumpon.c
233

This has to be in bits, not in bytes (regardless if we are requiring a security strength (the current code is doing) or a minimum size of key); using bytes is very confusing for readers as key sizes are always measured in bits.

The current code is using a security strength bits, as demanded in NIST SP800-57 (see reasoning in the comment below). Conceptually it's saying "This key is as strong as X bits symmetric cipher" regardless of the actual key size.

The proposed change would assume that the underlying key is a RSA key and requiring a minimal key size of 2048 bits (represented as 256 bytes), according to table 2 (page 54), the corresponding security strength is 112 bits, so it's equivalent to the current code.

I don't really have a strong opinion on which approach would be better here, but personally I prefer using the current one (use security bits instead of key length) because a) it's closer to the standard text (112 bits of strength acceptable up to 2030), and b) we don't have to change the code to support other public key encryption methods, because they would be mapped to the same strength bits table.

293

(see comment above; compare with bits not bytes. Personally I'd prefer using the security bits as we do today).

298

This should be EVP_PKEY_bits(pubkey), or EVP_PKEY_size(pubkey) * 8.

sbin/dumpon/dumpon.c
233

This is incredibly helpful/enlightening context; I'll digest this and mold the current code into what it needs to be.

I'll be sure to add references to the other NIST documents for further reading.

sbin/dumpon/dumpon.c
581

ERR_load_crypto_strings is deprecated as of OpenSSL 1.1; see D40353

sbin/dumpon/dumpon.c
581

Good catch!