Re: [PATCH] Add FAQ entry on how Cygwin counters man-in-the-middle (MITM) attacks

[Corinna Vinschen <corinna-cygwin at cygwin dot com>]

On Mar 31 14:08, David A. Wheeler wrote:
> Signed-off-by: David A. Wheeler <dwheeler@dwheeler.com>
> ---
>  winsup/doc/faq-setup.xml | 129 ++++++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 128 insertions(+), 1 deletion(-)

Ok, it's review time.

First things first, a patch should come with a plain text ChangeLog
entry.  See the ChangeLog file in winsup/doc for examples.  Don't add
the ChangeLog entry to the diffs, just add it verbatim to the mail.

> diff --git a/winsup/doc/faq-setup.xml b/winsup/doc/faq-setup.xml
> index 614d4a9..3764214 100644
> --- a/winsup/doc/faq-setup.xml
> +++ b/winsup/doc/faq-setup.xml
> @@ -156,6 +156,128 @@ and that installing the older version will not help improve Cygwin.
>  </para>
>  </answer></qandaentry>
>  
> +<qandaentry id="faq.setup.mitm">
> +<question><para>How does Cygwin counter man-in-the-middle (MITM) attacks during installation and upgrade?</para></question>
> +<answer>

The title is too specific, IMHO.  What about something along the lines
of "How Cygwin secures the installation process"?

> +<para>
> +A man-in-the-middle (MITM) attack occurs when an attacker secretly relays and
> +possibly alters the communication between two parties
> +who believe they are directly communicating with each other.
> +Here is how Cygwin counters man-in-the-middle (MITM) attacks
> +during installation and update (including enough details so
> +technical people can confirm it):
> +</para>

I would drop this para.  Just refer to 
https://en.wikipedia.org/wiki/Man-in-the-middle_attack
at some convenient point in the following para.

> +[...]
> +<para>
> +Up through 2015 Cygwin used the MD5 algorithm for cryptographic hashes.
> +Cygwin used both MD5 and length checks, which makes some attacks harder
> +than if Cygwin used only MD5,
> +but MD5 is no longer considered a secure cryptographic hash algorithm.
> +The 2015-02-06 update of the setup program
> +added support for the SHA-512 cryptographic hash algorithm for
> +sigining the <literal>setup.ini</literal> package list, as described in
> +<ulink url="https://cygwin.com/ml/cygwin/2015-02/msg00093.html"/>.
> +The announcement also noted that there will be a switch to SHA-512
> +checksums in the <literal>setup.ini</literal> files.
> +There are no known practical exploits of SHA-512 (SHA-512 is part of the
> +widely-used SHA-2 suite of cryptographic hashes).
> +</para>
> +</answer></qandaentry>

We already switched to sha512, so you can skip the entire MD5
consideration.  Just describe the sha512 checking.

All in all the text looks good to me.  You're not interested to improve
other parts of the documentation as well, by any chance? :)


Thanks,
Corinna

-- 
Corinna Vinschen                  Please, send mails regarding Cygwin to
Cygwin Maintainer                 cygwin AT cygwin DOT com
Red Hat

Attachment: pgpjtcEDVWtnu.pgp
Description: PGP signature