Bug#212732: Bug#427909: Bug#212732: support redirects and interactive authentication (Progeny)

2009-11-11 Thread Simon Paillard
On Sat, Jan 31, 2009 at 11:01:46AM +1000, Anthony Towns wrote:
 On Fri, Jan 30, 2009 at 09:14:54PM +0100, Michael Vogt wrote:
  On Sun, Dec 21, 2008 at 10:45:13PM +1000, Anthony Towns wrote:
   Attached is a patch against apt 0.7.19 (current in lenny/sid)
   including just the Redirect support from Jeff Licquia's patch in
   Bug#212732. 
  Thanks a lot for this, I merged it into my bzr tree and it will be
  part of the next merge into debian (experimental initially). 
 
 Great!

Great too !
This is in apt since 0.7.21 (but bug closure missing in debian/changelog)

I tested it successfully using 302 redirects.

Could you retroactively close this bug through the changelog ?

apt (0.7.21) unstable; urgency=low
[...]
  [ Michael Vogt ]
[...]
  * [ABI break] merge support for http redirects, thanks to
Jeff Licquia and Anthony Towns
[...]
  * methods/https.cc:
- add Acquire::https::AllowRedirect support

 -- Michael VogtTue, 14 Apr 2009 14:12:51 +0200


-- 
Simon Paillard



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



Bug#212732: Bug#427909: Bug#212732: support redirects and interactive authentication (Progeny)

2009-01-30 Thread Michael Vogt
On Sun, Dec 21, 2008 at 10:45:13PM +1000, Anthony Towns wrote:
 Attached is a patch against apt 0.7.19 (current in lenny/sid)
 including just the Redirect support from Jeff Licquia's patch in
 Bug#212732. 

Thanks a lot for this, I merged it into my bzr tree and it will be
part of the next merge into debian (experimental initially). 
 
 As far as the issues described in Bug#66434 with bad redirection and
 mod_speling, that seems mostly unlikely to be a problem these days thanks
 to the md5 validation and signature support. The only way you could get
 unexpected data is if your original Release and Release.gpg files were
 redirected to the wrong place, but were completely consistent and had
 corresponding Packages files and debs.

One possible issue I can see is that consistency may become a
issue. If the server that redirects does that to mirrors that are not
in sync and the Release file comes from A but the Packages file from B
users may run into hashsum failures. We have the same problem with
users behind proxies and round-robin DNS servers sometimes. The same
for debs when some mirrors may return 404 or fimilar.

That is not a argument against the patch of course, just a
observation.

I can not think of any security concerns about the patch, the
signature and hashsum code should protect us here to the extend
possible. 
 
 In the event that is a concern, the patch lets the user set the 
 Acquire::http::AllowRedirect config option to false to block that behaviour.
 It'd be possible to have an option to verify the filename part of the URL
 is unchanged as well without much difficulty.

