Re: [PATCH] memcmp.3: Recast security caveat

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Branden,

On 2/15/23 16:20, G. Branden Robinson wrote:
> Use terminology more carefully.
> 
> * Refer to the info sec property of confidentiality instead of saying,
>   vaguely, "security-critical".
>     https://informationsecurity.wustl.edu/items/\
>     confidentiality-integrity-and-availability-the-cia-triad/
> * Try not to confuse anyone who's studied the analysis of algorithms:
>   don't say "constant time" when "deterministic time" is meant.  The
>   time to perform the memory comparison remains linear (O(n)), not
>   constant (O(1)).
> * Tighten wording.
> 
> Signed-off-by: G. Branden Robinson <g.branden.robinson@xxxxxxxxx>

Thanks for the patch!  Please see some comments below.

Cheers,

Alex

> ---
>  man3/memcmp.3 | 16 ++++++++++++----
>  1 file changed, 12 insertions(+), 4 deletions(-)
> 
> diff --git a/man3/memcmp.3 b/man3/memcmp.3
> index c2427a00a..004556744 100644
> --- a/man3/memcmp.3
> +++ b/man3/memcmp.3
> @@ -67,12 +67,20 @@ POSIX.1-2001, POSIX.1-2008, C99, SVr4, 4.3BSD.
>  .SH NOTES
>  Do not use
>  .BR memcmp ()
> -to compare security critical data, such as cryptographic secrets,
> -because the required CPU time depends on the number of equal bytes.
> -Instead, a function that performs comparisons in constant time is required.
> +to compare confidential data,
> +such as cryptographic secrets;

I'm not sure there should go a semicolon.  If you remove
"such as cryptographic secrets", you could even have the sentence with
no commas, which is a hint that it's a single sentence, I think.

> +because the CPU time required for the comparison depends on the contents
> +of the addresses compared,

Let me be a bit too picky here, since I already have comments for other
parts of the patch.  I think a better break point would be after "comparison".

> +this function is subject to timing-based side-channel attacks.
> +In such cases,
> +a function that performs comparisons in deterministic time,
> +depending only on
> +.I n
> +(the quantity of bytes compared)
> +is required.
>  Some operating systems provide such a function (e.g., NetBSD's
>  .BR consttime_memequal ()),
> -but no such function is specified in POSIX.
> +but none is specified in POSIX.

While for prose one may reasonably want to avoid repeating oneself,
I like boring language in manual pages.  It has less moving parts,
and clearly states to the reader what it's about.  In this specific
case, "no such function" seems very clear to me, and see no
compelling reasons to change it.  If you have them, please clarify.

>  On Linux, it may be necessary to implement such a function oneself.
>  .SH SEE ALSO
>  .BR bstring (3),

-- 
<http://www.alejandro-colomar.es/>
GPG key fingerprint: A9348594CE31283A826FBDD8D57633D441E25BB5

Attachment: OpenPGP_signature
Description: OpenPGP digital signature


[Index of Archives]     [Kernel Documentation]     [Netdev]     [Linux Ethernet Bridging]     [Linux Wireless]     [Kernel Newbies]     [Security]     [Linux for Hams]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux RAID]     [Linux Admin]     [Samba]

  Powered by Linux