2008-02-21 18:44:17

by Kevin Coffman

[permalink] [raw]
Subject: [PATCH 06/19] Use get_random_bytes() to create confounder

Instead of using an incementing value for the confounder, use
get_random_bytes() which gives us the desired unpredictable value.

Signed-off-by: Kevin Coffman <[email protected]>
---

net/sunrpc/auth_gss/gss_krb5_wrap.c | 15 +--------------
1 files changed, 1 insertions(+), 14 deletions(-)

diff --git a/net/sunrpc/auth_gss/gss_krb5_wrap.c b/net/sunrpc/auth_gss/gss_krb5_wrap.c
index a2c92f1..7a0002f 100644
--- a/net/sunrpc/auth_gss/gss_krb5_wrap.c
+++ b/net/sunrpc/auth_gss/gss_krb5_wrap.c
@@ -90,20 +90,7 @@ out:
static inline void
make_confounder(char *p, int blocksize)
{
- static u64 i = 0;
- u64 *q = (u64 *)p;
-
- /* rfc1964 claims this should be "random". But all that's really
- * necessary is that it be unique. And not even that is necessary in
- * our case since our "gssapi" implementation exists only to support
- * rpcsec_gss, so we know that the only buffers we will ever encrypt
- * already begin with a unique sequence number. Just to hedge my bets
- * I'll make a half-hearted attempt at something unique, but ensuring
- * uniqueness would mean worrying about atomicity and rollover, and I
- * don't care enough. */
-
- BUG_ON(blocksize != 8);
- *q = i++;
+ get_random_bytes(p, blocksize);
}

/* Assumptions: the head and tail of inbuf are ours to play with.



2008-03-12 16:46:19

by J. Bruce Fields

[permalink] [raw]
Subject: Re: [PATCH 06/19] Use get_random_bytes() to create confounder

On Thu, Feb 21, 2008 at 01:44:17PM -0500, Kevin Coffman wrote:
> Instead of using an incementing value for the confounder, use
> get_random_bytes() which gives us the desired unpredictable value.

OK, admittedly I was probably nuts to substitute a predictable number
for a random number in any cryptographic protocol, even if I was pretty
sure it didn't matter in our case--so thanks for doing this.

But my impression is that other callers of this function are using it on
a per-connection basis instead of (as in our case) a per-rpc basis. Is
there any problem with calling it that frequently? Is it fast enough?
Will it deplete some common entropy pool?

--b.

>
> Signed-off-by: Kevin Coffman <[email protected]>
> ---
>
> net/sunrpc/auth_gss/gss_krb5_wrap.c | 15 +--------------
> 1 files changed, 1 insertions(+), 14 deletions(-)
>
> diff --git a/net/sunrpc/auth_gss/gss_krb5_wrap.c b/net/sunrpc/auth_gss/gss_krb5_wrap.c
> index a2c92f1..7a0002f 100644
> --- a/net/sunrpc/auth_gss/gss_krb5_wrap.c
> +++ b/net/sunrpc/auth_gss/gss_krb5_wrap.c
> @@ -90,20 +90,7 @@ out:
> static inline void
> make_confounder(char *p, int blocksize)
> {
> - static u64 i = 0;
> - u64 *q = (u64 *)p;
> -
> - /* rfc1964 claims this should be "random". But all that's really
> - * necessary is that it be unique. And not even that is necessary in
> - * our case since our "gssapi" implementation exists only to support
> - * rpcsec_gss, so we know that the only buffers we will ever encrypt
> - * already begin with a unique sequence number. Just to hedge my bets
> - * I'll make a half-hearted attempt at something unique, but ensuring
> - * uniqueness would mean worrying about atomicity and rollover, and I
> - * don't care enough. */
> -
> - BUG_ON(blocksize != 8);
> - *q = i++;
> + get_random_bytes(p, blocksize);
> }
>
> /* Assumptions: the head and tail of inbuf are ours to play with.
>
> -
> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html

2008-03-12 17:50:42

by Matt Mackall

[permalink] [raw]
Subject: Re: [PATCH 06/19] Use get_random_bytes() to create confounder


On Wed, 2008-03-12 at 12:46 -0400, J. Bruce Fields wrote:
> On Thu, Feb 21, 2008 at 01:44:17PM -0500, Kevin Coffman wrote:
> > Instead of using an incementing value for the confounder, use
> > get_random_bytes() which gives us the desired unpredictable value.
>
> OK, admittedly I was probably nuts to substitute a predictable number
> for a random number in any cryptographic protocol, even if I was pretty
> sure it didn't matter in our case--so thanks for doing this.
>
> But my impression is that other callers of this function are using it on
> a per-connection basis instead of (as in our case) a per-rpc basis. Is
> there any problem with calling it that frequently? Is it fast enough?
> Will it deplete some common entropy pool?

get_random_bytes is in the many megabytes/second range, so it's fast
enough for most things, but considered too slow for per-socket use
(which means the comment above it is quite stale!). If per-rpc means
once per every stat over NFS, then definitely too expensive. It draws
from the non-blocking pool, so no worries about entropy depletion.

Take a look at lib/random32.c for a moderately stong and fast PRND.
Reseeding that periodically with get_random_bytes might be sufficient.
Or look at secure_tcp_sequence_number in random.c for a more ad-hoc
approach.

--
Mathematics is the supreme nostalgia of our time.


2008-03-12 18:03:40

by J. Bruce Fields

[permalink] [raw]
Subject: Re: [PATCH 06/19] Use get_random_bytes() to create confounder

On Wed, Mar 12, 2008 at 12:50:38PM -0500, Matt Mackall wrote:
>
> On Wed, 2008-03-12 at 12:46 -0400, J. Bruce Fields wrote:
> > On Thu, Feb 21, 2008 at 01:44:17PM -0500, Kevin Coffman wrote:
> > > Instead of using an incementing value for the confounder, use
> > > get_random_bytes() which gives us the desired unpredictable value.
> >
> > OK, admittedly I was probably nuts to substitute a predictable number
> > for a random number in any cryptographic protocol, even if I was pretty
> > sure it didn't matter in our case--so thanks for doing this.
> >
> > But my impression is that other callers of this function are using it on
> > a per-connection basis instead of (as in our case) a per-rpc basis. Is
> > there any problem with calling it that frequently? Is it fast enough?
> > Will it deplete some common entropy pool?
>
> get_random_bytes is in the many megabytes/second range, so it's fast
> enough for most things, but considered too slow for per-socket use
> (which means the comment above it is quite stale!). If per-rpc means
> once per every stat over NFS,

Yep, that's what I meant. Of course we are also encrypting every stat
in this particular case, so we're accepting a certain amount of
overhead.

> then definitely too expensive.

OK.

> It draws
> from the non-blocking pool, so no worries about entropy depletion.
>
> Take a look at lib/random32.c for a moderately stong and fast PRND.
> Reseeding that periodically with get_random_bytes might be sufficient.
> Or look at secure_tcp_sequence_number in random.c for a more ad-hoc
> approach.

OK. Yes, I'd definitely be happy to sacrifice quality for performance
in this case, at least until I see an argument pointing out some reason
we need good randomness here....

Thanks for the help!

--b.

2008-03-12 18:37:28

by Kevin Coffman

[permalink] [raw]
Subject: Re: [PATCH 06/19] Use get_random_bytes() to create confounder

On Wed, Mar 12, 2008 at 2:03 PM, J. Bruce Fields <[email protected]> wrote:
> On Wed, Mar 12, 2008 at 12:50:38PM -0500, Matt Mackall wrote:
>
> > Take a look at lib/random32.c for a moderately stong and fast PRND.
> > Reseeding that periodically with get_random_bytes might be sufficient.
> > Or look at secure_tcp_sequence_number in random.c for a more ad-hoc
> > approach.
>
> OK. Yes, I'd definitely be happy to sacrifice quality for performance
> in this case, at least until I see an argument pointing out some reason
> we need good randomness here....
>
> Thanks for the help!

Yes, thanks Matt. It looks to me like random32() should be good (and
fast) enough for this. Do you agree Bruce?

K.C.

2008-03-12 18:39:15

by J. Bruce Fields

[permalink] [raw]
Subject: Re: [PATCH 06/19] Use get_random_bytes() to create confounder

On Wed, Mar 12, 2008 at 02:37:26PM -0400, Kevin Coffman wrote:
> On Wed, Mar 12, 2008 at 2:03 PM, J. Bruce Fields <[email protected]> wrote:
> > On Wed, Mar 12, 2008 at 12:50:38PM -0500, Matt Mackall wrote:
> >
> > > Take a look at lib/random32.c for a moderately stong and fast PRND.
> > > Reseeding that periodically with get_random_bytes might be sufficient.
> > > Or look at secure_tcp_sequence_number in random.c for a more ad-hoc
> > > approach.
> >
> > OK. Yes, I'd definitely be happy to sacrifice quality for performance
> > in this case, at least until I see an argument pointing out some reason
> > we need good randomness here....
> >
> > Thanks for the help!
>
> Yes, thanks Matt. It looks to me like random32() should be good (and
> fast) enough for this. Do you agree Bruce?

OK by me.

--b.

2008-03-12 18:53:33

by Matt Mackall

[permalink] [raw]
Subject: Re: [PATCH 06/19] Use get_random_bytes() to create confounder


On Wed, 2008-03-12 at 14:39 -0400, J. Bruce Fields wrote:
> On Wed, Mar 12, 2008 at 02:37:26PM -0400, Kevin Coffman wrote:
> > On Wed, Mar 12, 2008 at 2:03 PM, J. Bruce Fields <[email protected]> wrote:
> > > On Wed, Mar 12, 2008 at 12:50:38PM -0500, Matt Mackall wrote:
> > >
> > > > Take a look at lib/random32.c for a moderately stong and fast PRND.
> > > > Reseeding that periodically with get_random_bytes might be sufficient.
> > > > Or look at secure_tcp_sequence_number in random.c for a more ad-hoc
> > > > approach.
> > >
> > > OK. Yes, I'd definitely be happy to sacrifice quality for performance
> > > in this case, at least until I see an argument pointing out some reason
> > > we need good randomness here....
> > >
> > > Thanks for the help!
> >
> > Yes, thanks Matt. It looks to me like random32() should be good (and
> > fast) enough for this. Do you agree Bruce?
>
> OK by me.

Polite users of random32 should call srandom32 with some interesting
values occasionally. At authentication/connection time might be
appropriate.

--
Mathematics is the supreme nostalgia of our time.