Re: [Minios-devel] [UNIKRAFT PATCH v2 3/4] lib/ukswrand: Add ChaCha algorithm

[Vlad-Andrei BĂDOIU (78692) <vlad_andrei.badoiu@xxxxxxxxxxxxxxx>]

Hey Simon,

Please see my reply inline.

Thanks,

Vlad

On 18.10.2019 16:11, Simon Kuenzer wrote:
> On 16.10.19 17:41, Vlad-Andrei BĂDOIU (78692) wrote:
>> ChaCha20 is a cryptographically secure pseudorandom number generator. 
>> We replace
>> the existing implementation, which is not secure, with ChaCha20. The
>> implementation is based on the reference implementation of the 
>> author[1].
>>
>> [1] 
>> http://cr.yp.to/streamciphers/timings/estreambench/submissions/salsa20/chacha8/ref/chacha.c
>>
>> Signed-off-by: Vlad-Andrei Badoiu <vlad_andrei.badoiu@xxxxxxxxxxxxxxx>
>> ---
>>   lib/ukswrand/Config.uk           |   7 +-
>>   lib/ukswrand/Makefile.uk         |   1 +
>>   lib/ukswrand/chacha.c            | 156 +++++++++++++++++++++++++++++++
>>   lib/ukswrand/include/uk/swrand.h |   5 +
>>   4 files changed, 168 insertions(+), 1 deletion(-)
>>   create mode 100644 lib/ukswrand/chacha.c
>>
>> diff --git a/lib/ukswrand/Config.uk b/lib/ukswrand/Config.uk
>> index 8db3af03..305b83e6 100644
>> --- a/lib/ukswrand/Config.uk
>> +++ b/lib/ukswrand/Config.uk
>> @@ -6,12 +6,17 @@ menuconfig LIBUKSWRAND
>>   if LIBUKSWRAND
>>   choice
>>       prompt "Algorithm"
>> -    default LIBUKSWRAND_MWC
>> +    default LIBUKSWRAND_CHACHA
>>     config LIBUKSWRAND_MWC
>>       bool "Multiply-with-carry"
>>       help
>>           Use multiply-with-carry algorithm
>> +
>> +config LIBUKSWRAND_CHACHA
>> +    bool "ChaCha20"
>> +    help
>> +        Use ChaCha20 algorithm
>>   endchoice
>>     config LIBUKSWRAND_USE_INITIALSEED
>> diff --git a/lib/ukswrand/Makefile.uk b/lib/ukswrand/Makefile.uk
>> index da85d381..fe6a63e1 100644
>> --- a/lib/ukswrand/Makefile.uk
>> +++ b/lib/ukswrand/Makefile.uk
>> @@ -4,6 +4,7 @@ CINCLUDES-$(CONFIG_LIBUKSWRAND)    += 
>> -I$(LIBUKSWRAND_BASE)/include
>>   CXXINCLUDES-$(CONFIG_LIBUKSWRAND) += -I$(LIBUKSWRAND_BASE)/include
>>     LIBUKSWRAND_SRCS-$(CONFIG_LIBUKSWRAND_MWC) += 
>> $(LIBUKSWRAND_BASE)/mwc.c
>> +LIBUKSWRAND_SRCS-$(CONFIG_LIBUKSWRAND_CHACHA) += 
>> $(LIBUKSWRAND_BASE)/chacha.c
>>   LIBUKSWRAND_SRCS-$(CONFIG_LIBUKSWRAND_DEVFS) += 
>> $(LIBUKSWRAND_BASE)/dev.c
>>   LIBUKSWRAND_SRCS-y += $(LIBUKSWRAND_BASE)/swrand.c
>>   LIBUKSWRAND_SRCS-y += $(LIBUKSWRAND_BASE)/getrandom.c
>> diff --git a/lib/ukswrand/chacha.c b/lib/ukswrand/chacha.c
>> new file mode 100644
>> index 00000000..180e6b73
>> --- /dev/null
>> +++ b/lib/ukswrand/chacha.c
>> @@ -0,0 +1,156 @@
>> +/* SPDX-License-Identifier: BSD-3-Clause */
>> +/*
>> + * Authors: Vlad-Andrei Badoiu <vlad_andrei.badoiu@xxxxxxxxxxxxxxx>
>> + *
>> + * Copyright (c) 2019, University Politehnica of Bucharest. All 
>> rights reserved.
>> + *
>> + * Redistribution and use in source and binary forms, with or without
>> + * modification, are permitted provided that the following conditions
>> + * are met:
>> + *
>> + * 1. Redistributions of source code must retain the above copyright
>> + *    notice, this list of conditions and the following disclaimer.
>> + * 2. Redistributions in binary form must reproduce the above copyright
>> + *    notice, this list of conditions and the following disclaimer 
>> in the
>> + *    documentation and/or other materials provided with the 
>> distribution.
>> + * 3. Neither the name of the copyright holder nor the names of its
>> + *    contributors may be used to endorse or promote products 
>> derived from
>> + *    this software without specific prior written permission.
>> + *
>> + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND 
>> CONTRIBUTORS "AS IS"
>> + * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED 
>> TO, THE
>> + * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A 
>> PARTICULAR PURPOSE
>> + * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR 
>> CONTRIBUTORS BE
>> + * LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
>> + * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
>> + * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR 
>> BUSINESS
>> + * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, 
>> WHETHER IN
>> + * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR 
>> OTHERWISE)
>> + * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF 
>> ADVISED OF THE
>> + * POSSIBILITY OF SUCH DAMAGE.
>> + *
>> + * THIS HEADER MAY NOT BE EXTRACTED OR MODIFIED IN ANY WAY.
>> + */
>> +
>> +#include <string.h>
>> +#include <uk/swrand.h>
>> +#include <uk/print.h>
>> +#include <uk/assert.h>
>> +#include <uk/ctors.h>
>> +
>> +struct uk_swrand {
>> +    int k;
>> +    __u32 input[16], output[16];
>> +};
>> +
>> +struct uk_swrand uk_swrand_def;
>> +
>> +/* This value isn't important, as long as it's sufficiently 
>> asymmetric */
>> +static const char sigma[16] = "expand 32-byte k";
>> +
>> +static void _uk_swrand_ctor(void);
>> +
>> +static inline void _uk_quarterround(__u32 x[16], int a, int b, int 
>> c, int d)
>> +{
>> +    x[a] = x[a] + x[b];
>> +    x[d] = _uk_rotl32(x[d] ^ x[a], 16);
>> +
>> +    x[c] = x[c] + x[d];
>> +    x[b] = _uk_rotl32(x[b] ^ x[c], 12);
>> +
>> +    x[a] = x[a] + x[b];
>> +    x[d] = _uk_rotl32(x[d] ^ x[a], 8);
>> +
>> +    x[c] = x[c] + x[d];
>> +    x[b] = _uk_rotl32(x[b] ^ x[c], 7);
>> +}
>> +
>> +static inline void
>> +_uk_salsa20_wordtobyte(__u32 output[16], const __u32 input[16])
>> +{
>> +    __u32 i;
>> +
>> +    for (i = 0; i < 16; i++)
>> +        output[i] = input[i];
>> +
>> +    for (i = 8; i > 0; i -= 2) {
>> +        _uk_quarterround(output, 0, 4, 8, 12);
>> +        _uk_quarterround(output, 1, 5, 9, 13);
>> +        _uk_quarterround(output, 2, 6, 10, 14);
>> +        _uk_quarterround(output, 3, 7, 11, 15);
>> +        _uk_quarterround(output, 0, 5, 10, 15);
>> +        _uk_quarterround(output, 1, 6, 11, 12);
>> +        _uk_quarterround(output, 2, 7, 8, 13);
>> +        _uk_quarterround(output, 3, 4, 9, 14);
>> +    }
>> +
>> +    for (i = 0; i < 16; i++)
>> +        output[i] += input[i];
>> +}
>> +
>> +static inline void _uk_key_setup(struct uk_swrand *r, __u32 k[8])
>> +{
>> +    int i;
>> +
>> +    for (i = 0; i < 8; i++)
>> +        r->input[i + 4] = k[i];
>> +
>> +    for (i = 0; i < 4; i++)
>> +        r->input[i] = ((__u32 *)sigma)[i];
>> +}
>> +
>> +static inline void _uk_iv_setup(struct uk_swrand *r, __u32 iv[2])
>> +{
>> +    r->input[12] = 0;
>> +    r->input[13] = 0;
>> +    r->input[14] = iv[0];
>> +    r->input[15] = iv[1];
>> +}
>> +
>> +void uk_swrand_init_r(struct uk_swrand *r, __u32 seed __unused)
>> +{
>> +    __u32 i;
>> +
>> +    UK_ASSERT(r);
>> +    /* Initialize chacha */
>> +    __u32 k[8], iv[2];
>> +
>> +    for (i = 0; i < 8; i++)
>> +        k[i] = _get_random_seed32();
>> +
>> +    iv[0] = _get_random_seed32();
>> +    iv[1] = _get_random_seed32();
>> +
>> +    _uk_key_setup(r, k);
>> +    _uk_iv_setup(r, iv);
>> +
>> +    r->k = 16;
>> +}
>> +
>> +__u32 uk_swrand_randr_r(struct uk_swrand *r)
>> +{
>> +    __u32 res;
>> +
>> +    for (;;) {
>> +        _uk_salsa20_wordtobyte(r->output, r->input);
>> +        r->input[12] = r->input[12] + 1;
>> +        if (r->input[12] == 0)
>> +            r->input[13]++;
>> +
>> +        if (r->k < 16) {
>> +            res = r->output[r->k];
>> +            r->k += 1;
>> +            return res;
>> +        }
>> +
>> +        r->k = 0;
>> +    }
>> +}
>> +
>> +static void _uk_swrand_ctor(void)
>> +{
>> +    uk_pr_info("Initialize random number generator...\n");
>> +    uk_swrand_init_r(&uk_swrand_def, 0);
>
> No random seed anymore for ChaCha? ;-)
> In general, you could add two parameters to the API function or change 
> the seed to a u64 value. I would prefer the u64 value.

For chacha we call _get_random_seed32 10 times. In this case I call the 
function directly in uk_swrand_init_r. I could make a separate patch

to change the seed value to u64 since we would also require changes 
specific to arm32. Let me know what you think.

>
>> +}
>> +
>> +UK_CTOR_FUNC(UK_SWRAND_CTOR_PRIO, _uk_swrand_ctor);
>> diff --git a/lib/ukswrand/include/uk/swrand.h 
>> b/lib/ukswrand/include/uk/swrand.h
>> index a3a1e227..7880fda4 100644
>> --- a/lib/ukswrand/include/uk/swrand.h
>> +++ b/lib/ukswrand/include/uk/swrand.h
>> @@ -54,6 +54,11 @@ extern struct uk_swrand uk_swrand_def;
>>   void uk_swrand_init_r(struct uk_swrand *r, __u32 seed);
>>   __u32 uk_swrand_randr_r(struct uk_swrand *r);
>>   +static inline __u32 _uk_rotl32(__u32 v, int c)
>> +{
>> +    return (v << c) | (v >> (32 - c));
>> +}
>> +
>
> Why do you put rotl32 to the header? I think it makes sense if it 
> stays within chacha.c. swrandr.h is a publicly exposed header.
Right.
>
>>   static inline __u32 _get_random_seed32(void)
>>   {
>>       __u32 val;
>>

_______________________________________________
Minios-devel mailing list
Minios-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/minios-devel