* [PATCH setup draft 1/4] Allow validation of signatures using the cygwin key only
2017-12-11 21:41 [PATCH setup draft 0/4] Improve setup.ini validation Ken Brown
2017-12-11 21:41 ` [PATCH setup draft 4/4] If signature validation is turned off, check 'release:' tag Ken Brown
@ 2017-12-11 21:41 ` Ken Brown
2017-12-11 21:41 ` [PATCH setup draft 2/4] Insist on cygwin signing key for official mirrors Ken Brown
2017-12-11 21:41 ` [PATCH setup draft 3/4] Try cygwin signing key for private mirrors Ken Brown
3 siblings, 0 replies; 5+ messages in thread
From: Ken Brown @ 2017-12-11 21:41 UTC (permalink / raw)
To: cygwin-apps
Add an optional argument 'main_key_only' to
crypto.cc:verify_ini_file_sig() and ini.cc:check_ini_sig(). The
argument is 'false' by default. If it is 'true', validation uses the
official cygwin signing key only.
---
crypto.cc | 5 +++--
crypto.h | 3 ++-
ini.cc | 5 +++--
3 files changed, 8 insertions(+), 5 deletions(-)
diff --git a/crypto.cc b/crypto.cc
index 5a10e16..6a97acb 100644
--- a/crypto.cc
+++ b/crypto.cc
@@ -429,7 +429,8 @@ add_key_from_sexpr (gcry_sexp_t key)
/* Verify the signature on an ini file. Takes care of all key-handling. */
bool
-verify_ini_file_sig (io_stream *ini_file, io_stream *ini_sig_file, HWND owner)
+verify_ini_file_sig (io_stream *ini_file, io_stream *ini_sig_file,
+ HWND owner, bool main_key_only)
{
/* DSA public key in s-expr format. */
gcry_sexp_t dsa_key;
@@ -629,7 +630,7 @@ verify_ini_file_sig (io_stream *ini_file, io_stream *ini_sig_file, HWND owner)
// Well, we're actually there! Try it against the main key.
rv = gcry_pk_verify (dsa_sig, dsa_hash, dsa_key);
// If not that, try any supplied on the commandline.
- if (rv != GPG_ERR_NO_ERROR)
+ if (rv != GPG_ERR_NO_ERROR && !main_key_only)
{
std::vector<gcry_sexp_t>::iterator it;
for (it = keys_to_try.begin (); it < keys_to_try.end (); ++it)
diff --git a/crypto.h b/crypto.h
index 860df6c..9725b19 100644
--- a/crypto.h
+++ b/crypto.h
@@ -30,7 +30,8 @@ class io_stream;
file in another. It is called from ini.cc/do_remote_ini() and returns
true if the signature verified OK; if it returns false, you MUST NOT
use the failed ini file - doubly so if it's a compressed stream! */
-extern bool verify_ini_file_sig (io_stream *ini_file, io_stream *ini_sig_file, HWND owner);
+extern bool verify_ini_file_sig (io_stream *ini_file, io_stream *ini_sig_file,
+ HWND owner, bool main_key_only = false);
/*
5.2.2. Version 3 Signature Packet Format
diff --git a/ini.cc b/ini.cc
index f021ed2..18ab2e3 100644
--- a/ini.cc
+++ b/ini.cc
@@ -172,7 +172,8 @@ decompress_ini (io_stream *ini_file)
static io_stream*
check_ini_sig (io_stream* ini_file, io_stream* ini_sig_file,
- bool& sig_fail, const char* site, const char* sig_name, HWND owner)
+ bool& sig_fail, const char* site, const char* sig_name,
+ HWND owner, bool main_key_only = false)
{
/* Unless the NoVerifyOption is set, check the signature for the
current setup and record the result. On a failed signature check
@@ -192,7 +193,7 @@ check_ini_sig (io_stream* ini_file, io_stream* ini_sig_file,
sig_fail = true;
}
}
- else if (!verify_ini_file_sig (ini_file, ini_sig_file, owner))
+ else if (!verify_ini_file_sig (ini_file, ini_sig_file, owner, main_key_only))
{
note (owner, IDS_SIG_INVALID, sig_name, site);
delete ini_sig_file;
--
2.15.1
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH setup draft 4/4] If signature validation is turned off, check 'release:' tag
2017-12-11 21:41 [PATCH setup draft 0/4] Improve setup.ini validation Ken Brown
@ 2017-12-11 21:41 ` Ken Brown
2017-12-11 21:41 ` [PATCH setup draft 1/4] Allow validation of signatures using the cygwin key only Ken Brown
` (2 subsequent siblings)
3 siblings, 0 replies; 5+ messages in thread
From: Ken Brown @ 2017-12-11 21:41 UTC (permalink / raw)
To: cygwin-apps
If we aren't doing signature validation, look instead for "release:
cygwin" in the setup.ini files. If this fails for an official mirror,
reject the mirror. If it fails for a purported private mirror,
silently change the status of the site to "user site" and put a note
in the log file. This change will take effect on the next setup run
or if the user selects 'Back'.
---
ini.cc | 29 ++++++++++++++++++++++++-----
res.rc | 1 +
resource.h | 1 +
3 files changed, 26 insertions(+), 5 deletions(-)
diff --git a/ini.cc b/ini.cc
index 62b7e83..3cffa82 100644
--- a/ini.cc
+++ b/ini.cc
@@ -271,11 +271,15 @@ do_local_ini (HWND owner)
}
static void
-mirror_warn (site_list_type site)
+mirror_warn (site_list_type site, bool sig)
{
- Log (LOG_BABBLE) << "Signature validation failed for " << site.url
- << " using the cygwin key but succeeded using other keys. "
- << endLog;
+ if (sig)
+ Log (LOG_BABBLE) << "Signature validation failed for " << site.url
+ << " using the cygwin key but succeeded using other keys. "
+ << endLog;
+ else
+ Log (LOG_BABBLE) << "setup.ini from " << site.url
+ << " is not from the cygwin release." << endLog;
Log (LOG_BABBLE) << "Changing status from 'mirror' to 'user site'." << endLog;
SiteList::iterator i = find (all_site_list.begin (), all_site_list.end (), site);
if (i != all_site_list.end ())
@@ -343,7 +347,7 @@ do_remote_ini (HWND owner)
if (ini_file)
{
if (n->is_mirror && !main_key_only)
- mirror_warn (*n);
+ mirror_warn (*n, true);
break;
}
}
@@ -369,6 +373,21 @@ do_remote_ini (HWND owner)
}
else
{
+ if (NoVerifyOption && n->is_mirror && aBuilder.release != "cygwin")
+ {
+ if (n->from_mirrors_lst)
+ {
+ // Reject setup.ini.
+ note (owner, IDS_SETUPINI_NOTCYGWIN,
+ SetupBaseName.c_str (), n->url.c_str ());
+ delete ini_file;
+ ini_file = NULL;
+ continue;
+ }
+ else
+ mirror_warn (*n, false);
+ }
+
/* save known-good setup.ini locally */
const std::string fp = "file://" + local_dir + "/" +
rfc1738_escape_part (n->url) +
diff --git a/res.rc b/res.rc
index 14f1109..cd202e0 100644
--- a/res.rc
+++ b/res.rc
@@ -562,6 +562,7 @@ BEGIN
IDS_MIRROR_LST "http://cygwin.com/mirrors.lst"
IDS_ERR_OPEN_WRITE "Can't open %s for writing: %s"
IDS_SETUPINI_MISSING "Unable to get %s from <%s>"
+ IDS_SETUPINI_NOTCYGWIN "%s from <%s> is not from cygwin release"
IDS_OLD_SETUPINI "This setup.ini is older than the one you used last time you installed cygwin. Proceed anyway?"
IDS_NOTHING_INSTALLED "Nothing needed to be installed"
IDS_INSTALL_COMPLETE "Installation Complete"
diff --git a/resource.h b/resource.h
index 79575fb..666e93b 100644
--- a/resource.h
+++ b/resource.h
@@ -39,6 +39,7 @@
#define IDS_ELEVATED 139
#define IDS_INSTALLEDB_VERSION 140
#define IDS_DOWNLOAD_INCOMPLETE_EXIT 141
+#define IDS_SETUPINI_NOTCYGWIN 142
// Dialogs
--
2.15.1
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH setup draft 3/4] Try cygwin signing key for private mirrors
2017-12-11 21:41 [PATCH setup draft 0/4] Improve setup.ini validation Ken Brown
` (2 preceding siblings ...)
2017-12-11 21:41 ` [PATCH setup draft 2/4] Insist on cygwin signing key for official mirrors Ken Brown
@ 2017-12-11 21:41 ` Ken Brown
3 siblings, 0 replies; 5+ messages in thread
From: Ken Brown @ 2017-12-11 21:41 UTC (permalink / raw)
To: cygwin-apps
If validation with the cygwin signing key fails for a purported
private mirror, retry with other supplied keys. If this succeeds,
silently change the status of the site to "user site" and put a note
in the log file. This change will take effect on the next setup run
or if the user selects 'Back'.
This uses a new optional argument 'retry_sig_ok' to
ini.cc:check_ini_sig(). If this is 'true', don't destroy ini_file and
ini_sig_file on a validation failure.
---
ini.cc | 65 +++++++++++++++++++++++++++++++++++++++++++++++++++++++----------
site.h | 2 ++
2 files changed, 57 insertions(+), 10 deletions(-)
diff --git a/ini.cc b/ini.cc
index 4be8263..62b7e83 100644
--- a/ini.cc
+++ b/ini.cc
@@ -18,6 +18,8 @@
flex parsers are provided also. We check to see if this setup.ini
is older than the one we used last time, and if so, warn the user. */
+#include <algorithm>
+
#include "ini.h"
#include "csu_util/rfc1738.h"
@@ -173,7 +175,7 @@ decompress_ini (io_stream *ini_file)
static io_stream*
check_ini_sig (io_stream* ini_file, io_stream* ini_sig_file,
bool& sig_fail, const char* site, const char* sig_name,
- HWND owner, bool main_key_only = false)
+ HWND owner, bool main_key_only = false, bool retry_sig_ok = false)
{
/* Unless the NoVerifyOption is set, check the signature for the
current setup and record the result. On a failed signature check
@@ -195,12 +197,15 @@ check_ini_sig (io_stream* ini_file, io_stream* ini_sig_file,
}
else if (!verify_ini_file_sig (ini_file, ini_sig_file, owner, main_key_only))
{
- note (owner, IDS_SIG_INVALID, sig_name, site);
- delete ini_sig_file;
- ini_sig_file = NULL;
- delete ini_file;
- ini_file = NULL;
sig_fail = true;
+ if (!retry_sig_ok)
+ {
+ note (owner, IDS_SIG_INVALID, sig_name, site);
+ delete ini_sig_file;
+ ini_sig_file = NULL;
+ delete ini_file;
+ ini_file = NULL;
+ }
}
}
return ini_file;
@@ -265,6 +270,27 @@ do_local_ini (HWND owner)
return ini_error;
}
+static void
+mirror_warn (site_list_type site)
+{
+ Log (LOG_BABBLE) << "Signature validation failed for " << site.url
+ << " using the cygwin key but succeeded using other keys. "
+ << endLog;
+ Log (LOG_BABBLE) << "Changing status from 'mirror' to 'user site'." << endLog;
+ SiteList::iterator i = find (all_site_list.begin (), all_site_list.end (), site);
+ if (i != all_site_list.end ())
+ {
+ all_site_list.erase (i);
+ site_list_type newsite (site.url, "", "", "", false, false);
+ all_site_list.push_back (newsite);
+ selected_usersite_list.push_back (newsite);
+ SiteList::iterator j = find (selected_mirror_list.begin (),
+ selected_mirror_list.end (), site);
+ if (j != selected_mirror_list.end ())
+ selected_mirror_list.erase (j);
+ }
+}
+
static bool
do_remote_ini (HWND owner)
{
@@ -293,14 +319,33 @@ do_remote_ini (HWND owner)
ini_sig_file = get_url_to_membuf (current_ini_sig_name, owner);
ini_file = get_url_to_membuf (current_ini_name, owner);
- // Official mirrors must be signed by the cygwin key.
- bool main_key_only = n->from_mirrors_lst;
+ // Mirrors should be signed by the cygwin key.
+ bool main_key_only = n->is_mirror;
+
+ // If this fails for a purported private mirror, allow a
+ // retry with additional keys; if this succeeds, reclassify
+ // the site as a user site.
+ bool retry_sig_ok = n->is_mirror && !n->from_mirrors_lst;
+ retry:
ini_file = check_ini_sig (ini_file, ini_sig_file, sig_fail,
- n->url.c_str (), current_ini_sig_name.c_str (), owner, main_key_only);
+ n->url.c_str (),
+ current_ini_sig_name.c_str (), owner,
+ main_key_only, retry_sig_ok);
+ if (retry_sig_ok && sig_fail)
+ {
+ sig_fail = false;
+ retry_sig_ok = false;
+ main_key_only = false;
+ goto retry;
+ }
// stop searching as soon as we find a setup file
if (ini_file)
- break;
+ {
+ if (n->is_mirror && !main_key_only)
+ mirror_warn (*n);
+ break;
+ }
}
if (ini_file)
ini_file = decompress_ini (ini_file);
diff --git a/site.h b/site.h
index 3525931..5b01829 100644
--- a/site.h
+++ b/site.h
@@ -79,6 +79,8 @@ typedef std::vector <site_list_type> SiteList;
/* user chosen sites */
extern SiteList site_list;
+extern SiteList selected_mirror_list;
+extern SiteList selected_usersite_list;
/* potential sites */
extern SiteList all_site_list;
--
2.15.1
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH setup draft 0/4] Improve setup.ini validation
@ 2017-12-11 21:41 Ken Brown
2017-12-11 21:41 ` [PATCH setup draft 4/4] If signature validation is turned off, check 'release:' tag Ken Brown
` (3 more replies)
0 siblings, 4 replies; 5+ messages in thread
From: Ken Brown @ 2017-12-11 21:41 UTC (permalink / raw)
To: cygwin-apps
This patch series presupposes the one posted starting at [1].
Currently, signatures are verified using the cygwin signing key and
other keys supplied by the user. Validation with any key is accepted.
This patch series makes the following changes:
- For official cygwin mirrors (those listed in mirrors.lst), only the
cygwin key is tried.
- For purported private mirrors (from the "last-mirror" user setting
or the "Add URL box" or the command line), the cygwin key is tried
first. If this fails, then the remaining keys are tried. If one
of these succeeds, then the site is silently reclassified (with a
message in the log file) as a 'user site' rather than a 'mirror'.
The change takes effect on the next setup run or when the user
selects 'Back'.
- If the user turns off signature validation with the -X option, a
weaker check is done: We look for "release: cygwin" in the
setup.ini file. If that fails for an official mirror, the file is
rejected. If it fails for a purported private mirror, the site is
silently reclassified, as above.
The reclassification is done silently because it could easily be
necessary, through no fault of the user. There are three reasons for
this. First, the distinction between mirrors and user sites is new,
and it will take time for users to become accustomed to it. Second,
for setup.rc files that were written before [1], "last-mirror"
includes all selected sites, whether mirrors or not. Finally, sites
specified on the command line are initially assumed to be mirrors
until we can perform the above checks.
[1] https://sourceware.org/ml/cygwin-apps/2017-12/msg00051.html
Ken Brown (4):
Allow validation of signatures using the cygwin key only
Insist on cygwin signing key for official mirrors
Try cygwin signing key for private mirrors
If signature validation is turned off, check 'release:' tag
crypto.cc | 5 ++--
crypto.h | 3 ++-
ini.cc | 87 +++++++++++++++++++++++++++++++++++++++++++++++++++++++-------
res.rc | 1 +
resource.h | 1 +
site.h | 2 ++
6 files changed, 87 insertions(+), 12 deletions(-)
--
2.15.1
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH setup draft 2/4] Insist on cygwin signing key for official mirrors
2017-12-11 21:41 [PATCH setup draft 0/4] Improve setup.ini validation Ken Brown
2017-12-11 21:41 ` [PATCH setup draft 4/4] If signature validation is turned off, check 'release:' tag Ken Brown
2017-12-11 21:41 ` [PATCH setup draft 1/4] Allow validation of signatures using the cygwin key only Ken Brown
@ 2017-12-11 21:41 ` Ken Brown
2017-12-11 21:41 ` [PATCH setup draft 3/4] Try cygwin signing key for private mirrors Ken Brown
3 siblings, 0 replies; 5+ messages in thread
From: Ken Brown @ 2017-12-11 21:41 UTC (permalink / raw)
To: cygwin-apps
If a mirror comes from mirrors.lst, validate the signature using the
cygwin signing key only.
---
ini.cc | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/ini.cc b/ini.cc
index 18ab2e3..4be8263 100644
--- a/ini.cc
+++ b/ini.cc
@@ -292,8 +292,12 @@ do_remote_ini (HWND owner)
current_ini_sig_name = current_ini_name + ".sig";
ini_sig_file = get_url_to_membuf (current_ini_sig_name, owner);
ini_file = get_url_to_membuf (current_ini_name, owner);
+
+ // Official mirrors must be signed by the cygwin key.
+ bool main_key_only = n->from_mirrors_lst;
ini_file = check_ini_sig (ini_file, ini_sig_file, sig_fail,
- n->url.c_str (), current_ini_sig_name.c_str (), owner);
+ n->url.c_str (), current_ini_sig_name.c_str (), owner, main_key_only);
+
// stop searching as soon as we find a setup file
if (ini_file)
break;
--
2.15.1
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2017-12-11 21:41 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-12-11 21:41 [PATCH setup draft 0/4] Improve setup.ini validation Ken Brown
2017-12-11 21:41 ` [PATCH setup draft 4/4] If signature validation is turned off, check 'release:' tag Ken Brown
2017-12-11 21:41 ` [PATCH setup draft 1/4] Allow validation of signatures using the cygwin key only Ken Brown
2017-12-11 21:41 ` [PATCH setup draft 2/4] Insist on cygwin signing key for official mirrors Ken Brown
2017-12-11 21:41 ` [PATCH setup draft 3/4] Try cygwin signing key for private mirrors Ken Brown
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).