On Tue, Mar 16, 2021 at 11:39 PM Brad House <brad_at_brad-house.com> wrote:
>
> On 3/16/21 3:57 PM, Alexey Tikhonov wrote:
> > Hello,
> >
> > commit [1] introduced an embedded implementation of RC4 cipher to
> > generate secure transaction IDs:
> > ```
> > the implementation to use a more secure way of generating
> > unique IDs. It starts by obtaining a key with reasonable
> > entropy which is used with an RC4 stream to generate
> > the cryptographically secure transaction IDs.
> > ```
> >
> > Besides the general rule “don’t roll your own crypto”, RC4 is just
> > broken and can't really be considered as a CSPRNG.
> >
> > I wonder are there any particular reasons to use RC4 instead of random
> > IDs generated
> > (1) either with the help of a function similar to the function
> > currently used to generate the initial key (`randomize_key()`[2]),
> > (2) or with the help of corresponding function from one of well known
> > crypto libraries (for example, `RAND_bytes()` from OpenSSL)?
> >
> > (1) wouldn't introduce new dependencies but might have performance
> > impact. Is this important / reason behind RC4 usage?
> >
> >
> >
> > [1] https://github.com/c-ares/c-ares/commit/7a77f24d26f755421b91a0ea3852b6348d637861
> > [2] https://github.com/c-ares/c-ares/blob/master/src/lib/ares_init.c#L2504
> >
>
> This isn't encrypting data, its generating a pseudo-random DNS
> transaction id (only 16bits!), so doesn't have the same strength
> requirements as an actual stream cipher would. arc4random was a fairly
> common method to generate pseudo random data, infact its still used
> quite a bit but often recommended to discard the first 3k of generated
> data due to known bias. Attackers are more likely to just flood data
> rather than actually try to guess the transaction id due to the limited
> range.
>
> We definitely wouldn't want to pull something like OpenSSL in as a
> dependency for c-ares as that would be overkill and reduce portability.
>
> I wouldn't necessarily be against pulling in a stronger PRNG, but I'm
> also not entirely sure there is any actual security benefit.
I was thinking about reusing existing `randomize_key()` function to
generate every ID but I don't have anything better than "many auditors
are not ok finding RC4 in the code even if it is not used for
encryption" to back my proposal up.
And I guess that's not very convincing for you.
In this case, would you be ok with the introduction of ./configure
option that would allow to compile out RC4 (replacing it with
`randomize_key()` usage)? This option could default to keep using RC4.
> Do you have any evidence or test case where this has been proven to be a
> weak point in c-ares? If so, then by all means we can and should
> address it.
>
> -Brad
>
>
Received on 2021-03-18