Re: [PATCH v5 4/5] x86/sgx: Implement ENCLS[EUPDATESVN]

["Huang, Kai" <kai.huang@intel.com>]

> > 
> > > +
> > > +	for (int i = 0; i < RDRAND_RETRY_LOOPS; i++) {
> > > +		ret = __eupdatesvn();
> > > +
> > > +		/* Stop on success or unexpected errors: */
> > > +		if (ret != SGX_INSUFFICIENT_ENTROPY)
> > > +			break;
> > > +	}
> > > +
> > > +	/*
> > > +	 * SVN either was up-to-date or SVN update failed due
> > > +	 * to lack of entropy. In both cases, we want to return
> > > +	 * 0 in order not to break sgx_(vepc_)open. We dont expect
> > > +	 * SGX_INSUFFICIENT_ENTROPY error unless underlying RDSEED
> > > +	 * is under heavy pressure.
> > > +	 */
> > > +	if ((ret == SGX_NO_UPDATE) || (ret == SGX_INSUFFICIENT_ENTROPY))
> > > +		return 0;
> > 
> > I am a little bit confused why we should return 0 when running out of
> > entropy.
> > 
> > It seems in v4 you said it's not that hard to cause EUPDATESVN to fail reliably:
> > 
> >     And to make it more concrete, I made some simple tests based
> >     on program for stress testing rdrand/rdseed that was discussed in that
> >     threat earlier: https://lkml.org/lkml/2024/2/6/746
> >     Using this stress testing and enough threads, I can make EUPDATESVN fail
> >     reliably and quite easily even with 10 time retry loop by kernel.
> > 
> > Returning 0 will make sgx_open() succeed if I read your next patch correctly,
> > which means this will allow enclave to be created when updating SVN fails.
> 
> Yes, correct. 
> 
> > 
> > Why not just fail sgx_open() (e.g., return -EBUSY, or -EAGAIN) in that case?
> > Userspace can then retry?
> 
> The idea on the patch was that such a scenario where we run out of entropy
> should not happen in real life unless RDSEED is under stress (in case we 
> accidentally collided, we do a 10 time retry). So, in this case we keep the legacy
> behaviour, i.e. proceeding without EUPDATESVN. But I can change to the above
> logic to return -EAGAIN in this case if everyone thinks it is a better approach. 

Well I think I am seeing conflicting message:

You mentioned in v4 that some simple (userspace!) tests can make EUPDATESVN fail
"reliably and quite easily even with 10 time retry loop by kernel".  This seems
to me that "RDSEED is under stress" can certainly happen in in real life.

Or are you suggesting that kinda "simple tests" cannot happen in real life?

Even we agree that such test cannot happen in real life, since updating SVN is
about security, I think it's quite fair that we need to consider that the
platform is under attack.

Allowing enclave to be created when EUPDATESVN fails due to running out of
entropy is a clear violation of security to me.  And what's even worse is AFAICT
userspace is not notified about this by any means.