| Commit message (Collapse) | Author | Age | Files | Lines |
| |
|
|
| |
Static analysis has shown that there are many minor memleaks.
|
| |
|
|
|
|
|
|
|
|
|
| |
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
|
| |
|
|
|
|
|
|
|
|
|
| |
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>
|
| | |
|
| |\
| |
| |
| |
| | |
Drop unused and obsolete macros as well as documentation
See merge request apt-team/apt!428
|
| | |
| |
| |
| |
| |
| |
| |
| |
| | |
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_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.
|
| |/
|
|
|
|
|
|
|
|
|
|
| |
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.
|
| |
|
|
|
| |
OpenSSL is mandatory now, it is no longer possible to build
without https support either.
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
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!
|
| |
|
|
|
|
|
| |
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.
|
| |
|
|
|
|
|
|
|
| |
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).
|
| |
|
|
|
|
| |
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.
|
| |
|
|
|
|
|
|
|
|
| |
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
|
| |
|
|
|
|
|
| |
This makes them retriable, and brings them more into line with
TCP, where handshake is also a transient error.
LP: #1928100
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
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.
|
| |
|
|
|
|
|
|
| |
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.
|
| |
|
|
| |
Reported-By: cppcheck
|
| |\
| |
| |
| | |
apt Debian release 1.8.2
|
| | |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| |
| | |
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
|
| |/
|
|
|
|
|
|
| |
warning: moving a local object in a return statement prevents copy
elision [-Wpessimizing-move]
Reported-By: gcc-9
Gbp-Dch: Ignore
|
| |
|
|
|
|
|
| |
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
|
| |
|
|
| |
Closes: #898886
|
| |
|
|
| |
Prompted-by: Jakub Wilk <jwilk@debian.org>
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
| |
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
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| |
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
|
| |
|
|
|
|
|
|
| |
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
|
| |
|
|
|
|
| |
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.
|
| |
|
|
|
|
|
|
| |
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
|
| |
|
|
|
|
|
|
|
| |
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
|
| |
|
|
|
|
|
|
|
|
|
|
| |
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.
|
| |
|
|
|
|
|
|
|
| |
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.
|
| |
|
|
|
|
|
|
|
|
|
|
|
| |
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.
|
| |
|
|
|
|
|
|
|
| |
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]
|
| |
|
|
|
|
|
|
|
|
| |
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
|
| |
|
|
| |
This probably makes more sense if Verify-Peer is set to off.
|
| |
|
|
|
| |
This should make it easier to figure out what was
going on.
|
| |
|
|
|
| |
We can actually just pass null as a hostname, so let's just
do that when Verify-Host is set to false.
|
| |
|
|
|
| |
Silently ignoring the options might be a security issue,
so produce an error instead.
|
| |
|
|
|
|
|
|
|
|
| |
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.
|
| |
|
|
|
| |
This is especially needed if we use an HTTPS proxy to CONNECT
to an HTTPS URI, as we run TLS-inside-TLS then.
|
| |
|
|
|
|
|
|
|
|
|
|
| |
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.
|
| |
|
|
|
|
|
| |
This fixes a regression from ~alpha2.
Closes: #866559
Gbp-Dch: Full
|
| |
|
|
|
| |
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.
|
| |
|
|
|
|
| |
Tell the user to install ca-certificates.
Closes: #866377
|
| |
|
|
|
|
|
|
|
|
|
| |
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.
|
| |
|
|
|
|
| |
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.
|
| |
|
|
|
|
|
|
| |
GnuTLS can already have data pending in its buffers, we need
to to drain that first otherwise select() might block
indefinitely.
Gbp-Dch: ignore
|
| |
|
|
|
|
|
|
|
|
|
|
|
|
| |
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
|
| |
|
|
|
|
|
|
| |
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
|