summaryrefslogtreecommitdiff
path: root/methods
Commit message (Collapse)AuthorAgeFilesLines
* Stop bragging about old speeds in http.cc commentsJulian Andres Klode2017-07-031-8/+0
| | | | | | That's just ridiculous these days. Gbp-Dch: ignore
* don't set ip addresses as server names for SNIDavid Kalnischkies2017-07-031-2/+12
| | | | | | | | | It is kinda unlikely that apt will ever encounter a certificate for an IP and a user actually using it, but the API documentation for gnutls_server_name_set explicitly says that "IPv4 or IPv6 addresses are not permitted to be set by this function.", so we should follow it. [jak@d.o: Slightly rebased]
* Swap file descriptors before the handshakeJulian Andres Klode2017-07-031-2/+4
| | | | | | | | | | This makes more sense. If the handshake failed midway, we still should run the gnutls bye stuff. The thinking here is to only set the fd after the session setup, as we do not modify it before, so if it fails in session setup, you retain a usable file descriptor. Gbp-Dch: ignore
* Do not error out, only warn if ca certificates are not availableJulian Andres Klode2017-07-031-5/+5
| | | | This probably makes more sense if Verify-Peer is set to off.
* tls: Add more details to error messages, and detect more errorsJulian Andres Klode2017-07-031-9/+11
| | | | | This should make it easier to figure out what was going on.
* http: A response with Content-Length: 0 has no contentJulian Andres Klode2017-07-011-1/+4
| | | | | | APT considered any response with a Content-Length to have a body, even if the value of the header was 0. A 0 length body however, is equal to no body.
* Make Verify-Host and Verify-Peer independent againJulian Andres Klode2017-07-011-2/+2
| | | | | We can actually just pass null as a hostname, so let's just do that when Verify-Host is set to false.
* TLS support: Error out on unsupported curl optionsJulian Andres Klode2017-06-301-2/+4
| | | | | Silently ignoring the options might be a security issue, so produce an error instead.
* Improve closing the TLS connectionJulian Andres Klode2017-06-301-3/+3
| | | | | | | | | | If gnutls_session_bye() exited with an error, we never closed the underlying file descriptor, causing the method to think the connection was still open. This caused problems especially in test-partial-file-support where we checked that a "complete" file and an incomplete file work. The first GET returns a 416 with Connection: close, and the next GET request then accidentally reads the body of the 416 as the header for its own request.
* Switch to 'http' as the default https methodJulian Andres Klode2017-06-303-6/+13
| | | | | The old curl based method is still available as 'curl', 'curl+http', and 'curl+https'.
* http: Add support for https:// proxiesJulian Andres Klode2017-06-301-3/+8
| | | | | | HTTPS proxies just require unwrapping the TLS layer at the proxy connection, that's easy, and of course sending proxy-specific headers that are sent on "http" proxies.
* http: Add support for CONNECT proxying to HTTPS locationsJulian Andres Klode2017-06-302-1/+131
| | | | | | | | | | | | | | | | | | | Proxying HTTPS traffic requires the proxy providing the CONNECT method. This implements the client side of it, although it is a bit hacky. HTTP connect is a normal HTTP CONNECT request, followed by a normal HTTP response, just that the body of the response is the TCP stream of the target host. We use a special wrapper in case there are data bytes in the header packets - in that case, the bytes are stored in a buffer and the buffer will be drained first, afterwards the connection continues directly with the TCP stream (with one more vcall). Also: Do not send full URI to https destinations when proxying, as we are directly interfacing with the destination data stream.
* Allow running the TLS stack on any lower connectionJulian Andres Klode2017-06-301-1/+19
| | | | | This is especially needed if we use an HTTPS proxy to CONNECT to an HTTPS URI, as we run TLS-inside-TLS then.
* Reset failure reason when connection was successfulJulian Andres Klode2017-06-301-1/+3
| | | | | | | | | | | | When APT was trying multiple addresses, any later error somewhere else would be reported with ConnectionRefused or ConnectionTimedOut as the FailReason because that was set by early connect attempts. This causes APT to handle the failures differently, leading to some weirdly breaking test cases (like the changed one). Add debugging to the previously failing test case so we can find out when something goes wrong there again.
* Don't read CaInfo if not specified (missing else)Julian Andres Klode2017-06-301-0/+1
| | | | | | | This fixes a regression from ~alpha2. Closes: #866559 Gbp-Dch: Full
* http: Only use system CA store if CaInfo is not setJulian Andres Klode2017-06-291-7/+10
| | | | | It turns out that curl only sets the system trust store if the CaInfo option is not set, so let's do the same here.
* Improve error message if system CA store is emptyJulian Andres Klode2017-06-291-1/+4
| | | | | | Tell the user to install ca-certificates. Closes: #866377
* use port from SRV record instead of initial portDavid Kalnischkies2017-06-291-1/+5
| | | | | | | | | | | An SRV record includes a portnumber to use with the host given, but apt was ignoring the portnumber and instead used either the port given by the user for the initial host or the default port for the service. In practice the service usually runs on another host on the default port, so it tends to work as intended and even if not and apt can't get a connection there it will gracefully fallback to contacting the initial host with the right port, so its a user invisible bug most of the time.
* support tor+https being handled by httpDavid Kalnischkies2017-06-281-3/+10
| | | | | | The apt-transport-tor package operates via simple symlinks which can result in 'http' being called as 'tor+https', so it must pick up the right configuration pieces and trigger https support also in plus names.
* Introduce Acquire::AllowTLS to turn off TLS supportJulian Andres Klode2017-06-281-0/+3
| | | | | | As requested by Henrique de Moraes Holschuh, here comes an option to disable TLS support. If the option is set to false, the internal TLS layer is disabled.
* Fix https->http redirect issuesDavid Kalnischkies2017-06-281-1/+1
| | | | Gbp-Dch: ignore
* methods: http: Drain pending data before selectingJulian Andres Klode2017-06-283-1/+20
| | | | | | | | GnuTLS can already have data pending in its buffers, we need to to drain that first otherwise select() might block indefinitely. Gbp-Dch: ignore
* Allow building without curlJulian Andres Klode2017-06-281-3/+14
| | | | | This makes testing easier and prepares us for the transition.
* methods: Add HTTPS support to http method, using GnuTLSJulian Andres Klode2017-06-284-181/+379
| | | | | | | | | | | | | | The http method will eventually replace the curl-based https method, but for now, this is an opt-in experiment that can be enabled by setting Dir::Bin::Methods::https to "http". Known issues: - We do not support HTTPS proxies yet - We do not support proxying HTTPS connections yet (CONNECT) - IssuerCert and SslForceVersion are unsupported Gbp-Dch: Full
* methods: connect: Switch from int fds to new MethodFdJulian Andres Klode2017-06-286-72/+126
| | | | | | | | Use std::unique_ptr<MethodFd> everywhere we used an integer-based file descriptor before. This allows us to implement stuff like TLS support easily. Gbp-Dch: ignore
* methods: connect: Change PkgAcqMethod to aptMethodJulian Andres Klode2017-06-284-12/+14
| | | | | | | This will allow us to access ConfigFind() and stuff which makes it possible for us to implement TLS support. Gbp-Dch: ignore
* deal with 3xx httpcodes as required by HTTP/1.1 specDavid Kalnischkies2017-06-261-12/+12
| | | | | | | | | | | | | | An unknown code should be handled the same as the x00 code of this group, but for redirections we used to treat 300 (and a few others) as an error while unknown codes were considered redirections. Instead we check now explicitly for the redirection codes we support for redirecting (and add the 308 defined in RFC 7538) to avoid future problems if new 3xx codes are added expecting certain behaviours. Potentially strange would have been e.g. "305 Use Proxy" sending a Location for the proxy to use – which wouldn't have worked and resulted in an error anyhow, but probably confused users in the process.
* avoid changing directory in mirror methodDavid Kalnischkies2017-06-261-17/+13
|
* Annotate intended switch fall through in httpsDavid Kalnischkies2017-06-261-0/+1
| | | | | Reported-By: gcc-7 Gbp-Dch: Ignore
* basehttp: Only read Content-Range on 416 and 206 responsesJulian Andres Klode2017-01-241-1/+5
| | | | | | | | | | | | This fixes issues with sourceforge where the redirector includes such a Content-Range in a 302 redirect. Since we do not really know what file is meant in a redirect, let's just ignore it for all responses other than 416 and 206. Maybe we should also get rid of the other errors, and just ignore the field in those cases as well? LP: #1657567
* fix various typos reported by spellintianDavid Kalnischkies2017-01-193-5/+5
| | | | | | | | Most of them in (old) code comments. The two instances of user visible string changes the po files of the manpages are fixed up as well. Gbp-Dch: Ignore Reported-By: spellintian
* stop rred from leaking debug messages on recovered errorsDavid Kalnischkies2017-01-191-3/+6
| | | | | | | | rred can fail for a plentory of reasons, but its failure is usually recoverable (Ign lines) so it shouldn't leak unrequested debug messages to an observing user. Closes: #850759
* https: Quote path in URL before passing it to curlJulian Andres Klode2017-01-171-0/+4
| | | | | | | | | | | | | | | | | | | | | | | Curl requires URLs to be urlencoded. We are however giving it undecoded URLs. This causes it go completely nuts if there is a space in the URI, producing requests like: GET /a file HTTP/1.1 which the servers then interpret as a GET request for "/a" with HTTP version "file" or some other non-sense. This works around the issue by encoding the path component of the URL. I'm not sure if we should encode other parts of the URL as well, this one seems to do the trick for the actual issue at hand. A more correct fix is to avoid the dequoting and (re-)quoting of URLs when a redirect occurs / a new request is sent. That's been on the radar for probably a year or two now, but nobody bothered implementing that yet. LP: #1651923
* rename ServerMethod to BaseHttpMethodDavid Kalnischkies2016-12-317-45/+45
| | | | | | | This 'method' is the abstract base for http and https and should as such be called out like this rather using an easily confused name. Gbp-Dch: Ignore
* separating state variables regarding server/requestDavid Kalnischkies2016-12-316-226/+211
| | | | | | | | | | | | | | Having a Reset(bool) method to partially reset certain variables like the download size always were strange, so this commit splits the ServerState into an additional RequestState living on the stack for as long as we deal with this request causing an automatic "reset". There is much to do still to make this code look better, but this is a good first step which compiles cleanly and passes all tests, so keeping it as history might be beneficial and due to avoiding explicit memory allocations it ends up fixing a small memory leak in https, too. Closes: #440057
* Honour Acquire::ForceIPv4/6 in the https transportLukasz Kawczynski2016-12-081-0/+5
|
* gpgv: Untrust SHA1, RIPE-MD/160, but allow downgrading to weakJulian Andres Klode2016-11-251-4/+4
| | | | | | | Change the trust level check to allow downgrading an Untrusted option to weak (APT::Hashes::SHA1::Weak "yes";), so it prints a warning instead of an error; and change the default values for SHA1 and RIPE-MD/160 from Weak to Untrusted.
* report apt-key errors via status-fd messagesDavid Kalnischkies2016-11-241-1/+6
| | | | | | | | | | | | | | | | | | | | We report warnings from apt-key this way already since 29c590951f812d9e9c4f17706e34f2c3315fb1f6, so reporting errors seems like a good addition. Most of those errors aren't really from apt-key through, but from the code setting up and actually calling it which used to just print to stderr which might or might not intermix them with (other) progress lines in update calls. Having them as proper error messages in the system means that the errors are actually collected later on for the list instead of ending up with our relatively generic but in those cases bogus hint regarding "is gpgv installed?". The effective difference is minimal as the errors apply mostly to systems which have far worse problems than a not as nice looking error message, which makes this pretty hard to test – but at least now the hint that your system is broken can be read in proper order (= there aren't many valid cases in which the permissions of /tmp are messed up…). LP: #1522988
* http: skip connection cleanup if we close it anyhowDavid Kalnischkies2016-11-111-0/+3
| | | | Suggested in #529794
* http: clear content before reporting the failureEdgar Fuß2016-11-111-1/+1
| | | | | | | | | | [Comment from commiter:] I have the feeling that the issue itself is fixed for a while already as nowadays we have testcases involving a webserver closing the connection on error (look for "closeOnError") and no even remotely recent reports about it, but moving the content clearance above the failure report is a valid change and shouldn't hurt. Closes: #465572
* improve SOCKS error messages for http slightlyDavid Kalnischkies2016-11-101-14/+46
| | | | | | | | | The 0.0.0.0:0 tor reports is pretty useless by itself, but even if an IP would be reported it is better to show the user the hostname we wanted the proxy to connect to in the same error message. We improve upon it further by looking for this bind address in particular and remap error messages slightly to give users a better chance of figuring out what went wrong. Upstream Tor can't do that as it is technically wrong.
* abort connection on '.' target replies in SRVDavid Kalnischkies2016-09-041-0/+8
| | | | | | | | | | | | | | | Commit 3af3ac2f5ec007badeded46a94be2bd06b9917a2 (released in 1.3~pre1) implements proper fallback for SRV, but that works actually too good as the RFC defines that such an SRV record should indicate that the server doesn't provide this service and apt should respect this. The solution is hence to fail again as requested even if that isn't what the user (and perhaps even the server admins) wanted. At least we will print a message now explicitly mentioning SRV to point people in the right direction. Reported-In: https://bugs.kali.org/view.php?id=3525 Reported-By: Raphaël Hertzog
* support long keyid and fingerprint in gpgv's GOODSIGDavid Kalnischkies2016-09-011-4/+20
| | | | | | | | | | | | | | In gpgv1 GOODSIG (and the other messages of status-fd) are documented as sending the long keyid. In gpgv2 it is documented to be either long keyid or the fingerprint. At the moment it is still the long keyid, but the documentation hints at the possibility of changing this. We care about this for Signed-By support as we detect this way if the right fingerprint has signed this file (or not). The check itself is done via VALIDSIG which always is a fingerprint, but there must also be a GOODSIG (as expired sigs are valid, too) found to be accepted which wouldn't be found in the fingerprint-case and the signature hence refused.
* try not to call memcpy with length 0 in hash calculationsDavid Kalnischkies2016-09-011-4/+5
| | | | | | | | | | memcpy is marked as nonnull for its input, but ignores the input anyhow if the declared length is zero. Our SHA2 implementations do this as well, it was "just" MD5 and SHA1 missing, so we add the length check here as well as along the callstack as it is really pointless to do all these method calls for "nothing". Reported-By: gcc -fsanitize=undefined
* Merge branch 'portability/freebsd'Julian Andres Klode2016-08-272-1/+3
|\
| * methods/connect.cc: Only use AI_IDN if definedJulian Andres Klode2016-08-261-0/+2
| | | | | | | | Gbp-Dch: ignore
| * CMake: Do not use -lresolv if res_init exists in libcJulian Andres Klode2016-08-261-1/+1
| | | | | | | | Gbp-Dch: ignore
* | show apt-key warnings in apt updateDavid Kalnischkies2016-08-251-0/+3
|/ | | | | | | | | | | | | | | | | | | | In 105503b4b470c124bc0c271bd8a50e25ecbe9133 we got a warning implemented for unreadable files which greatly improves the behavior of apt update already as everything will work as long as we don't need the keys included in these files. The behavior if they are needed is still strange through as update will fail claiming missing keys and a manual test (which the user will likely perform as root) will be successful. Passing the new warning generated by apt-key through to apt is a bit strange from an interface point of view, but basically duplicating the warning code in multiple places doesn't feel right either. That means we have no translation for the message through as apt-key has no i18n yet. It also means that if the user has a bunch of sources each of them will generate a warning for each unreadable file which could result in quite a few duplicated warnings, but "too many" is better than none. Closes: 834973
* methods: read config in most to least specific orderDavid Kalnischkies2016-08-171-2/+2
| | | | | | | | | | | | The implementation of the generic config fallback did the fallback in the wrong order so that the least specific option wasn't the last value picked but in fact the first one… doh! So in the bugreports case http -> https -> http::<hostname> -> https::<hostname> while it should have been the reverse as before. Regression-In: 30060442025824c491f58887ca7369f3c572fa57 Closes: 834642
* don't try pipelining if server closes connectionsDavid Kalnischkies2016-08-161-1/+9
| | | | | | | | | | | | | | | | | | | | | | | | If a server closes a connection after sending us a file that tends to mean that its a type of server who always closes the connection – it is therefore relatively pointless to try pipelining with it even if it isn't a problem by itself: apt is just restarting the pipeline each time after it got served one file and the connection is closed. The problem starts if one or more proxies are between the server and apt and they disagree about how the connection should be as in the bugreporters case where the responses apt gets contain both Keep-Alive and Proxy-Connection headers (which apt both ignores) indicating a proxy is trying to keep a connection open while the response also contains "Connection: close" indicating the opposite which apt understands and respects as it is required to do. We avoid stepping into this abyss by not performing pipelining anymore if we got a respond with the indication to close connection if the response was otherwise a success – error messages are sent by some servers via this method as their pages tend to be created dynamically and hence their size isn't known a priori to them. Closes: #832113