crypt.c:49:4: warning: 'break' will never be executed
[-Wunreachable-code-break]
(... and 3 more of the same)
crypt.c:627:7: warning: variable 'f' may be uninitialized when used
here [-Wconditional-uninitialized]
crypt.c:539:12: note: initialize the variable 'f' to silence this
warning
- Add (void) casts for unused function parameters
- Rearrange member in `struct rb_mbedtls_cfg_context' for data alignment
- Document a `clang-4.0 -Weverything' (-Wcast-qual) diagnostic
- Avoid pointless conversions between positive/negative error codes
- Use capital hexadecimals in error codes and properly cast to
(unsigned int) for %x/%X
the [manpage][] says:
> unsigned int * cert_max
> Initially must hold the maximum number of certs. It will be updated
> with the number of certs available.
ratbox doesn't actually initialize that variable, so gnutls naturally
fails. i would also recommend considering dynamically allocating the
cert list to deal with that error in other ways than failing to
configured SSL completely in GnuTLS. the apache gnutls module has a
similar problem and came up with a [patch][] to do exactly this which
you may want to consider.
but since our cert chain is only (!) 5 certs long, our itched is
scratch by this particular patch.
[manpage]: https://manpages.debian.org/jessie/gnutls-doc/gnutls_x509_crt_list_import.3.en.html
[patch]: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=511573#35
This backports the code responsible for SPKI digests from release/4.
It also adjusts doc/reference.conf to note that SPKI digests are now
supported, and how to generate them. It does NOT backport the mkfingerprint
program -- the instructions in reference.conf are sufficient. I am ofcourse
open to anyone else backporting the program, but I don't see the need.
No code was changed in this commit; just entire lines moved up or down.
Ofcourse, most git diff tools won't see it that way.
The diff between the MbedTLS backend and this one is now fairly
minimal.
Note to auditors importing this series of patches for review and
integration: This means you can skip this patch if you don't trust me.
Properly check whether the library was interrupted by the kernel
before assuming that a nonzero errno was caused by the kernel.
Otherwise, a memory allocation failure in the library for example
would incorrectly be interpreted as a syscall error instead of a
library error.
* Set errno to 0 before attempting any read/write operations as it may
affect our tests otherwise.
* Properly check whether the gnutls_record_recv()/gnutls_record_send()
call failed and distinguish between kernel and library errors.
* Add debugging assertions.
* Reduce the buffer size in line with the other backends.
* Ask for the cipher name directly instead of constructing it ourselves
from the key exchange / authentication algorithm, symmetric encryption
algorithm, and message authentication code algorithm.
Explicitly ignore the snprintf return value and properly indent
the following line.
Also add a 'v' before the version strings so it reads as e.g.:
library (v3.3.8), compiled (v3.3.8)
* Give function parameters const correctness.
* Use similar variable names as the other backends -- this will reduce
the diff between them.
* Check for more kinds of errors in retrieving the peer's certificate.
* Log failure to generate fingerprints, like the MbedTLS backend does.
Use the variable name instead of its type as an argument to a sizeof
allocation. This will prevent possible future errors being introduced
when the type of the variable is changed, but the sizeof argument is
not updated.
* Add more debugging assertions
* Cancel timeouts and callbacks when handshake succeeds or hard fails
* Differentiate between kernel and library errors
* Increase F->handshake_count on handshake success
I'm still not exactly sure what this is even used for. It may be
removed from all backends at a later time if I find it's not being
used for anything important, as right now it can only have the values
0 or 1.
This is similar to commit db12df5c16 for
the MbedTLS backend.
The difference is, GNUTLS will not accept positive values as input to
gnutls_strerror(), so we invert the sign bit after retrieving the value
too, not just when storing it.
Also add a forgotten ssl_errno assignment to rb_ssl_connect_common().
This avoids a compiler warning regarding casting a file descriptor to a
pointer (as input to gnutls_transport_set_ptr()), and also ensures that
the pointer is valid for the lifetime of the session.
If gnutls_bye() fails with a fatal error, we would reattempt it again
and again, even though this may then go on to e.g. cause a segmentation
fault.
Now we just keep retrying if it was interrupted, in line with the other
backends, up to a maximum of 3 retries.
Yeah, I know, I said I was happy with it and wouldn't be changing it.
However, the new GNUTLS backend I'm working on has prompted this.
E.g. MbedTLS error codes and GNUTLS error codes are both negative ints,
but GNUTLS will not tolerate positive input values. Let's treat
both backends the same.
