Page MenuHomeFreeBSD

kerberos: Fix numerous segfaults when using weak crypto
ClosedPublic

Authored by cy on Dec 12 2023, 8:54 PM.
Tags
None
Referenced Files
Unknown Object (File)
Sun, May 5, 8:23 PM
Unknown Object (File)
Wed, May 1, 1:35 AM
Unknown Object (File)
Wed, May 1, 1:34 AM
Unknown Object (File)
Wed, May 1, 1:32 AM
Unknown Object (File)
Wed, May 1, 1:31 AM
Unknown Object (File)
Tue, Apr 30, 4:41 PM
Unknown Object (File)
Sun, Apr 28, 5:02 PM
Unknown Object (File)
Sun, Apr 28, 4:14 PM
Subscribers

Details

Summary

Weak crypto is provided by the openssl legacy provider which is
not load by default. Load the legacy providers as needed.

When the legacy provider is loaded into the default context the default
provider will no longer be automatically loaded. Without the default
provider the various kerberos applicaions and functions will abort().

PR: 272835
MFC after: 3 days

Test Plan

Tested locally in a jail and currently running on all my systems.

Tested by the person who reported PR/272835.

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

cy requested review of this revision.Dec 12 2023, 8:54 PM

Looking now. For future patch updates would you generate with full context (e.g. git diff -U9999999 or git show -U9999999?

curious about inconsistent fbsd_ossl_provider_load calls - i.e. return value checked / not checked / (void) cast

Overall looks like a reasonable approach

Makefile.inc1
2625

update comment, maybe just libroken dependencies: or so, such that it doesn't get out of date again

Looking now. For future patch updates would you generate with full context (e.g. git diff -U9999999 or git show -U9999999?

Looks like git-format-patch has a -U argument. I'll try that.

Use git-format-patch -U.

cy marked an inline comment as done.Dec 12 2023, 11:15 PM

Comment adjusted.

kerberos5/lib/libroken/fbsd_ossl_provider_load.c
22–23

It might be good to return the exact value of errno as set by the call to atexit(3) in case of failures. According to its manual page and to its source code, ENOMEM is the only likely one, but atexit(3) from the libc is a weak symbol and might be overridden.

From what I can tell OSSL_PROVIDER_load(3) simply returns OK/FAIL and is not expected to set errno to any specific value, so EINVAL is probably as good as it gets.

cy marked an inline comment as done.Dec 13 2023, 7:42 PM

Implement latest review suggestion.

Implement Ed's suggestion to universally check return codes.

This revision was not accepted when it landed; it landed in state Needs Review.Jan 11 2024, 1:32 PM
This revision was automatically updated to reflect the committed changes.
kerberos5/lib/libroken/Makefile
78

Rest of SRCS is nicely sorted (as it should be), either slot this into the right spot,
or if you want to call it out as an addition use SRCS+=

89

you can remove the extra line