Re: Bug#139710: first version of https support available
Thanks heaps for commenting Jason!
(Should I be Cc: all those lists?
(Below: Ack == Yes, I'll do that)
On Tue, 2 Apr 2002, Jason Gunthorpe wrote:
> On Tue, 2 Apr 2002, Tomas Pospisek wrote:
>
> > http: //sourcepole.ch/sources/software/apt_https/
>
> > * how do you like the integration with the rest of your stuff?
>
> Going down the diff..
>
> +ah_WITH_SSL
>
> You are not 'ah', use to_ or something..
Ack, thanks (autotools newbie here)
> The macro does not seem to actually test if SSL is available and working
> to build against, it probably should.
Ack. Fixed. As of now I'm testing for -lssl and for openssl/ssl.h. And
just failing and telling the user to install it somewhere where configure
can find it. If that's not traditional, expected behaveour I'd appreciate
a hint.
> All the changes to connect.cc appear to be needless.
Ack.
> +#ifdef WITH_SSL
> +bool CircleBuf::Read(SSL * Ssl)
>
> This is C++. The way to implement this without wacks of #ifdefs everywere
> is to suck a few more things into circlebuf and subclass it for the SSL
> case. The wacks of ifdefs and 'if https's are really excessive.
I sympathize. My motive for using #ifdefs was to keep the executable
SSL-clean, so that it's possible to link it without libssl.
I can see that I can subclass "class CircleBufSSL : CircleBuf". Should I
put that into a separate file or leave that in http.*?
But what about the other classes? SSL-enabling f.ex. ServerState means
just minimal changements to ServerState. If I subclass it I will have to
override some methods and copy-paste 95% of their code over to the new
method (f.ex. in ServerState::Open), which will result in two nearly
identical code sets that need to be maintained. Is this what you want me
to do? (IMHO not a good idea). See further below for more about the issue.
> + string ProxyStr;
>
> Making 'ProxyStr' a string is silly, it is never assigned to anything but
> a constant.
Ack, thanks.
> ConnectSSL really needs to use the normal connect function and then just
> bind the SSL library to that FD (there are functions in openssl to do
> this). The normal connect function does much more than what openssl
> provides.
I'm not sure I understand you correctly - this is exactly what's happening
right now - except that it's in ServerState::Open where this is happening
(ifdefs and logging removed):
if (ServerName.Access == "https") {
if (Connect(Host,Port,"https",443,ServerFd,TimeOut,Owner) == false)
return false;
else if (ConnectSSL(ServerFd,&Ssl))
return false;
return false;
}
else if (Connect(Host,Port,"http",80,ServerFd,TimeOut,Owner) == false)
return false;
We first do Connect through the allready existing mechanism and bind then
SSL on top of it. Maybe you want me to move this *into* ConnectSSL, which
I can do. Or alternatively I can:
if (ServerName.Access == "https")
{
ProxyStr = "https_proxy";
Acquire = "Acquire::https::Proxy";
DefaultPort = 443;
Protocol = "https";
}
else
{
ProxyStr = "http_proxy";
Acquire = "Acquire::http::Proxy";
DefaultPort = 80;
Protocol = "http";
}
if (Connect(Host,Port,Protocol,DefaultPort,ServerFd,TimeOut,Owner) == false)
return false;
if(Protocol == "https" && ConnectSSL(ServerFd,&Ssl)
return false;
Looks cleaner anyway :-), let me know.
> Ultimately I don't think we can expect to have https included with APT, as
> that needs openssl on the base install.
Ack. That was my intention.
(OT: Btw. don't you think basic encryption algorithms will move into base
with time? Most networking programs will be SSL enabled one day anyway).
> So at best it would have to compile seperately to something called https
> (this is trivial).
Ack.
> You also
> forgot the makefile bit to actually make the https symlink, which would be
> necessary for this to work at all.
Ack.
> > Certificate validation is on my todo list.
>
> SSL is utterly pointless without this,
Well at least you can not hijack the connection any more and the transfer
is unreadable. But Ack all the same!
> and I'm not sure how you would go about implementing it in this sort of
> context..
I do want to implement it - I'll see! My intention with the posting
was to see if the apt team was interested at all in my contribution and to
make sure it fits in before I proceed implementing.
*t
-----------------------------------------------------------------------
Tomas Pospisek
Infinite Justice for the World:
http://www.heise.de/tp/deutsch/inhalt/co/11621/11621_2.jpg
------------------------------------------------------------------------
--
To UNSUBSCRIBE, email to deity-request@lists.debian.org
with a subject of "unsubscribe". Trouble? Contact listmaster@lists.debian.org
Reply to: