Received-SPF: pass (google.com: domain of linux-crypto-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Received-SPF: Pass (protection.outlook.com: domain of arm.com designates
 63.35.35.123 as permitted sender) receiver=protection.outlook.com;
 client-ip=63.35.35.123; helo=64aa7808-outbound-1.mta.getcheckrecipient.com;
 pr=C
Authentication-Results-Original: dkim=none (message not signed)
 header.d=none;dmarc=none action=none header.from=arm.com;
Date:   Thu, 28 Jul 2022 11:29:30 +0100
From:   Szabolcs Nagy <szabolcs.nagy@arm.com>
To:     "Jason A. Donenfeld" <Jason@zx2c4.com>
Cc:     libc-alpha@sourceware.org, adhemerval.zanella@linaro.org,
        Florian Weimer <fweimer@redhat.com>,
        Eric Biggers <ebiggers@kernel.org>,
        linux-crypto@vger.kernel.org
Subject: Re: [PATCH v6] arc4random: simplify design for better safety
Message-ID: <YuJlCijOtAXOco6h@arm.com>
References: <20220726190830.1189339-1-Jason@zx2c4.com>
 <20220726195822.1223048-1-Jason@zx2c4.com>
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <20220726195822.1223048-1-Jason@zx2c4.com>
MIME-Version: 1.0
NoDisclaimer: true
X-Microsoft-Antispam-Message-Info-Original: nRHL+yu2mrba/skE3P3f9liJxqJkdWMWxYPCRpY5mnxZi4uR4B1aStxnyfSJu+wWAtooXl8E1NSu5DQTsmSBaPWdmoAGO3wnIK7wob+6pMsjxYwcacg+aLeu1xeaa0AvI1bwTQMb9LZpmfr6OlEsqcEEZ8f0SEyLM3wd1qqKK/VEZyeTYEQra75/Ed1bXBx1LOkE9ygqwzW3OjxQyH+Ju5blILMkgq3NBzl6/rj5gUQW0MB+UoXT7aw+po3Q2KPMSRci6B9D8o0sXWXqUKZzA60Ec9ZuVIxAlT2gvTcbjqKuBCtmwwWgO3COZk3GM14Cbv3FZliObOK1Cu3fyjOyhbajMc4Kk/Ufct9zkGF0cBmpu4b/BtyKkjK6R8MRLKg8achMp6UW1cvu0mUwqi76rE1tVm3dGuSCUL6W7PhYLsWc01mjYepLq4XyTXdUntHL6aDN3hFstG4jke8z2Mo40Weq5U5XqgeBfF6qA/7GOTjnMYgLQk4BifQVrYgozQ8p0kWYd0b7Ld2PxygqQxo2stqMetcAyRBEBp4MGKfhmuw4HmKgLXa0W5LVoiwizd8Vu14wZxY/iK9TedvD6CW+d1t6HdEYhfkKnDvF2B4pwHitzn22zYXjemavszTZB9qeCB/WlfnFx5xBRuX3rZXzRT1NHHRYFMMJqiEyP1Wzz1XvLMokSzIRsLsQvVxn1Hfb5o9eXmQX+UitrVCBHA+Twvjj+C49pUjGxLiIYcGgPXjFSqwQa10uEEIJqkbabzdAkdnLxjRIjvdXoe6rPhHayatzIR7eJyl2d/jTcE7FIXk=
X-Forefront-Antispam-Report-Untrusted: CIP:255.255.255.255;CTRY:;LANG:en;SCL:1;SRV:;IPV:NLI;SFV:NSPM;H:DB9PR08MB7179.eurprd08.prod.outlook.com;PTR:;CAT:NONE;SFS:(13230016)(4636009)(136003)(376002)(396003)(39860400002)(366004)(346002)(36756003)(5660300002)(66556008)(66476007)(6486002)(4326008)(66946007)(186003)(44832011)(86362001)(8936002)(6512007)(54906003)(41300700001)(38100700002)(8676002)(6506007)(2616005)(6916009)(478600001)(83380400001)(6666004)(2906002)(66574015)(316002)(26005)(32563001);DIR:OUT;SFP:1101;
X-MS-Exchange-Transport-CrossTenantHeadersStamped: AM9PR08MB6292
Original-Authentication-Results: dkim=none (message not signed)
 header.d=none;dmarc=none action=none header.from=arm.com;
X-EOPAttributedMessage: 0
X-MS-Exchange-Transport-CrossTenantHeadersStripped: DBAEUR03FT020.eop-EUR03.prod.protection.outlook.com
X-MS-PublicTrafficType: Email
X-MS-Office365-Filtering-Correlation-Id-Prvs: 747baaac-3248-4974-554f-08da70841646
X-Microsoft-Antispam: BCL:0;
X-Microsoft-Antispam-Message-Info: LSq1AT6pu3C/nizX/bh+kn/3nG48GkvPpeN+IcjQOy0nWRLGb5mR0gS3D+TgQ0tzqOr6esiVnNs1KItk6MTjjUZml0hkzq8cm5iybYfUt60GLEXU8Vjhg3R8I3b42fw6CcToF/rtWuh/RHEY19ArQ2BB8uJZ/gnSZ7KKbZDjp+JIIGAXTRxuc+eazbP6hWZBMa5hFVnbf1YSDsWakGuhsJB+eQJzM+sGjemHwhRtvNgpd/pTFlwCrIErWPSLQs+IAQf2I9jCna6H1pjvGb6IVZqh23Y0KgP9nOyd8wl+XfyilrbWzKRRDuWrPnhjZWwj/WYgFgzrwuleCbPO+ObJfGBGxw3vnfHvExeY36RBp8Ky31Hx0TfgLf+dHR6MRtHVcW1A41fZxvPacBMM0sHyLzdyOep3yw8C2etpDMaj6Arn8WGxFF/Z6YMdCD1pKec3BU7ss9IM+wU8pPUptgE9vExY0D8fbaVMswKtx0Fho0mOKEMmNiec7P95TP2jntqt2mNwgk0tkyVfnPFVShmqp7HBlFopzrSjyalfha4tbaVWmcmoHdJ6gIyApr24Sao7PyT8yaPB+k4BtOa9nnkTF+kM+iM4yuz5xwvfiwZCLwRJln7yYpK8OrRXZgf0fldsSiV92G47Pk0MNAo5k4pU1ojfsxKUMDNotkPpUVa3k9kxyWGjJBF/eRpSwzsVUQvRJBzhs8p/4mrT8r5PXZFrkBV6Oyv+xDGCf4ZK5e4WwB+Ooc+agmsAtZpt8+fyXjn3vLxzHArQlGiAAv+0Er5fI4Mo1ysOellPzFfFCv9FIYVKLckx3A1LWpaQdJbzVutRmAWK+3UFchfVYyx+scG5cg==
X-Forefront-Antispam-Report: CIP:63.35.35.123;CTRY:IE;LANG:en;SCL:1;SRV:;IPV:CAL;SFV:NSPM;H:64aa7808-outbound-1.mta.getcheckrecipient.com;PTR:ec2-63-35-35-123.eu-west-1.compute.amazonaws.com;CAT:NONE;SFS:(13230016)(4636009)(346002)(39860400002)(396003)(376002)(136003)(40470700004)(36840700001)(46966006)(6862004)(8936002)(5660300002)(2906002)(8676002)(47076005)(316002)(86362001)(40480700001)(36756003)(70206006)(40460700003)(82310400005)(70586007)(54906003)(4326008)(356005)(6506007)(82740400003)(6666004)(6512007)(6486002)(26005)(2616005)(44832011)(66574015)(83380400001)(41300700001)(336012)(36860700001)(81166007)(186003)(478600001)(32563001);DIR:OUT;SFP:1101;
X-OriginatorOrg: arm.com
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 28 Jul 2022 10:29:53.6897
 (UTC)
X-MS-Exchange-CrossTenant-Network-Message-Id: c2c2b2f2-dfe3-4a17-09e4-08da70841c2d
X-MS-Exchange-CrossTenant-Id: f34e5979-57d9-4aaa-ad4d-b122a662184d
X-MS-Exchange-CrossTenant-OriginalAttributedTenantConnectingIp: TenantId=f34e5979-57d9-4aaa-ad4d-b122a662184d;Ip=[63.35.35.123];Helo=[64aa7808-outbound-1.mta.getcheckrecipient.com]
X-MS-Exchange-CrossTenant-AuthSource: DBAEUR03FT020.eop-EUR03.prod.protection.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Anonymous
X-MS-Exchange-CrossTenant-FromEntityHeader: HybridOnPrem
X-MS-Exchange-Transport-CrossTenantHeadersStamped: VI1PR08MB4189
X-Spam-Status: No, score=-0.9 required=5.0 tests=BAYES_00,DKIM_SIGNED,
        DKIM_VALID,FORGED_SPF_HELO,RCVD_IN_DNSWL_NONE,RCVD_IN_MSPIKE_H2,
        SPF_HELO_PASS,SPF_NONE,UNPARSEABLE_RELAY autolearn=no
        autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-crypto.vger.kernel.org>
X-Mailing-List: linux-crypto@vger.kernel.org

The 07/26/2022 21:58, Jason A. Donenfeld via Libc-alpha wrote:
> Rather than buffering 16 MiB of entropy in userspace (by way of
> chacha20), simply call getrandom() every time.
> 
> This approach is doubtlessly slower, for now, but trying to prematurely
> optimize arc4random appears to be leading toward all sorts of nasty
> properties and gotchas. Instead, this patch takes a much more
> conservative approach. The interface is added as a basic loop wrapper
> around getrandom(), and then later, the kernel and libc together can
> work together on optimizing that.
> 
> This prevents numerous issues in which userspace is unaware of when it
> really must throw away its buffer, since we avoid buffering all
> together. Future improvements may include userspace learning more from
> the kernel about when to do that, which might make these sorts of
> chacha20-based optimizations more possible. The current heuristic of 16
> MiB is meaningless garbage that doesn't correspond to anything the
> kernel might know about. So for now, let's just do something
> conservative that we know is correct and won't lead to cryptographic
> issues for users of this function.
> 
> This patch might be considered along the lines of, "optimization is the
> root of all evil," in that the much more complex implementation it
> replaces moves too fast without considering security implications,
> whereas the incremental approach done here is a much safer way of going
> about things. Once this lands, we can take our time in optimizing this
> properly using new interplay between the kernel and userspace.
> 
> getrandom(0) is used, since that's the one that ensures the bytes
> returned are cryptographically secure. But on systems without it, we
> fallback to using /dev/urandom. This is unfortunate because it means
> opening a file descriptor, but there's not much of a choice. Secondly,
> as part of the fallback, in order to get more or less the same
> properties of getrandom(0), we poll on /dev/random, and if the poll
> succeeds at least once, then we assume the RNG is initialized. This is a
> rough approximation, as the ancient "non-blocking pool" initialized
> after the "blocking pool", not before, and it may not port back to all
> ancient kernels, though it does to all kernels supported by glibc
> (≥3.2), so generally it's the best approximation we can do.
> 
> The motivation for including arc4random, in the first place, is to have
> source-level compatibility with existing code. That means this patch
> doesn't attempt to litigate the interface itself. It does, however,
> choose a conservative approach for implementing it.
> 
> Cc: Adhemerval Zanella Netto <adhemerval.zanella@linaro.org>
> Cc: Florian Weimer <fweimer@redhat.com>
> Cc: Cristian Rodríguez <crrodriguez@opensuse.org>
> Cc: Paul Eggert <eggert@cs.ucla.edu>
> Cc: Mark Harris <mark.hsj@gmail.com>
> Cc: Eric Biggers <ebiggers@kernel.org>
> Cc: linux-crypto@vger.kernel.org
> Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>

fyi, after this patch i see

FAIL: stdlib/tst-arc4random-thread

with

$ cat stdlib/tst-arc4random-thread.out
info: arc4random: minimum of 1750000 blob results expected
info: arc4random: 1750777 blob results observed
info: arc4random_buf: minimum of 1750000 blob results expected
info: arc4random_buf: 1750000 blob results observed
info: arc4random_uniform: minimum of 1750000 blob results expected
Timed out: killed the child process
Termination time: 2022-07-27T14:41:33.766791947
Last write to standard output: 2022-07-27T14:41:22.522497854

on an arm and aarch64 builder.

running it manually it takes >30s to complete.

> ---
>  LICENSES                                      |  23 -
>  NEWS                                          |   4 +-
>  include/stdlib.h                              |   3 -
>  manual/math.texi                              |  13 +-
>  stdlib/Makefile                               |   2 -
>  stdlib/arc4random.c                           | 196 ++----
>  stdlib/arc4random.h                           |  48 --
>  stdlib/chacha20.c                             | 191 ------
>  stdlib/tst-arc4random-chacha20.c              | 167 -----
>  sysdeps/aarch64/Makefile                      |   4 -
>  sysdeps/aarch64/chacha20-aarch64.S            | 314 ----------
>  sysdeps/aarch64/chacha20_arch.h               |  40 --
>  sysdeps/generic/chacha20_arch.h               |  24 -
>  sysdeps/generic/not-cancel.h                  |   3 +
>  sysdeps/generic/tls-internal-struct.h         |   1 -
>  sysdeps/generic/tls-internal.c                |  10 -
>  sysdeps/mach/hurd/_Fork.c                     |   2 -
>  sysdeps/mach/hurd/not-cancel.h                |   4 +
>  sysdeps/nptl/_Fork.c                          |   2 -
>  .../powerpc/powerpc64/be/multiarch/Makefile   |   4 -
>  .../powerpc64/be/multiarch/chacha20-ppc.c     |   1 -
>  .../powerpc64/be/multiarch/chacha20_arch.h    |  42 --
>  sysdeps/powerpc/powerpc64/power8/Makefile     |   5 -
>  .../powerpc/powerpc64/power8/chacha20-ppc.c   | 256 --------
>  .../powerpc/powerpc64/power8/chacha20_arch.h  |  37 --
>  sysdeps/s390/s390-64/Makefile                 |   6 -
>  sysdeps/s390/s390-64/chacha20-s390x.S         | 573 ------------------
>  sysdeps/s390/s390-64/chacha20_arch.h          |  45 --
>  sysdeps/unix/sysv/linux/not-cancel.h          |   8 +-
>  sysdeps/unix/sysv/linux/tls-internal.c        |  10 -
>  sysdeps/unix/sysv/linux/tls-internal.h        |   1 -
>  sysdeps/x86_64/Makefile                       |   7 -
>  sysdeps/x86_64/chacha20-amd64-avx2.S          | 328 ----------
>  sysdeps/x86_64/chacha20-amd64-sse2.S          | 311 ----------
>  sysdeps/x86_64/chacha20_arch.h                |  55 --
>  35 files changed, 64 insertions(+), 2676 deletions(-)
>  delete mode 100644 stdlib/arc4random.h
>  delete mode 100644 stdlib/chacha20.c
>  delete mode 100644 stdlib/tst-arc4random-chacha20.c
>  delete mode 100644 sysdeps/aarch64/chacha20-aarch64.S
>  delete mode 100644 sysdeps/aarch64/chacha20_arch.h
>  delete mode 100644 sysdeps/generic/chacha20_arch.h
>  delete mode 100644 sysdeps/powerpc/powerpc64/be/multiarch/Makefile
>  delete mode 100644 sysdeps/powerpc/powerpc64/be/multiarch/chacha20-ppc.c
>  delete mode 100644 sysdeps/powerpc/powerpc64/be/multiarch/chacha20_arch.h
>  delete mode 100644 sysdeps/powerpc/powerpc64/power8/chacha20-ppc.c
>  delete mode 100644 sysdeps/powerpc/powerpc64/power8/chacha20_arch.h
>  delete mode 100644 sysdeps/s390/s390-64/chacha20-s390x.S
>  delete mode 100644 sysdeps/s390/s390-64/chacha20_arch.h
>  delete mode 100644 sysdeps/x86_64/chacha20-amd64-avx2.S
>  delete mode 100644 sysdeps/x86_64/chacha20-amd64-sse2.S
>  delete mode 100644 sysdeps/x86_64/chacha20_arch.h