summaryrefslogtreecommitdiff
path: root/methods/connect.cc
Commit message (Collapse)AuthorAgeFilesLines
* apt-pkg: methods: fixed many minor memleaksHerman Semenoff2026-05-011-0/+3
| | | | Static analysis has shown that there are many minor memleaks.
* tls: Do not trigger assertion after closing connectionJulian Andres Klode2025-06-241-2/+1
| | | | | | | | | | | In HasPending(), we assert that there is an open connection object (ssl), but it seems we can end up calling HasPending() after calling Close(). Therefore, remove the assertion and include ssl != nullptr as the first requirement. Closes: #1107827
* Fix regression that prevented sending intermediate client certificatesSam Hartman2025-01-231-1/+1
| | | | | | | | | | | The change from the GNUTLS backend to the OpenSSL backend only loads the first certificate from a client certificate file. As a result it is no longer possible to send intermediate client certificates to the server to assist in certificate verification. Instead call ssl_ctx_use_certificate_chain_file so intermediate certificates can be sent. Signed-off-by: Sam Hartman <hartmans@debian.org>
* methods/connect: Avoid unused argument warningJulian Andres Klode2025-01-141-1/+1
|
* Merge branch 'fix/cleanup' into 'main'Julian Andres Klode2025-01-141-4/+4
|\ | | | | | | | | Drop unused and obsolete macros as well as documentation See merge request apt-team/apt!428
| * Drop usage of macro APT_OVERRIDE for simple overrideDavid Kalnischkies2025-01-051-4/+4
| | | | | | | | | | | | | | | | | | We were rather inconsistent in using it and as our public headers contain deduction guides (a c++17 feature) it seems silly to try to hide a c++11 feature in a macro, so lets stop this charade and drop the macro and while we are changing all these lines lets apply [[nodiscard]] (another c++17 feature) and other suggestions from clang-tidy and formatting for a little more consistency.
* | ssl: Explictly return -1 (error) or 0 (server closed connection)Julian Andres Klode2025-01-081-5/+9
| | | | | | | | | | | | SSL_read() and SSL_write may return 0 for errors, whereas our callers only expect -1 for errors and 0 for closed connections, so map this accordingly.
* | ssl: Set an error when we receive SSL_ERROR_SYSCALLJulian Andres Klode2025-01-081-0/+1
|/ | | | | | | | | | | | The OpenSSL docs say: > For socket I/O on Unix systems, consult errno for details but also: > This value can also be returned for other errors, > check the error queue for details. This means it's possible errno isn't set, in which case we may appear to have reached an EOF when we did not.
* Remove GnuTLS and gcrypt supportJulian Andres Klode2024-12-221-259/+0
| | | | | OpenSSL is mandatory now, it is no longer possible to build without https support either.
* hashes, methods: Add OpenSSL backendsJulian Andres Klode2024-12-221-4/+291
| | | | | | | | | | | | | | | | | | | | Introduce an OpenSSL::Crypto backend for the hashes library and an OpenSSL::SSL backend for the TLS support in our https method. Many thanks to curl for showing the way with how to handle a CRL file. There are some memory leaks here with the TlsFd itself as well as the proxy support; and we should reorganize the code to generate the ssl object as late as possible. A peculiar aspect of OpenSSL is that SSL_has_pending() returns 1 even if SSL_read() will fail to read anything and return the equivalent of EAGAIN. We work around this here by also peeking ahead 1 byte. I was running a very high RTT connection from Germany to Australia for testing, and with the peeking it's using negligible amounts of CPU; before that, it was busy looping at 100%. Bad OpenSSL!
* Tag hidden acquire classes as final for devirtualizationDavid Kalnischkies2024-11-221-2/+2
| | | | | | | gcc reports `warning: Declaring type ‘class X’ final would enable devirtualization of Y calls [-Wsuggest-final-types]`. Not that this would massively improve performance, but it shouldn't hurt telling the compiler what is obvious for a human.
* Support building without gnutlsSteve Langasek2024-03-031-2/+5
| | | | | | | | | Once in a generation, it may be necessary to bootstrap apt in an environment where gnutls is not yet available. This makes gnutls support in apt optional. You may also want a configure flag to force gnutls to be required from outside the buildsystem (e.g. debian/rules).
* Modernize standard library includesJulian Andres Klode2024-02-201-3/+3
| | | | | | This was automated with sed and git-clang-format, and then I had to fix up the top of policy.cc by hand as git-clang-format accidentally indented it by two spaces.
* Use https config on https proxies for http serversDavid Kalnischkies2021-09-131-9/+10
| | | | | | | | | | The settings used for unwrapping TLS connections depend on the access and hostname we connect to more than what we eventually unwrap. The bugreport mentions CaInfo, but all other https-settings should also apply (regardless of generic or hostname specific) to an https proxy, even if the connection we proxy through it is http-only. Closes: #990555
* Turn TLS handshake issues into transient errorsJulian Andres Klode2021-05-121-1/+1
| | | | | | | This makes them retriable, and brings them more into line with TCP, where handshake is also a transient error. LP: #1928100
* connect: use ServiceNameOrPort, not Port, as the cache keyFaidon Liambotis2020-12-231-4/+7
| | | | | | | | | | | | | | | | | | | The "last connection" cache is currently being stored and looked up on the combination of (LastHost, LastPort). However, these are not what the arguments to getaddrinfo() were on the first try: the call is to getaddrinfo(Host, ServiceNameOrPort, ...), i.e. with the port *or if 0, the service name* (e.g. http). Effectively this means that the connection cache lookup for: https://example.org/... i.e. Host = example.org, Port = 0, Service = http would end up matching the "last" connection of (if existed): https://example.org/... i.e. Host = example.org, Port = 0, Service = https ...and thus performing a TLS request over an (unrelated) port 80 connection. Therefore, an HTTP request, followed up by an (unrelated) HTTPS request to the same server, would always fail. Address this by using as the cache key the ServiceNameOrPort, rather than Port.
* connect: convert a C-style string to std::stringFaidon Liambotis2020-12-231-11/+8
| | | | | | | | Convert the fixed-size (300) char array "ServStr" to a std::string, and simplify the code by removing snprintfs in the process. While at it, rename to the more aptly named "ServiceNameOrPort" and update the comment to reflect what this variable is meant to be.
* Apply various suggestions by cppcheckDavid Kalnischkies2019-07-081-1/+1
| | | | Reported-By: cppcheck
* Merge tag '1.8.2'Julian Andres Klode2019-06-111-28/+51
|\ | | | | | | apt Debian release 1.8.2
| * methods: https: handle requests for TLS re-handshakeMichael Zhivich2019-05-211-28/+51
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | When accessing repository protected by TLS mutual auth, apt may receive a "re-handshake" request from the server, which must be handled in order for download to proceed. This situation arises when the server requests a client certificate based on the resource path provided in the GET request, after the inital handshake in UnwrapTLS() has already occurred, and a secure connection has been established. This issue has been observed with Artifactory-backed Debian repository. To address the issue, split TLS handshake code out into its own method in TlsFd, and call it when GNUTLS_E_REHANDSHAKE error is received. Signed-off-by: Michael Zhivich <mzhivich@akamai.com> (merged from Debian/apt#93) LP: #1829861
* | Fix new gcc-9 warning -Wpessimizing-moveDavid Kalnischkies2019-04-161-1/+1
|/ | | | | | | | warning: moving a local object in a return statement prevents copy elision [-Wpessimizing-move] Reported-By: gcc-9 Gbp-Dch: Ignore
* Handle a missed case of timed out ip addressesJulian Andres Klode2018-05-241-0/+1
| | | | | | | Correctly register timed out IP addresses from a timed out select() call as a bad address so we do not try it again. LP: #1766542
* Reword error for timed out read/write on SOCKS proxyDavid Kalnischkies2018-05-191-1/+6
| | | | Closes: #898886
* Remove obsolete RCS keywordsGuillem Jover2018-05-071-1/+0
| | | | Prompted-by: Jakub Wilk <jwilk@debian.org>
* Correctly report transient errors againJulian Andres Klode2018-01-031-4/+8
| | | | | | | | | | | | | | Commit 47c0bdc310c8cd62374ca6e6bb456dd183bdfc07 ("report transient errors as transient error") accidentally changed some connection failures to become non-transient, because the result of the error checks where being ignored and then fatal error was returned if an error was pending - even if that error was trivial. After the merge of pu/happy-eyeballs2a this becomes a lot clearer, and easy to fix. Gbp-Dch: ignore Regression-Of: 47c0bdc310c8cd62374ca6e6bb456dd183bdfc07
* Add rapid "happy eyeballs" connection fallback (RFC 8305)Julian Andres Klode2018-01-031-26/+100
| | | | | | | | | | | | | | | | | | | | | | Try establishing connections in alternating address families in rapid intervals of 250 ms, adding more connections to the wait list until one succeeds (RFC 8305, happy eyeballs 2). It is important that WaitAndCheckErrors() waits until it has a successful connection, a time out, or all connections failed - otherwise the timing between tries might be wrong, and the final long wait might exit early because one connection failed without trying the others. Timing wise, this only works correctly on Linux, as select() counts down there. But we rely on that in some other places too, so this is not the time to fix that. Timeouts are only reported in the final long wait - the short inner waits are expected to time out more often, and multiple times, we do not want to report them. Closes: #668948 LP: #1308200 Gbp-Dch: paragraph
* connect: Extract Connection::CheckError() methodJulian Andres Klode2018-01-031-0/+6
| | | | | | | | Extracting the error checking method allows us to reuse it in different places, so we can move the waiting and checking out of DoConnect() eventually. Gbp-Dch: ignore
* connect: Store the IP used when picking a connectionJulian Andres Klode2018-01-031-9/+5
| | | | | | There's no real point in storing the IP address while resolving it - failure messages include the IP address in any case. Do this when picking the connection for actual use instead.
* connect: Extract a Connection structJulian Andres Klode2018-01-031-8/+36
| | | | | | | | This struct holds information about a connection attempt, like the addrinfo, the resolved address, the fd for the connection, and so on. Gbp-Dch: ignore
* connect: Alternate address families for addressesJulian Andres Klode2018-01-031-24/+48
| | | | | | | | | As a first step to implementing Happy Eyeballs version 2, we need to order the list of hosts getaddrinfo() gave us so it alternates between preferred and other address families. RFC: https://tools.ietf.org/html/rfc8305 Gbp-Dch: ignore
* report transient errors as transient errorsDavid Kalnischkies2017-12-131-79/+182
| | | | | | | | | | | | The Fail method for acquire methods has a boolean parameter indicating the transient-nature of a reported error. The problem with this is that Fail is called very late at a point where it is no longer easily identifiable if an error is indeed transient or not, so some calls were and some weren't and the acquire system would later mostly ignore the transient flag and guess by using the FailReason instead. Introducing a tri-state enum we can pass the information about fatal or transient errors through the callstack to generate the correct fails.
* proper error reporting for v3 onion servicesDavid Kalnischkies2017-09-261-3/+3
| | | | | | | | | APT connects just fine to any .onion address given, only if the connect fails somehow it will perform checks on the sanity of which in this case is checking the length as they are well defined and as the strings are arbitrary a user typing them easily mistypes which apt should can be slightly more helpful in figuring out by saying the onion hasn't the required length.
* Reformat and sort all includes with clang-formatJulian Andres Klode2017-07-121-10/+10
| | | | | | | | | | | | | This makes it easier to see which headers includes what. The changes were done by running git grep -l '#\s*include' \ | grep -E '.(cc|h)$' \ | xargs sed -i -E 's/(^\s*)#(\s*)include/\1#\2 include/' To modify all include lines by adding a space, and then running ./git-clang-format.sh.
* 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.
* 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.
* 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.
* 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.
* methods: http: Drain pending data before selectingJulian Andres Klode2017-06-281-0/+9
| | | | | | | | GnuTLS can already have data pending in its buffers, we need to to drain that first otherwise select() might block indefinitely. Gbp-Dch: ignore
* methods: Add HTTPS support to http method, using GnuTLSJulian Andres Klode2017-06-281-0/+361
| | | | | | | | | | | | | | 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-281-15/+41
| | | | | | | | 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