Excellent, thanks.

 I bumped the library version, mostly so I could be sure I was testing the
 right thing, but I presume this requires a libapt-pkg ABI bump anyway
 (there's an Acquire::Redirect() callback added), so I left it in the
 patch.

We will make it part of a update that breaks the abi for other items
too. 

Thanks,
 Michael




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



Bug#212732: Bug#427909: Bug#212732: support redirects and interactive authentication (Progeny)

2009-01-30 Thread Anthony Towns
On Fri, Jan 30, 2009 at 09:14:54PM +0100, Michael Vogt wrote:
 On Sun, Dec 21, 2008 at 10:45:13PM +1000, Anthony Towns wrote:
  Attached is a patch against apt 0.7.19 (current in lenny/sid)
  including just the Redirect support from Jeff Licquia's patch in
  Bug#212732. 
 Thanks a lot for this, I merged it into my bzr tree and it will be
 part of the next merge into debian (experimental initially). 

Great!

  As far as the issues described in Bug#66434 with bad redirection [...]
 One possible issue I can see is that consistency may become a
 issue. If the server that redirects does that to mirrors that are not
 in sync and the Release file comes from A but the Packages file from B
 users may run into hashsum failures. 

Yup; that'll be caught and give an error though. I presume the most
likely use will be either redirecting all requests -- in which case
synchronisation isn't an issue; or redirecting pool/ but not dists/ -- in
which case 404s are the only risk, I can see, and seems reasonably minor.

 I can not think of any security concerns about the patch, the
 signature and hashsum code should protect us here to the extend
 possible. 

Yup, that matches my understanding.

Cheers,
aj




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



Bug#427909: Bug#212732: support redirects and interactive authentication (Progeny)

2008-12-21 Thread Anthony Towns
Attached is a patch against apt 0.7.19 (current in lenny/sid)
including just the Redirect support from Jeff Licquia's patch in
Bug#212732. 

As far as the issues described in Bug#66434 with bad redirection and
mod_speling, that seems mostly unlikely to be a problem these days thanks
to the md5 validation and signature support. The only way you could get
unexpected data is if your original Release and Release.gpg files were
redirected to the wrong place, but were completely consistent and had
corresponding Packages files and debs.

In the event that is a concern, the patch lets the user set the 
Acquire::http::AllowRedirect config option to false to block that behaviour.
It'd be possible to have an option to verify the filename part of the URL
is unchanged as well without much difficulty.

I bumped the library version, mostly so I could be sure I was testing the
right thing, but I presume this requires a libapt-pkg ABI bump anyway
(there's an Acquire::Redirect() callback added), so I left it in the
patch.

Cheers,
aj

diff -urb apt-0.7.19/apt-pkg/acquire-method.cc apt-0.7.19aj1/apt-pkg/acquire-method.cc
--- apt-0.7.19/apt-pkg/acquire-method.cc	2008-06-10 07:10:08.0 +1000
+++ apt-0.7.19aj1/apt-pkg/acquire-method.cc	2008-12-21 21:50:41.0 +1000
@@ -447,6 +447,38 @@
 }
 	/*}}}*/
 
+// AcqMethod::Redirect - Send a redirect message   /*{{{*/
+// -
+/* This method sends the redirect message and also manipulates the queue
+   to keep the pipeline synchronized. */
+void pkgAcqMethod::Redirect(const string NewURI)
+{
+   string CurrentURI = UNKNOWN;
+   if (Queue != 0)
+  CurrentURI = Queue-Uri;
+ 
+   char S[1024];
+   snprintf(S, sizeof(S)-50, 103 Redirect\nURI: %s\nNew-URI: %s\n\n,
+ CurrentURI.c_str(), NewURI.c_str());
+
+   if (write(STDOUT_FILENO,S,strlen(S)) != (ssize_t)strlen(S))
+  exit(100);
+
+   // Change the URI for the request.
+   Queue-Uri = NewURI;
+
+   /* To keep the pipeline synchronized, move the current request to
+  the end of the queue, past the end of the current pipeline. */
+   FetchItem *I;
+   for (I = Queue; I-Next != 0; I = I-Next) ;
+   I-Next = Queue;
+   Queue = Queue-Next;
+   I-Next-Next = 0;
+   if (QueueBack == 0)
+  QueueBack = I-Next;
+}
+/*}}}*/
+
 // AcqMethod::FetchResult::FetchResult - Constructor			/*{{{*/
 // -
 /* */
diff -urb apt-0.7.19/apt-pkg/acquire-method.h apt-0.7.19aj1/apt-pkg/acquire-method.h
--- apt-0.7.19/apt-pkg/acquire-method.h	2008-06-10 07:10:08.0 +1000
+++ apt-0.7.19aj1/apt-pkg/acquire-method.h	2008-12-21 21:50:02.0 +1000
@@ -84,6 +84,8 @@
void Log(const char *Format,...);
void Status(const char *Format,...);

+   void Redirect(const string NewURI);
+ 
int Run(bool Single = false);
inline void SetFailExtraMsg(string Msg) {FailExtra = Msg;};

diff -urb apt-0.7.19/apt-pkg/acquire-worker.cc apt-0.7.19aj1/apt-pkg/acquire-worker.cc
--- apt-0.7.19/apt-pkg/acquire-worker.cc	2008-11-24 19:35:21.0 +1000
+++ apt-0.7.19aj1/apt-pkg/acquire-worker.cc	2008-12-21 21:42:06.0 +1000
@@ -220,6 +220,20 @@
 	 Status = LookupTag(Message,Message);
 	 break;
 	
+ // 103 Redirect
+ case 103:
+ {
+if (Itm == 0)
+{
+   _error-Error(Method gave invalid 103 Redirect message);
+   break;
+}
+ 
+string NewURI = LookupTag(Message,New-URI,URI.c_str());
+Itm-URI = NewURI;
+break;
+ }
+   
 	 // 200 URI Start
 	 case 200:
 	 {
diff -urb apt-0.7.19/apt-pkg/makefile apt-0.7.19aj1/apt-pkg/makefile
--- apt-0.7.19/apt-pkg/makefile	2008-06-10 07:10:08.0 +1000
+++ apt-0.7.19aj1/apt-pkg/makefile	2008-12-21 21:54:58.0 +1000
@@ -13,7 +13,7 @@
 # methods/makefile - FIXME
 LIBRARY=apt-pkg
 LIBEXT=$(GLIBC_VER)$(LIBSTDCPP_VER)
-MAJOR=4.6
+MAJOR=4.7
 MINOR=0
 SLIBS=$(PTHREADLIB) $(INTLLIBS) -lutil
 APT_DOMAIN:=libapt-pkg$(MAJOR)
diff -urb apt-0.7.19/methods/http.cc apt-0.7.19aj1/methods/http.cc
--- apt-0.7.19/methods/http.cc	2008-11-24 19:32:23.0 +1000
+++ apt-0.7.19aj1/methods/http.cc	2008-12-21 22:25:28.0 +1000
@@ -39,6 +39,7 @@
 #include errno.h
 #include string.h
 #include iostream
+#include map
 #include apti18n.h
 
 // Internet stuff
@@ -57,6 +58,7 @@
 time_t HttpMethod::FailTime = 0;
 unsigned long PipelineDepth = 10;
 unsigned long TimeOut = 120;
+bool AllowRedirect = false;
 bool Debug = false;
 URI Proxy;
 
@@ -628,6 +630,12 @@
   return true;
}
 
+   if (stringcasecmp(Tag,Location:) == 0)
+   {
+  Location = Val;
+  return true;
+   }
+
return true;
 }
 	/*}}}*/
@@ -900,7 +908,9 @@
  1 - IMS hit
  3 - Unrecoverable error 
  4 - Error with error content page
- 5 -