[PATCH] crypto: Jitter RNG - use ktime_get_raw_ns as fallback

[PATCH] crypto: Jitter RNG - use ktime_get_raw_ns as fallback

[Stephan Mueller]

As part of the Y2038 development, __getnstimeofday is not supposed to be
used any more. It is now replaced with ktime_get_raw_ns. Albeit
ktime_get_raw_ns is monotonic compared to __getnstimeofday, this
difference is irrelevant as the Jitter RNG uses the time stamp to
measure the execution time of a given code path and tries to detect
variations in the execution time. Therefore, the only requirement the
Jitter RNG has, is a sufficient high resolution to detect these
variations.

The change was tested on x86 to show an identical behavior as RDTSC. The
used test code simply measures the execution time of the heart of the
RNG:

	jent_get_nstime(&time);
	jent_memaccess(ec, min);
	jent_fold_time(NULL, time, &folded, min);
	jent_get_nstime(&time2);
	return ((time2 - time));

Signed-off-by: Stephan Mueller <smueller@chronox.de>
---
 crypto/jitterentropy-kcapi.c | 23 ++++++++++++++---------
 1 file changed, 14 insertions(+), 9 deletions(-)

diff --git a/crypto/jitterentropy-kcapi.c b/crypto/jitterentropy-kcapi.c
index 597cedd..69a2988 100644
--- a/crypto/jitterentropy-kcapi.c
+++ b/crypto/jitterentropy-kcapi.c
@@ -87,24 +87,29 @@ void jent_memcpy(void *dest, const void *src, unsigned int n)
 	memcpy(dest, src, n);
 }
 
+/*
+ * Obtain a high-resolution time stamp value. The time stamp is used to measure
+ * the execution time of a given code path and its variations. Hence, the time
+ * stamp must have a sufficiently high resolution. It is valid if the time
+ * runs backwards for short period of time as the RNG code is able handle that.
+ *
+ * Note, if the function returns zero because a given architecture does not
+ * implement a high-resolution time stamp, the RNG code's runtime test
+ * will detect it and will not produce output.
+ */
 void jent_get_nstime(__u64 *out)
 {
-	struct timespec ts;
 	__u64 tmp = 0;
 
 	tmp = random_get_entropy();
 
 	/*
-	 * If random_get_entropy does not return a value (which is possible on,
-	 * for example, MIPS), invoke __getnstimeofday
+	 * If random_get_entropy does not return a value, i.e. it is not
+	 * implemented for a given architecture, invoke ktime_get_raw_ns
 	 * hoping that there are timers we can work with.
 	 */
-	if ((0 == tmp) &&
-	   (0 == __getnstimeofday(&ts))) {
-		tmp = ts.tv_sec;
-		tmp = tmp << 32;
-		tmp = tmp | ts.tv_nsec;
-	}
+	if (tmp == 0)
+		tmp = ktime_get_raw_ns();
 
 	*out = tmp;
 }
-- 
2.5.5


_______________________________________________
Y2038 mailing list
Y2038@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/y2038

Re: [PATCH] crypto: Jitter RNG - use ktime_get_raw_ns as fallback

[John Stultz]

