Bug#749795: apt: no authentication checks for source packages

2014-06-18 Thread Michael Vogt
On Mon, Jun 16, 2014 at 11:18:27AM +0200, Jakub Wilk wrote:
 * Michael Vogt m...@debian.org, 2014-06-16, 09:35:
 +   _error-Warning(_(The data from '%s' is not signed. All packages from 
 + that repository can not be authenticated.),
 
 s/can not/cannot/
 
 Also, All with a negated verb sounds awkward to me (but that may
 be due to my non-native-englishness). How about:
 
 No packages from that repository can be authenticated.
 
 or simply
 
 Packages from that repository cannot be authenticated.
 
 ?

I like the second string, I updated my branch. Thanks!

Cheers,
 Michael


-- 
To UNSUBSCRIBE, email to debian-bugs-rc-requ...@lists.debian.org
with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org



Bug#749795: apt: no authentication checks for source packages

2014-06-16 Thread Michael Vogt
On Fri, May 30, 2014 at 03:21:20PM +0200, Michael Vogt wrote:
[..]
  Hmm. There is no warning suggesting that anything fishy is going on,
  and the exit code indicates success. (Perhaps the Igns could raise
  suspicion of an observant sysadmin. But who knows what Ign exactly
  means? At least the apt-get(1) manpage doesn't know.)
 
 Right, I think apt should show a more prominent warning here. I will
 look into this next.
[..]

I create a git branch that shows a warning if it comes accross a
unauthenticated repository:

+   _error-Warning(_(The data from '%s' is not signed. All packages from 
+ that repository can not be authenticated.),
+   MetaIndexURIDesc.c_str());


I think for the future we actually should not allow a apt-get update
of untrusted repos without --allow-unauthenticated  or
[trusted=no]. But this will probably break some setups so we need to
be careful and not rush it.



Cheers,
 Michael


-- 
To UNSUBSCRIBE, email to debian-bugs-rc-requ...@lists.debian.org
with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org



Bug#749795: apt: no authentication checks for source packages

2014-06-16 Thread Jakub Wilk

* Michael Vogt m...@debian.org, 2014-06-16, 09:35:

+   _error-Warning(_(The data from '%s' is not signed. All packages from 
+ that repository can not be authenticated.),


s/can not/cannot/

Also, All with a negated verb sounds awkward to me (but that may be 
due to my non-native-englishness). How about:


No packages from that repository can be authenticated.

or simply

Packages from that repository cannot be authenticated.

?

--
Jakub Wilk


--
To UNSUBSCRIBE, email to debian-bugs-rc-requ...@lists.debian.org
with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org



Bug#749795: apt: no authentication checks for source packages

2014-06-16 Thread Christoph Anton Mitterer
On Mon, 2014-06-16 at 09:35 +0200, Michael Vogt wrote: 
 I think for the future we actually should not allow a apt-get update
 of untrusted repos without --allow-unauthenticated  or
 [trusted=no]. But this will probably break some setups so we need to
 be careful and not rush it.

And what about the setups, which assume secure data to be retrieved (as
far as I can see the whole build stack of Debian), which is already
broken now?

Security is much more critical here then things continuing to work... if
someone's setup really depend on not verifying integrity... he will
immediately notice (and can add the flag),... but no one notices if his
security is compromised by MitMs... :-(


So I see not much of a reason to not implement that right away.


Cheers,
Chris.


-- 
To UNSUBSCRIBE, email to debian-bugs-rc-requ...@lists.debian.org
with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org



Bug#749795: apt: no authentication checks for source packages

2014-06-16 Thread Michael Vogt
On Mon, Jun 16, 2014 at 02:58:28PM +0200, Christoph Anton Mitterer wrote:
 On Mon, 2014-06-16 at 09:35 +0200, Michael Vogt wrote: 
  I think for the future we actually should not allow a apt-get update
  of untrusted repos without --allow-unauthenticated  or
  [trusted=no]. But this will probably break some setups so we need to
  be careful and not rush it.
 
 And what about the setups, which assume secure data to be retrieved (as
 far as I can see the whole build stack of Debian), which is already
 broken now?
 
 Security is much more critical here then things continuing to work... if
 someone's setup really depend on not verifying integrity... he will
 immediately notice (and can add the flag),... but no one notices if his
 security is compromised by MitMs... :-(

 So I see not much of a reason to not implement that right away.

Absolutely, security is (much!) more important.

However with the fix that recently went into -security apt-get source
foo will fail if foo comes from a not-authenticated source. What I
wrote above is about not allowing apt-get update at all for unsigned
repositories (unless --allow-unauthenticated is used). But maybe you
are right and the warning that I added to git should be a error that
tells the user to use --allow-unauthenticated if he/she really wants
to use a repository that we can not authenticate.

Cheers,
 Michael
 


-- 
To UNSUBSCRIBE, email to debian-bugs-rc-requ...@lists.debian.org
with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org



Bug#749795: apt: no authentication checks for source packages

2014-06-12 Thread Thijs Kinkhorst
Hi,

 apt: no authentication checks for source packages

The Debian security team has assigned CVE-2014-0478 to this issue.

APT developers: we should fix this in wheezy. Are you able to provide an
update for wheezy for this issue?

As for squeeze, if it's not too much extra work it would be great if an
update for squeeze was also possible. Perhaps it could also even include
the fix for https://security-tracker.debian.org/tracker/CVE-2011-3634?

Let us know.

Thanks,
Thijs


-- 
To UNSUBSCRIBE, email to debian-bugs-rc-requ...@lists.debian.org
with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org



Bug#749795: apt: no authentication checks for source packages

2014-06-12 Thread Michael Vogt
On Thu, Jun 12, 2014 at 11:44:20AM +0200, Thijs Kinkhorst wrote:
 Hi,
 
  apt: no authentication checks for source packages
 
 The Debian security team has assigned CVE-2014-0478 to this issue.
 
 APT developers: we should fix this in wheezy. Are you able to provide an
 update for wheezy for this issue?

Yes, I will work on a backport for this today.

 As for squeeze, if it's not too much extra work it would be great if an
 update for squeeze was also possible. Perhaps it could also even include
 the fix for https://security-tracker.debian.org/tracker/CVE-2011-3634?

I look into this too, I don't know yet how much extra work it is.

Cheers,
 Michael


-- 
To UNSUBSCRIBE, email to debian-bugs-rc-requ...@lists.debian.org
with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org



Bug#749795: apt: no authentication checks for source packages

2014-06-12 Thread Michael Vogt
On Thu, Jun 12, 2014 at 11:44:20AM +0200, Thijs Kinkhorst wrote:
  apt: no authentication checks for source packages
 
 The Debian security team has assigned CVE-2014-0478 to this issue.
 
 APT developers: we should fix this in wheezy. Are you able to provide an
 update for wheezy for this issue?
[..]

Attached is the fix for wheezy with a regression test, a additional
test run is very welcome (works in my wheezy container both the
testcase and a manual test when removing /var/lib/apt/lists/*Release*).

Cheers,
 Michael
diff -Nru apt-0.9.7.9+deb7u1/cmdline/apt-get.cc 
apt-0.9.7.9+deb7u2/cmdline/apt-get.cc
--- apt-0.9.7.9+deb7u1/cmdline/apt-get.cc   2013-03-01 11:51:21.0 
+0100
+++ apt-0.9.7.9+deb7u2/cmdline/apt-get.cc   2014-06-12 13:35:58.0 
+0200
@@ -1046,25 +1046,8 @@
return true;
 }
/*}}}*/
-// CheckAuth - check if each download comes form a trusted source  /*{{{*/
-// -
-/* */
-static bool CheckAuth(pkgAcquire Fetcher)
+static bool AuthPrompt(std::string UntrustedList, bool const PromptUser)
 {
-   string UntrustedList;
-   for (pkgAcquire::ItemIterator I = Fetcher.ItemsBegin(); I  
Fetcher.ItemsEnd(); ++I)
-   {
-  if (!(*I)-IsTrusted())
-  {
- UntrustedList += string((*I)-ShortDesc()) +  ;
-  }
-   }
-
-   if (UntrustedList == )
-   {
-  return true;
-   }
-
ShowList(c2out,_(WARNING: The following packages cannot be 
authenticated!),UntrustedList,);
 
if (_config-FindB(APT::Get::AllowUnauthenticated,false) == true)
@@ -1073,6 +1056,9 @@
   return true;
}
 
+   if (PromptUser == false)
+ return _error-Error(_(Some packages could not be authenticated));
+
if (_config-FindI(quiet,0)  2
 _config-FindB(APT::Get::Assume-Yes,false) == false)
{
@@ -1090,6 +1076,27 @@
return _error-Error(_(There are problems and -y was used without 
--force-yes));
 }
/*}}}*/
+// CheckAuth - check if each download comes form a trusted source  /*{{{*/
+// -
+/* */
+static bool CheckAuth(pkgAcquire Fetcher, bool PromptUser=true)
+{
+   string UntrustedList;
+   for (pkgAcquire::ItemIterator I = Fetcher.ItemsBegin(); I  
Fetcher.ItemsEnd(); ++I)
+   {
+  if (!(*I)-IsTrusted())
+  {
+ UntrustedList += string((*I)-ShortDesc()) +  ;
+  }
+   }
+
+   if (UntrustedList == )
+   {
+  return true;
+   }
+
+   return AuthPrompt(UntrustedList, PromptUser);
+}
 // InstallPackages - Actually download and install the packages
/*{{{*/
 // -
 /* This displays the informative messages describing what is going to 
@@ -2483,6 +2490,7 @@
 
// Load the requestd sources into the fetcher
unsigned J = 0;
+   std::string UntrustedList;
for (const char **I = CmdL.FileList + 1; *I != 0; I++, J++)
{
   string Src;
@@ -2492,6 +2500,9 @@
 delete[] Dsc;
 return _error-Error(_(Unable to find a source package for 
%s),Src.c_str());
   }
+
+  if (Last-Index().IsTrusted() == false)
+ UntrustedList += Src +  ;
   
   string srec = Last-AsStr();
   string::size_type pos = srec.find(\nVcs-);
@@ -2576,6 +2587,10 @@
Last-Index().SourceInfo(*Last,*I),Src);
   }
}
+
+   // check authentication status of the source as well
+   if (UntrustedList !=   !AuthPrompt(UntrustedList, false))
+  return false;

// Display statistics
unsigned long long FetchBytes = Fetcher.FetchNeeded();
diff -Nru apt-0.9.7.9+deb7u1/debian/changelog 
apt-0.9.7.9+deb7u2/debian/changelog
--- apt-0.9.7.9+deb7u1/debian/changelog 2013-11-16 12:47:12.0 +0100
+++ apt-0.9.7.9+deb7u2/debian/changelog 2014-06-12 13:22:44.0 +0200
@@ -1,3 +1,10 @@
+apt (0.9.7.9+deb7u2) wheezy; urgency=low
+
+  * SECURITY UPDATE: apt-get source validation
+- CVE-2014-0478
+
+ -- Michael Vogt m...@debian.org  Thu, 12 Jun 2014 12:47:25 +0200
+
 apt (0.9.7.9+deb7u1) wheezy; urgency=low
 
   * Non-maintainer upload.
diff -Nru apt-0.9.7.9+deb7u1/test/integration/framework 
apt-0.9.7.9+deb7u2/test/integration/framework
--- apt-0.9.7.9+deb7u1/test/integration/framework   2013-03-01 
11:51:21.0 +0100
+++ apt-0.9.7.9+deb7u2/test/integration/framework   2014-06-12 
13:21:11.0 +0200
@@ -130,7 +130,7 @@
mkdir rootdir aptarchive keys
cd rootdir
mkdir -p etc/apt/apt.conf.d etc/apt/sources.list.d 
etc/apt/trusted.gpg.d etc/apt/preferences.d
-   mkdir -p var/cache var/lib var/log
+   mkdir -p var/cache var/lib var/log tmp
mkdir -p var/lib/dpkg/info var/lib/dpkg/updates var/lib/dpkg/triggers
touch var/lib/dpkg/available
mkdir -p usr/lib/apt
@@ 

Bug#749795: apt: no authentication checks for source packages

2014-06-12 Thread Thijs Kinkhorst
Hi Michael,

On Thu, June 12, 2014 13:52, Michael Vogt wrote:
 On Thu, Jun 12, 2014 at 11:44:20AM +0200, Thijs Kinkhorst wrote:
  apt: no authentication checks for source packages

 The Debian security team has assigned CVE-2014-0478 to this issue.

 APT developers: we should fix this in wheezy. Are you able to provide an
 update for wheezy for this issue?
 [..]

 Attached is the fix for wheezy with a regression test, a additional
 test run is very welcome (works in my wheezy container both the
 testcase and a manual test when removing /var/lib/apt/lists/*Release*).

Thanks! I've built it and verified that it works for me aswell (and solves
the issue). For the changelog: you need to target wheezy-security, and
may want to add closes: #749795 and urgency=high. With these changes you
can upload to security-master.debian.org. Make sure to build with full
source (-sa) as wheezy-security doesn't yet have the orig tarball.

The patch seems to apply rather cleanly to squeeze, so an update for that
would be nice if possible. Fixing CVE-2011-3634 aswell would be nice if
simple to do but not essential.


Cheers,
Thijs


-- 
To UNSUBSCRIBE, email to debian-bugs-rc-requ...@lists.debian.org
with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org



Bug#749795: apt: no authentication checks for source packages

2014-06-12 Thread Michael Vogt
On Thu, Jun 12, 2014 at 11:44:20AM +0200, Thijs Kinkhorst wrote:
[..]
  apt: no authentication checks for source packages
 
 The Debian security team has assigned CVE-2014-0478 to this issue.
[..]
 As for squeeze, if it's not too much extra work it would be great if an
 update for squeeze was also possible. Perhaps it could also even include
 the fix for https://security-tracker.debian.org/tracker/CVE-2011-3634?

Attached is the debdiff for squeeze. Additional testing welcome (work
in my debian-squeeze environment).

Cheers,
 Michael
diff -Nru apt-0.8.10.3+squeeze1/cmdline/apt-get.cc 
apt-0.8.10.3+squeeze2/cmdline/apt-get.cc
--- apt-0.8.10.3+squeeze1/cmdline/apt-get.cc2011-04-15 09:30:33.0 
+0200
+++ apt-0.8.10.3+squeeze2/cmdline/apt-get.cc2014-06-12 15:03:48.0 
+0200
@@ -959,25 +959,8 @@
return true;
 }
/*}}}*/
-// CheckAuth - check if each download comes form a trusted source  /*{{{*/
-// -
-/* */
-static bool CheckAuth(pkgAcquire Fetcher)
+static bool AuthPrompt(std::string UntrustedList, bool const PromptUser)
 {
-   string UntrustedList;
-   for (pkgAcquire::ItemIterator I = Fetcher.ItemsBegin(); I  
Fetcher.ItemsEnd(); ++I)
-   {
-  if (!(*I)-IsTrusted())
-  {
- UntrustedList += string((*I)-ShortDesc()) +  ;
-  }
-   }
-
-   if (UntrustedList == )
-   {
-  return true;
-   }
-
ShowList(c2out,_(WARNING: The following packages cannot be 
authenticated!),UntrustedList,);
 
if (_config-FindB(APT::Get::AllowUnauthenticated,false) == true)
@@ -986,6 +969,9 @@
   return true;
}
 
+   if (PromptUser == false)
+ return _error-Error(_(Some packages could not be authenticated));
+
if (_config-FindI(quiet,0)  2
 _config-FindB(APT::Get::Assume-Yes,false) == false)
{
@@ -1003,6 +989,27 @@
return _error-Error(_(There are problems and -y was used without 
--force-yes));
 }
/*}}}*/
+// CheckAuth - check if each download comes form a trusted source  /*{{{*/
+// -
+/* */
+static bool CheckAuth(pkgAcquire Fetcher, bool PromptUser=true)
+{
+   string UntrustedList;
+   for (pkgAcquire::ItemIterator I = Fetcher.ItemsBegin(); I  
Fetcher.ItemsEnd(); ++I)
+   {
+  if (!(*I)-IsTrusted())
+  {
+ UntrustedList += string((*I)-ShortDesc()) +  ;
+  }
+   }
+
+   if (UntrustedList == )
+   {
+  return true;
+   }
+
+   return AuthPrompt(UntrustedList, PromptUser);
+}
 // InstallPackages - Actually download and install the packages
/*{{{*/
 // -
 /* This displays the informative messages describing what is going to 
@@ -2229,6 +2236,7 @@
 
// Load the requestd sources into the fetcher
unsigned J = 0;
+   std::string UntrustedList;
for (const char **I = CmdL.FileList + 1; *I != 0; I++, J++)
{
   string Src;
@@ -2237,6 +2245,9 @@
   if (Last == 0)
 return _error-Error(_(Unable to find a source package for 
%s),Src.c_str());
   
+  if (Last-Index().IsTrusted() == false)
+ UntrustedList += Src +  ;
+  
   string srec = Last-AsStr();
   string::size_type pos = srec.find(\nVcs-);
   while (pos != string::npos)
@@ -2319,6 +2330,11 @@
   }
}

+   // check authentication status of the source as well
+   if (UntrustedList !=   !AuthPrompt(UntrustedList, false))
+  return false;
+   
+   
// Display statistics
unsigned long long FetchBytes = Fetcher.FetchNeeded();
unsigned long long FetchPBytes = Fetcher.PartialPresent();
diff -Nru apt-0.8.10.3+squeeze1/debian/changelog 
apt-0.8.10.3+squeeze2/debian/changelog
--- apt-0.8.10.3+squeeze1/debian/changelog  2011-04-15 09:30:33.0 
+0200
+++ apt-0.8.10.3+squeeze2/debian/changelog  2014-06-12 15:07:49.0 
+0200
@@ -1,3 +1,14 @@
+apt (0.8.10.3+squeeze2) squeeze-security; urgency=high
+
+  * SECURITY UPDATE: apt-get source validation (closes: #749795)
+- CVE-2014-0478
+  * SECURITY UPDATE: sensitive information disclosure via incorrect
+hostname validation (LP: #868353)
+- methods/https.cc: properly set CURLOPT_SSL_VERIFYHOST.
+- CVE-2011-3634
+
+ -- Michael Vogt m...@debian.org  Thu, 12 Jun 2014 14:30:59 +0200
+
 apt (0.8.10.3+squeeze1) stable; urgency=low
 
   [ Michael Vogt ]
diff -Nru apt-0.8.10.3+squeeze1/methods/https.cc 
apt-0.8.10.3+squeeze2/methods/https.cc
--- apt-0.8.10.3+squeeze1/methods/https.cc  2011-04-15 09:30:33.0 
+0200
+++ apt-0.8.10.3+squeeze2/methods/https.cc  2014-06-12 14:32:46.0 
+0200
@@ -143,13 +143,11 @@
curl_easy_setopt(curl, CURLOPT_SSL_VERIFYPEER, peer_verify);
 
// ... and hostname against cert CN or subjectAltName
-   int 

Bug#749795: apt: no authentication checks for source packages

2014-06-02 Thread Michael Vogt
On Sat, May 31, 2014 at 12:07:48AM +0200, David Kalnischkies wrote:
 On Fri, May 30, 2014 at 03:21:20PM +0200, Michael Vogt wrote:
  From b7f501b5cc8583f61467f0c7a0282acbb88e4b29 Mon Sep 17 00:00:00 2001
  From: Michael Vogt m...@debian.org
  Date: Fri, 30 May 2014 14:47:56 +0200
  Subject: [PATCH] Show unauthenticated warning for source packages as well
  
  This will show the same unauthenticated warning for source packages
  as for binary packages and will not download a source package if
  it is unauthenticated. This can be overriden with
 
 typo: overridden

Thanks, fixed.
 
  +   // check authentication status of the source as well
  +   if (UntrustedList !=   !AuthPrompt(UntrustedList, true))
  +  return false;
 
 As said, I don't think 'apt-get source' should be interactive, so this
 true should be a false, right?
 
 Reasons (as a repeat):
 - it was not interactive before
 - the error message on 'no' talks about install, so we would need a new
   string
 - 'apt-get download' isn't interactive either
 (- it is more in line with your own commit summary)
 
 Counter arguments?
[..]

Good point! No counter arguments, the risk of breaking script by
prompting is indeed a good reason not to show the prompt (and we do
the same for download).

I changed it to non-interactive now.

Cheers,
 Michael


-- 
To UNSUBSCRIBE, email to debian-bugs-rc-requ...@lists.debian.org
with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org



Bug#749795: apt: no authentication checks for source packages

2014-05-30 Thread Michael Vogt
On Thu, May 29, 2014 at 11:04:35PM +0200, Jakub Wilk wrote:
 Package: apt
 Version: 1.0.3
 Severity: grave
 Tags: security

Thanks for your bugreport. You raise a important issue, but I agree
with David that its best if this goes through the security team for
coordination.
 
 I've been investigating how apt behaves when the repository doesn't
 contain any Release signatures (possibly because they were stripped
 off by a man-in-the-middle attacker).
 
 This is what I found out:
 
 | # cat /etc/apt/sources.list
 | deb http://ftp.debian.org/debian/ unstable main
 | deb-src http://ftp.debian.org/debian/ unstable main
 |
 | # apt-get update
 | Ign http://ftp.debian.org unstable InRelease
 | Ign http://ftp.debian.org unstable Release.gpg
 | Get:1 http://ftp.debian.org unstable Release [205 kB]
 | Get:2 http://ftp.debian.org unstable/main Sources [7249 kB]
 | Get:3 http://ftp.debian.org unstable/main amd64 Packages [6758 kB]
 | Fetched 14.2 MB in 29s (479 kB/s)
 | Reading package lists... Done
 |
 | # echo $?
 | 0
 
 Hmm. There is no warning suggesting that anything fishy is going on,
 and the exit code indicates success. (Perhaps the Igns could raise
 suspicion of an observant sysadmin. But who knows what Ign exactly
 means? At least the apt-get(1) manpage doesn't know.)

Right, I think apt should show a more prominent warning here. I will
look into this next.
 
[..]
 So far, so good. However, apt-get happily downloads unauthenticated
 source packages, with no warning:
 
 | $ apt-get source -d nyancat
 | Reading package lists... Done
 | Building dependency tree
 | Reading state information... Done
 | Selected version '1.2.2-1' (unstable) for nyancat
 | Need to get 20.6 kB of source archives.
 | Get:1 http://ftp.debian.org/debian/ unstable/main nyancat 1.2.2-1 (dsc) 
 [1782 B]
 | Get:2 http://ftp.debian.org/debian/ unstable/main nyancat 1.2.2-1 (tar) 
 [14.1 kB]
 | Get:3 http://ftp.debian.org/debian/ unstable/main nyancat 1.2.2-1 (diff) 
 [4748 B]
 | Fetched 20.6 kB in 0s (1838 kB/s)
 | Download complete and in download only mode
[..]

Indeed, this is a problem that needs fixing. Attached is a patch that
addresses the issue.

Cheers,
 Michael
From b7f501b5cc8583f61467f0c7a0282acbb88e4b29 Mon Sep 17 00:00:00 2001
From: Michael Vogt m...@debian.org
Date: Fri, 30 May 2014 14:47:56 +0200
Subject: [PATCH] Show unauthenticated warning for source packages as well

This will show the same unauthenticated warning for source packages
as for binary packages and will not download a source package if
it is unauthenticated. This can be overriden with
--allow-unauthenticated

Closes: #749795
---
 apt-private/private-download.cc|  5 +
 apt-private/private-download.h |  6 +
 cmdline/apt-get.cc |  9 
 test/integration/test-apt-get-source-authenticated | 26 ++
 4 files changed, 46 insertions(+)
 create mode 100755 test/integration/test-apt-get-source-authenticated

diff --git a/apt-private/private-download.cc b/apt-private/private-download.cc
index a095f0c..be7d23c 100644
--- a/apt-private/private-download.cc
+++ b/apt-private/private-download.cc
@@ -28,6 +28,11 @@ bool CheckAuth(pkgAcquire Fetcher, bool const PromptUser)
if (UntrustedList == )
   return true;
 
+   return AuthPrompt(UntrustedList, PromptUser);
+}
+
+bool AuthPrompt(std::string UntrustedList, bool const PromptUser)
+{
ShowList(c2out,_(WARNING: The following packages cannot be authenticated!),UntrustedList,);
 
if (_config-FindB(APT::Get::AllowUnauthenticated,false) == true)
diff --git a/apt-private/private-download.h b/apt-private/private-download.h
index a108aa5..a90ac7e 100644
--- a/apt-private/private-download.h
+++ b/apt-private/private-download.h
@@ -5,7 +5,13 @@
 
 class pkgAcquire;
 
+// Check if all files in the fetcher are authenticated
 APT_PUBLIC bool CheckAuth(pkgAcquire Fetcher, bool const PromptUser);
+
+// show a authentication warning prompt and return true if the system
+// should continue
+APT_PUBLIC bool AuthPrompt(std::string UntrustedList, bool const PromptUser);
+
 APT_PUBLIC bool AcquireRun(pkgAcquire Fetcher, int const PulseInterval, bool * const Failure, bool * const TransientNetworkFailure);
 
 #endif
diff --git a/cmdline/apt-get.cc b/cmdline/apt-get.cc
index 0f18b0e..d74d6d5 100644
--- a/cmdline/apt-get.cc
+++ b/cmdline/apt-get.cc
@@ -76,6 +76,7 @@
 #include stdio.h
 #include stdlib.h
 #include string.h
+#include sys/ioctl.h
 #include sys/stat.h
 #include sys/statfs.h
 #include sys/statvfs.h
@@ -755,6 +756,7 @@ static bool DoSource(CommandLine CmdL)
 
// Load the requestd sources into the fetcher
unsigned J = 0;
+   std::string UntrustedList;
for (const char **I = CmdL.FileList + 1; *I != 0; I++, J++)
{
   string Src;
@@ -763,6 +765,9 @@ static bool DoSource(CommandLine CmdL)
   if (Last == 0) {
 	 return _error-Error(_(Unable to find a source package for %s),Src.c_str());
   }
+
+  

Bug#749795: apt: no authentication checks for source packages

2014-05-30 Thread David Kalnischkies
On Fri, May 30, 2014 at 03:21:20PM +0200, Michael Vogt wrote:
 From b7f501b5cc8583f61467f0c7a0282acbb88e4b29 Mon Sep 17 00:00:00 2001
 From: Michael Vogt m...@debian.org
 Date: Fri, 30 May 2014 14:47:56 +0200
 Subject: [PATCH] Show unauthenticated warning for source packages as well
 
 This will show the same unauthenticated warning for source packages
 as for binary packages and will not download a source package if
 it is unauthenticated. This can be overriden with

typo: overridden

 +   // check authentication status of the source as well
 +   if (UntrustedList !=   !AuthPrompt(UntrustedList, true))
 +  return false;

As said, I don't think 'apt-get source' should be interactive, so this
true should be a false, right?

Reasons (as a repeat):
- it was not interactive before
- the error message on 'no' talks about install, so we would need a new
  string
- 'apt-get download' isn't interactive either
(- it is more in line with your own commit summary)

Counter arguments?


Best regards

David Kalnischkies


signature.asc
Description: Digital signature


Bug#749795: apt: no authentication checks for source packages

2014-05-29 Thread Jakub Wilk

Package: apt
Version: 1.0.3
Severity: grave
Tags: security

I've been investigating how apt behaves when the repository doesn't 
contain any Release signatures (possibly because they were stripped off 
by a man-in-the-middle attacker).


This is what I found out:

| # cat /etc/apt/sources.list
| deb http://ftp.debian.org/debian/ unstable main
| deb-src http://ftp.debian.org/debian/ unstable main
|
| # apt-get update
| Ign http://ftp.debian.org unstable InRelease
| Ign http://ftp.debian.org unstable Release.gpg
| Get:1 http://ftp.debian.org unstable Release [205 kB]
| Get:2 http://ftp.debian.org unstable/main Sources [7249 kB]
| Get:3 http://ftp.debian.org unstable/main amd64 Packages [6758 kB]
| Fetched 14.2 MB in 29s (479 kB/s)
| Reading package lists... Done
|
| # echo $?
| 0

Hmm. There is no warning suggesting that anything fishy is going on, and 
the exit code indicates success. (Perhaps the Igns could raise 
suspicion of an observant sysadmin. But who knows what Ign exactly 
means? At least the apt-get(1) manpage doesn't know.)


Fortunately, apt-get won't let you install anything:

| # apt-get install -qq nyancat
| WARNING: The following packages cannot be authenticated!
|   nyancat
| E: There are problems and -y was used without --force-yes

And it won't let you even download binary packages:

| $ apt-get download nyancat
| WARNING: The following packages cannot be authenticated!
|   nyancat
| E: Some packages could not be authenticated

So far, so good. However, apt-get happily downloads unauthenticated 
source packages, with no warning:


| $ apt-get source -d nyancat
| Reading package lists... Done
| Building dependency tree
| Reading state information... Done
| Selected version '1.2.2-1' (unstable) for nyancat
| Need to get 20.6 kB of source archives.
| Get:1 http://ftp.debian.org/debian/ unstable/main nyancat 1.2.2-1 (dsc) [1782 
B]
| Get:2 http://ftp.debian.org/debian/ unstable/main nyancat 1.2.2-1 (tar) [14.1 
kB]
| Get:3 http://ftp.debian.org/debian/ unstable/main nyancat 1.2.2-1 (diff) 
[4748 B]
| Fetched 20.6 kB in 0s (1838 kB/s)
| Download complete and in download only mode
|
| $ echo $?
| 0

It is equally happy to unpack them:

| $ apt-get source nyancat
| Reading package lists... Done
| Building dependency tree
| Reading state information... Done
| Selected version '1.2.2-1' (unstable) for nyancat
| Need to get 20.6 kB of source archives.
| Get:1 http://ftp.debian.org/debian/ unstable/main nyancat 1.2.2-1 (dsc) [1782 
B]
| Get:2 http://ftp.debian.org/debian/ unstable/main nyancat 1.2.2-1 (tar) [14.1 
kB]
| Get:3 http://ftp.debian.org/debian/ unstable/main nyancat 1.2.2-1 (diff) 
[4748 B]
| Fetched 20.6 kB in 0s (1637 kB/s)
| gpgv: keyblock resource `/home/jwilk/.gnupg/trustedkeys.gpg': file open error
| gpgv: Signature made Fri Dec 13 23:42:11 2013 CET using RSA key ID 37AD3296
| gpgv: Can't check signature: public key not found
| dpkg-source: warning: failed to verify signature on ./nyancat_1.2.2-1.dsc
| dpkg-source: info: extracting nyancat in nyancat-1.2.2
| dpkg-source: info: unpacking nyancat_1.2.2.orig.tar.gz
| dpkg-source: info: unpacking nyancat_1.2.2-1.debian.tar.gz
| dpkg-source: info: applying 01-nyancat-debhelper.patch
|
| $ echo $?
| 0

And it will even let you build them:

| $ apt-get source -b nyancat
| Reading package lists... Done
| Building dependency tree
| Reading state information... Done
| Selected version '1.2.2-1' (unstable) for nyancat
| Need to get 20.6 kB of source archives.
| Get:1 http://ftp.debian.org/debian/ unstable/main nyancat 1.2.2-1 (dsc) [1782 
B]
| Get:2 http://ftp.debian.org/debian/ unstable/main nyancat 1.2.2-1 (tar) [14.1 
kB]
| Get:3 http://ftp.debian.org/debian/ unstable/main nyancat 1.2.2-1 (diff) 
[4748 B]
| Fetched 20.6 kB in 0s (1757 kB/s)
| gpgv: keyblock resource `/home/jwilk/.gnupg/trustedkeys.gpg': file open error
| gpgv: Signature made Fri Dec 13 23:42:11 2013 CET using RSA key ID 37AD3296
| gpgv: Can't check signature: public key not found
| dpkg-source: warning: failed to verify signature on ./nyancat_1.2.2-1.dsc
| dpkg-source: info: extracting nyancat in nyancat-1.2.2
| dpkg-source: info: unpacking nyancat_1.2.2.orig.tar.gz
| dpkg-source: info: unpacking nyancat_1.2.2-1.debian.tar.gz
| dpkg-source: info: applying 01-nyancat-debhelper.patch
| dpkg-buildpackage: source package nyancat
| dpkg-buildpackage: source version 1.2.2-1
| dpkg-buildpackage: source distribution unstable
| dpkg-buildpackage: source changed by Jonathan McCrohan jmccro...@gmail.com
| dpkg-buildpackage: host architecture amd64
|  dpkg-source --before-build nyancat-1.2.2
|  fakeroot debian/rules clean
[...]

The mitmproxy script I used for testing is attached.

-- System Information:
Debian Release: jessie/sid
 APT prefers unstable
 APT policy: (990, 'unstable')
Architecture: amd64 (x86_64)

Kernel: Linux 3.14-1-amd64 (SMP w/1 CPU core)
Locale: LANG=C, LC_CTYPE=C (charmap=ANSI_X3.4-1968)
Shell: /bin/sh linked to /bin/dash

Versions of packages apt depends on:
ii  

Bug#749795: apt: no authentication checks for source packages

2014-05-29 Thread David Kalnischkies
On Thu, May 29, 2014 at 11:04:35PM +0200, Jakub Wilk wrote:
 Package: apt
 Version: 1.0.3
 Severity: grave
 Tags: security

(personally, this feels a bit high. Mostly as deb-src isn't even part of
 many default configurations in which apt is found. And in those where
 you find it, you probably find also vcs checkouts and wget | sh …
 Anyway tl;dr: I wouldn't like to release jessie with it, so I leave it at
 that and we should ask the relevant security teams what they want, but
 I am too dreamy now to figure out a patch/all the mail addresses…)


 I've been investigating how apt behaves when the repository doesn't contain
 any Release signatures (possibly because they were stripped off by a
 man-in-the-middle attacker).

Thanks! Not many do, so such things usually never get reported…
The pain of being even less sexy than OpenSSL…

(may I ask to push security bugs through the security teams first
 though, so that they can coordinate properly as 'apt' and 'security'
 tends to make people super nervous as recovery tends to be painful)


 This is what I found out:
 
 | # cat /etc/apt/sources.list
 | deb http://ftp.debian.org/debian/ unstable main
 | deb-src http://ftp.debian.org/debian/ unstable main
 |
 | # apt-get update
 | Ign http://ftp.debian.org unstable InRelease
 | Ign http://ftp.debian.org unstable Release.gpg
 | Get:1 http://ftp.debian.org unstable Release [205 kB]
 | Get:2 http://ftp.debian.org unstable/main Sources [7249 kB]
 | Get:3 http://ftp.debian.org unstable/main amd64 Packages [6758 kB]
 | Fetched 14.2 MB in 29s (479 kB/s)
 | Reading package lists... Done
 |
 | # echo $?
 | 0
 
 Hmm. There is no warning suggesting that anything fishy is going on, and the
 exit code indicates success. (Perhaps the Igns could raise suspicion of an
 observant sysadmin. But who knows what Ign exactly means? At least the
 apt-get(1) manpage doesn't know.)

Well, Ign isn't Get - and it stands for Ignore, but that is surely
not your point here.

The problem is that apt supports unsigned repositories as too many
people would bitch too much if it would require a signature – it used
to work before apt 0.6, it has to work forever, man – FOR EVER!

So not getting a signature isn't an error and not even fishy.
It's absolutely fine…


Maybe we could be evil and enforce the usage of the flag on update, too?


 Fortunately, apt-get won't let you install anything:
 
 | # apt-get install -qq nyancat
 | WARNING: The following packages cannot be authenticated!
 |   nyancat
 | E: There are problems and -y was used without --force-yes
 
 And it won't let you even download binary packages:
 
 | $ apt-get download nyancat
 | WARNING: The following packages cannot be authenticated!
 |   nyancat
 | E: Some packages could not be authenticated

JFTR: --allow-unauthenticated is the flag littered all over the place to
let this error disappear. Similar to wget --no-check-certificate | sh.


 So far, so good. However, apt-get happily downloads unauthenticated source
 packages, with no warning:
 
 | $ apt-get source -d nyancat
 | Reading package lists... Done
 | Building dependency tree
 | Reading state information... Done
 | Selected version '1.2.2-1' (unstable) for nyancat
 | Need to get 20.6 kB of source archives.
 | Get:1 http://ftp.debian.org/debian/ unstable/main nyancat 1.2.2-1 (dsc) 
 [1782 B]
 | Get:2 http://ftp.debian.org/debian/ unstable/main nyancat 1.2.2-1 (tar) 
 [14.1 kB]
 | Get:3 http://ftp.debian.org/debian/ unstable/main nyancat 1.2.2-1 (diff) 
 [4748 B]
 | Fetched 20.6 kB in 0s (1838 kB/s)
 | Download complete and in download only mode
 |
 | $ echo $?
 | 0

Snipping the rest as if it downloads, everything else is just a follow
up. You see btw that dpkg-source (which does the unpack/build) would
check signatures on the dsc if it had them available. I think installing
debian-keyring would help (at least for packages from debian).

You will also notice that the check for the hashsum in the Sources, as
well as the check for the hashsum of Sources in Release is done, you
just don't get this lovely warning  error message for missing
signatures (aka: the MITM has to be a bit more active in modifying these
files as well to get away with sneaking bad files on your system).


The source (is this a pun now?) has no indication of ever wanting to
check this. Easy to add, for sure (CheckAuth method exists in various
versions in various places). Hardest part will be deciding if
a) sources should be interactive (needs a new string) OR
b) not just like e.g. 'download'.


I guess the later makes more sense (similar to download) and is more
backward compatible (not suddenly interactive), would therefore not
require a new string and should be simple(r) to backport if needed.
As this will surely find at least a few complainers for stable I will
repeat it though: This breaks (obviously) compatibility with unsigned
archives. Workaround for those buggers would be the flag from above.


Haven't written patch/testcase yet though, as I should be in