Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

NetworkPkg: Do not enforce secure RNG #66

Closed
wants to merge 1 commit into from

Conversation

mikebeaton
Copy link
Contributor

@mikebeaton mikebeaton commented Aug 11, 2024

Since edk2-stable202405 we require EFI_RNG_PROTOCOL (RNG=Random Number Generator) for various network stack drivers.

We can't avoid requiring the protocol, but we do not want to insist that a secure algorithm is present. If we do leave the Pcd TRUE, DxeNetLib logs at level DEBUG_ERROR when using OVMF -device virtio-rng-pci (even though the supported RNG protocols check eventually succeeds), and it may do the same with the available Rng in various firmware too (and there, the RN generation may not succeed, if none of the required algos are present; even if some other RNG algorithm is present).

Believe this is probably a change we want to make, for ease of use of OVMF (as above) and our shipped network drivers (which need to run on any firmware).

EDIT: This change is no longer required for ease of use of OVMF - the 202408 EDK 2 commit mentioned below fixes that; but I think we still want to be able to use whatever the existing RNG is (whether or not is is secure) with our network stack on other firmware - probably? Perhaps not - in which case maybe forget this change (i.e. if a user's firmware has only RNG considered insecure, then they will need to load RngDxe.efi).

EDIT: Obviously if compiling custom firmware which is supposed to include a secure algorithm, this value can and should be switched back to its previous default. (Note that in that case, it makes sense to downgrade this line from DEBUG_ERROR, a change which on checking now I find has already been introduced in edk2-stable202408 tianocore/edk2@6862b9d, see also tianocore/edk2#6171.)

Since edk2-stable202405 we require EFI_RNG_PROTOCOL for
various network stack drivers.

We can't avoid requiring the protocol, but we do not want to insist
that a secure algorithm is present.
If we do leave the Pcd TRUE, DxeNetLib logs at level DEBUG_ERROR when
using OVMF -device virtio-rng-pci (even though the RN generation
eventually succeeds), and it may do so with the available Rng in
various firmware too (and there, the RN generation may not succeed,
if none of the required algos are present).
@mikebeaton
Copy link
Contributor Author

mikebeaton commented Sep 9, 2024

I'm close to thinking I can retire this PR, and leave PcdEnforceSecureRngAlgorithms=TRUE.

The issue with the network stack potentially logging at DEBUG_ERROR when it is working (which actually happened, with -device virtio-rng-pci on OVMF) is fixed.

The remaining issue is that RngDxe (which is what the user would have to load, if a check for a secure RNG fails), will itself fail to load on processors with no rdrand, and on processors where the EDK 2 code can detect broken rdrand.

I wonder whether there are any systems with broken rdrand in the CPU and somewhat serviceable - but not in the secure list - RNG protocol in the firmware?

In this case, neither PcdEnforceSecureRngAlgorithms=TRUE not PcdEnforceSecureRngAlgorithms=TRUE + RngDxe will work, even though the user could have had a working network stack with PcdEnforceSecureRngAlgorithms=FALSE.

The disadvantage of the current EDK 2 PcdEnforceSecureRngAlgorithms=FALSE implementation is that it won't even try to look for a secure algo, before falling back to the default RNG.

These may be such edge cases as to be not worth worrying about - perhaps on most firmware, there's only one RNG, and it's either secure or not.

Or perhaps we might want to update our audk implementation of PcdEnforceSecureRngAlgorithms=FALSE so that it searches for a secure algorithm if it can find one, but falls back to the default RNG if it cannot? That way our network stack can always be secure whenever there is a secure RNG available, but can still work as long as there's any RNG available. (Which is still fewer systems than before the recent EDK 2 udpates, as before that no RNG protocol was required at all.)

@mikebeaton
Copy link
Contributor Author

mikebeaton commented Sep 9, 2024

Guess I will close this (leaving PcdEnforceSecureRngAlgorithms=TRUE in our stack), and either make or not make (probably not make, for now; unless/until it proves to be required) a PR for the above suggestion.

@mikebeaton mikebeaton closed this Sep 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

1 participant