On Tue, Jun 21, 2016 at 11:05 AM, Stephan Mueller <smueller@chronox.de> wrote:
> As part of the Y2038 development, __getnstimeofday is not supposed to be
> used any more. It is now replaced with ktime_get_raw_ns. Albeit
> ktime_get_raw_ns is monotonic compared to __getnstimeofday, this
> difference is irrelevant as the Jitter RNG uses the time stamp to
> measure the execution time of a given code path and tries to detect
> variations in the execution time. Therefore, the only requirement the
> Jitter RNG has, is a sufficient high resolution to detect these
> variations.
>
> The change was tested on x86 to show an identical behavior as RDTSC. The
> used test code simply measures the execution time of the heart of the
> RNG:
>
>         jent_get_nstime(&time);
>         jent_memaccess(ec, min);
>         jent_fold_time(NULL, time, &folded, min);
>         jent_get_nstime(&time2);
>         return ((time2 - time));
>
> Signed-off-by: Stephan Mueller <smueller@chronox.de>
> ---
>  crypto/jitterentropy-kcapi.c | 23 ++++++++++++++---------
>  1 file changed, 14 insertions(+), 9 deletions(-)
>
> diff --git a/crypto/jitterentropy-kcapi.c b/crypto/jitterentropy-kcapi.c
> index 597cedd..69a2988 100644
> --- a/crypto/jitterentropy-kcapi.c
> +++ b/crypto/jitterentropy-kcapi.c
> @@ -87,24 +87,29 @@ void jent_memcpy(void *dest, const void *src, unsigned int n)
>         memcpy(dest, src, n);
>  }
>
> +/*
> + * Obtain a high-resolution time stamp value. The time stamp is used to measure
> + * the execution time of a given code path and its variations. Hence, the time
> + * stamp must have a sufficiently high resolution. It is valid if the time
> + * runs backwards for short period of time as the RNG code is able handle that.
> + *
> + * Note, if the function returns zero because a given architecture does not
> + * implement a high-resolution time stamp, the RNG code's runtime test
> + * will detect it and will not produce output.
> + */
>  void jent_get_nstime(__u64 *out)
>  {
> -       struct timespec ts;
>         __u64 tmp = 0;
>
>         tmp = random_get_entropy();
>
>         /*
> -        * If random_get_entropy does not return a value (which is possible on,
> -        * for example, MIPS), invoke __getnstimeofday
> +        * If random_get_entropy does not return a value, i.e. it is not
> +        * implemented for a given architecture, invoke ktime_get_raw_ns
>          * hoping that there are timers we can work with.
>          */
> -       if ((0 == tmp) &&
> -          (0 == __getnstimeofday(&ts))) {
> -               tmp = ts.tv_sec;
> -               tmp = tmp << 32;
> -               tmp = tmp | ts.tv_nsec;
> -       }
> +       if (tmp == 0)
> +               tmp = ktime_get_raw_ns();


I don't see in the above an explanation of *why* you're using
ktime_get_raw_ns() instead of ktime_get_ns().

Also the bit about time running backwards being ok is confusing since
you're not using the "fast" accessor where that would be a risk.

thanks
-john

Re: [PATCH] crypto: Jitter RNG - use ktime_get_raw_ns as fallback

[Stephan Mueller]

Am Dienstag, 21. Juni 2016, 11:11:42 schrieb John Stultz:

Hi John,

> I don't see in the above an explanation of *why* you're using
> ktime_get_raw_ns() instead of ktime_get_ns().

Could you help me understand what the difference is or point me to some 
documentation? I understood that we only talked about the _raw variant.
> 
> Also the bit about time running backwards being ok is confusing since
> you're not using the "fast" accessor where that would be a risk.

Ok, if the running backwards hint is only applicable to the _fast variants, I 
can certainly drop it.

Thanks
Stephan
_______________________________________________
Y2038 mailing list
Y2038@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/y2038

Re: [PATCH] crypto: Jitter RNG - use ktime_get_raw_ns as fallback

[John Stultz]

On Tue, Jun 21, 2016 at 11:49 AM, Stephan Mueller <smueller@chronox.de> wrote:
> Am Dienstag, 21. Juni 2016, 11:11:42 schrieb John Stultz:
>
> Hi John,
>
>> I don't see in the above an explanation of *why* you're using
>> ktime_get_raw_ns() instead of ktime_get_ns().
>
> Could you help me understand what the difference is or point me to some
> documentation? I understood that we only talked about the _raw variant.

Using specialized interfaces with subtle semantics w/o understanding
them is sort of my concern here.

There are reasons why you might want to use the ktime_get_raw_ns()
interface over ktime_get_ns(), but they have not been made clear in
the comment. Arnd discussed some potential concerns that the freq
adjustment done by ntp might be somewhat predictable/controlled by
remote parties, which could have some effect in the calculation. That
feels a little overly vague to me, but I'm no crypto expert, so if
that is a reasonable concern, then it should be a conscious and
documented decision.

thanks
-john
_______________________________________________
Y2038 mailing list
Y2038@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/y2038

Re: [Y2038] [PATCH] crypto: Jitter RNG - use ktime_get_raw_ns as fallback

[Arnd Bergmann]

On Tuesday, June 21, 2016 12:05:06 PM CEST John Stultz wrote:
> On Tue, Jun 21, 2016 at 11:49 AM, Stephan Mueller <smueller@chronox.de> wrote:
> > Am Dienstag, 21. Juni 2016, 11:11:42 schrieb John Stultz:
> >
> > Hi John,
> >
> >> I don't see in the above an explanation of *why* you're using
> >> ktime_get_raw_ns() instead of ktime_get_ns().
> >
> > Could you help me understand what the difference is or point me to some
> > documentation? I understood that we only talked about the _raw variant.
> 
> Using specialized interfaces with subtle semantics w/o understanding
> them is sort of my concern here.
> 
> There are reasons why you might want to use the ktime_get_raw_ns()
> interface over ktime_get_ns(), but they have not been made clear in
> the comment. Arnd discussed some potential concerns that the freq
> adjustment done by ntp might be somewhat predictable/controlled by
> remote parties, which could have some effect in the calculation. That
> feels a little overly vague to me, but I'm no crypto expert, so if
> that is a reasonable concern, then it should be a conscious and
> documented decision.

My original patch changed __getnstimeofday() to __getnstimeofday64(),
which kept the original semantics of not warning in case the clock
source is suspended (which is the only different to the normal
getnstimeofday{,64}().

I did the patch a while time ago along with a number of other patches
that I never sent out until last week, so I don't remember the
reasoning for suggesting ktime_get_raw_fast_ns() over ktime_get_raw_ns(),
but I sure wanted to keep the non-warning behavior, and ktime_get_ns()
warns on timekeeping_suspended() while the other two don't.

If we don't care about the non-warning aspect, ktime_get_ns() makes
most sense here, and the original code should probably have used
getnstimeofday() as well.

	Arnd

Re: [PATCH] crypto: Jitter RNG - use ktime_get_raw_ns as fallback

[John Stultz]

On Tue, Jun 21, 2016 at 12:37 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> On Tuesday, June 21, 2016 12:05:06 PM CEST John Stultz wrote:
>> On Tue, Jun 21, 2016 at 11:49 AM, Stephan Mueller <smueller@chronox.de> wrote:
>> > Am Dienstag, 21. Juni 2016, 11:11:42 schrieb John Stultz:
>> >
>> > Hi John,
>> >
>> >> I don't see in the above an explanation of *why* you're using
>> >> ktime_get_raw_ns() instead of ktime_get_ns().
>> >
>> > Could you help me understand what the difference is or point me to some
>> > documentation? I understood that we only talked about the _raw variant.
>>
>> Using specialized interfaces with subtle semantics w/o understanding
>> them is sort of my concern here.
>>
>> There are reasons why you might want to use the ktime_get_raw_ns()
>> interface over ktime_get_ns(), but they have not been made clear in
>> the comment. Arnd discussed some potential concerns that the freq
>> adjustment done by ntp might be somewhat predictable/controlled by
>> remote parties, which could have some effect in the calculation. That
>> feels a little overly vague to me, but I'm no crypto expert, so if
>> that is a reasonable concern, then it should be a conscious and
>> documented decision.
>
> My original patch changed __getnstimeofday() to __getnstimeofday64(),
> which kept the original semantics of not warning in case the clock
> source is suspended (which is the only different to the normal
> getnstimeofday{,64}().
>
> I did the patch a while time ago along with a number of other patches
> that I never sent out until last week, so I don't remember the
> reasoning for suggesting ktime_get_raw_fast_ns() over ktime_get_raw_ns(),
> but I sure wanted to keep the non-warning behavior, and ktime_get_ns()
> warns on timekeeping_suspended() while the other two don't.

Right. But this code isn't called in late suspend or early resume is it?


> If we don't care about the non-warning aspect, ktime_get_ns() makes
> most sense here, and the original code should probably have used
> getnstimeofday() as well.

This is what I suspect as well.  But again, I don't mind if folks use
the specialized interfaces, as long as they document a clear reason
for it. Especially for things like crypto where intuition isn't always
the best guide.

thanks
-john
_______________________________________________
Y2038 mailing list
Y2038@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/y2038