GothenburgBitFactory / taskserver

Taskserver - Taskwarrior Synchronisation Server
Other
215 stars 38 forks source link

Sync no longer fails with invalid client PEM files #177

Closed bradyt closed 4 months ago

bradyt commented 2 years ago

Previously, invalid PEM files, for example where a user put taskd.certificate and taskd.key from a different server, in other words, they don't align with $TASKDDATA/ca.cert.pem, or your taskd config | grep ^ca.cert, users would see the following error with CLI Taskwarrior:

% task sync
Syncing with <taskd.server>

The specified session has been invalidated for some reason.
Sync failed.  Could not connect to the Taskserver.

For example, see https://github.com/GothenburgBitFactory/taskwarrior/issues/2211.

It seems that for taskd 1.2.0, behavior has changed, and the line The specified session has been invalidated for some reason. has been replaced with the following:

Cannot perform this action while handshake is in progress.

This seems fine from the perspective of CLI Taskwarrior. But it seems to correlate with changed behavior for third party taskd clients as well, and I am wondering what the implications are for secure communication.

For third party clients, I believe I am using Dart's BoringSSL, rather than gnutls, if it's relevant, the old behavior was that the Taskserver would respond with an empty list of bytes. I would catch this as a custom exception, and might suggest to the user it might be an issue with taskd.certificate and taskd.key.

The new behavior, rather than a response of zero bytes, is that the connection seems to be allowed. Further, the APIs I'm using allow to initialize TLS with no taskd.certificate and taskd.key provided, and the connection still succeeds.

I'm afraid this is an incomplete issue at the moment. I'd like to see if I can reproduce the behavior of Taskserver and Taskwarrior in a clean environment reliably, and possibly git bisect to improve the report. I'd also like to provide some minimal third party client that reproduces the connection with invalid or non-existent client PEM files.

I tried to keep the title to something that is potentially easy to reproduce, possibly we retitle or create new issues if we can clarify some of my more vague claims above. Potentially this seems to me to be, "connection allowed without client PEM files", or, "what are the implications ..."

Also the next release of my mobile app allows these connection attempts without client PEM files, and next UI should make it easier to experiment with adding or removing PEM files from a profile, and testing type: statistics. You would already be able to see some of the behavior with current version, by adding client PEM files from the wrong server.

I could try to write some minimal docker instance that uses some part of my codebase to connect with an automated taskd process.

bradyt commented 2 years ago

Further tests reveal I'm only seeing Cannot perform this action while handshake is in progress., and apparent ignoring of client PEM files, with WingTask.com.

I thought I had a local reproduce with localhost, but that is turning out to be some sort of strange cache-like behavior, where I can get mobile taskd client to "remember" if the socket worked with last known client PEM files, even after I remove them.

Maybe someone can independently confirm the WingTask behavior? Like using inthe.am certifcates with WingTask, or any invalid client PEM files, like,

cd wingtask_configuration
HOME=. task sync

With client PEM files from inthe.am like so:

taskd.server      = app.wingtask.com:53589
taskd.certificate = ~/.wingtask_certs/private.certificate.pem
taskd.key         = ~/.wingtask_certs/private.key.pem
bradyt commented 2 years ago

This is all on macOS. I just tested on a Debian VPS, and I simply get Handshake failed. Error in the certificate. for taskd.server=app.wingtask.com:53589, no matter what I put in .taskrc. Cert seems fine, with the following:

> gnutls-cli taskd.tangential.info:53589
...
- Status: The certificate is trusted.
...

Edit: I have server task as 2.5.1, will update and research.

bradyt commented 2 years ago

Upgraded Debian VPS task to 2.6.1. With invalid client PEM files, I can confirm the behavior with WingTask, Cannot perform this action while handshake is in progress..

bradyt commented 2 years ago

With other taskd servers, with invalid client PEM files, I see the following error:

> gnutls-cli <taskd.server> --x509keyfile <taskd.key> --x509certfile <taskd.certificate>
...
*** Fatal error: Error in the pull function.
*** Server has terminated the connection abnormally.

With taskd.server = app.wingtask.com:53589, this error does not occur for invalid client PEM files.

bradyt commented 2 years ago

I ran git bisect with the following scenario:

How serious of an issue is this? Should Taskserver be preventing connections with arbitrary client PEM files?

I may have invented the term "client PEM files" for taskd.certificate and taskd.key, not sure if this is ambiguous or if there may be a better shorthand term based on facts or existing docs or TLS details.

Aside: If there are some inconsistencies in earlier comments, some of it was due to confusion with the Dart taskd client seemingly caching sockets when sending requests after removing PEM files. I'll research that myself soon.

bradyt commented 2 years ago

Tried with faketime '2000-01-01' tool to generate expired client PEM files, and still allowed to connect to taskd. So I think this clarifies that these certs should not be trusted anywhere, even with some hypothetical mysterious CA distribution.

jakec-dev commented 2 years ago

I'm getting this error message too, even though my .pem files are directly copied from the server where they were generated from.

Anyone know what the deal is? Is this a bug or have I messed something up?

bradyt commented 2 years ago

@jakec-dev, I presume you are running a recent build of 1.2.0, not current stable 1.1.0.

The issue I filed above is about taskd behavior when using invalid client PEM files. So this is the only reproduce I'm aware of.

Maybe you can double check your client's taskd.certificate, taskd.key, and the ca.cert on the server? I think location of server ca.cert could be checked with taskd config | grep ^ca, I think it is often $TASKDDATA/ca.cert.pem.

In other words, in order to arrive at the above error, I think your server's certificate and/or client's taskd.ca are fine as-is.

Can you check the server logs for something like the following:

s: INFO Client certificate will be verified.
...
2021-12-28 00:00:00 Error: Handshake failed. The TLS connection was non-properly terminated.
...
s: INFO Verifying certificate.
...
s: ERROR Certificate status=66
...
s: INFO The certificate is NOT trusted. The certificate issuer is unknown. 
2021-12-28 00:00:00 Error: Handshake failed. Error in the certificate.

Maybe you can use certtool (gnutls-certtool on macOS) to verify your taskd.certificate against your server's ca.cert.

If your issue is not due to taskd.ca, taskd.certificate, and ca.cert I describe above, I think you may want to clarify steps for a minimal reproduce. Or perhaps others more qualified can assist.

jakec-dev commented 2 years ago

Thanks for the reply.

I've created a Stackoverflow issue about it - https://stackoverflow.com/questions/70501186/unable-to-connect-to-port-53589-on-ec2-instance-using-docker-and-caddy-server

This is all I get from the logs:

2021-12-27 01:13:18 ==== taskd 1.1.0  ====
2021-12-27 01:13:18 Serving from /var/taskd
2021-12-27 01:13:18 Using address 0.0.0.0
2021-12-27 01:13:18 Using port 53589
2021-12-27 01:13:18 Using family
2021-12-27 01:13:18 Queue size 10 requests
2021-12-27 01:13:18 Request size limit 1048576 bytes
2021-12-27 01:13:18 IP logging on
2021-12-27 01:13:18 CA          /var/taskd/pki/ca.cert.pem
2021-12-27 01:13:18 Certificate /var/taskd/pki/server.cert.pem
2021-12-27 01:13:18 Private Key /var/taskd/pki/server.key.pem
2021-12-27 01:13:18 CRL         /var/taskd/pki/server.crl.pem
2021-12-27 01:13:18 Server starting
2021-12-27 01:13:18 Server ready
bradyt commented 2 years ago

I was able to reproduce the issue with just Taskwarrior stable, and the same git bisect commit I reported above. I created a deliberately invalid pair of client PEM files, both by way of different CA, etc, as well as being over ten years old with faketime. Before fb5fbe9, the behavior was as follows:

Syncing with localhost:53589

The specified session has been invalidated for some reason.
Sync failed.  Could not connect to the Taskserver.

After that commit, the behavior is as follows:

Syncing with localhost:53589

Sync successful.  No changes.

I created a repo including a Dockerfile, with FROM dart:stable, as I thought I would only find the issue while using Dart's BoringSSL. But I was able to clarify that task 2.6.1 can also connect to taskd 1.2.0 with invalid client PEM files.

Here is a link to the repo, and the commit at which I think I finally was able to clarify the issue today:

If you call make now, it will call task sync and fail. If you comment out RUN git checkout HEAD~, the sync will succeed.

bradyt commented 2 years ago

In summary, changes in behavior with invalid client PEM files, regarding commit fb5fbe9ac76c0cd838bfa2ffa52e3ba34f75b70d:

If it's helpful, here is the Docker setup I used to research this:

bradyt commented 2 years ago

Interestingly, Inthe.am is at taskd 1.1.0 (according to request with type: statistics), and with invalid client PEM files returns:

Cannot perform this action while handshake is in progress.
Sync failed.  Could not connect to the Taskserver.
bradyt commented 2 years ago

Behavior of Dart client is similar as Python client. I tested with https://github.com/jrabbit/taskd-client-py, and found behavior before commit fb5fbe9 was:

Traceback (most recent call last):
  File "main.py", line 3, in <module>
    resp = tc.pull()
  File "/usr/local/lib/python3.7/dist-packages/taskc/simple.py", line 47, in conn_wrapper
    x = f(self, *args, **kwargs) # noqa
  File "/usr/local/lib/python3.7/dist-packages/taskc/simple.py", line 194, in pull
    return self.recv()
  File "/usr/local/lib/python3.7/dist-packages/taskc/simple.py", line 123, in recv
    bytes = struct.unpack('>L', a[:4])[0]
struct.error: unpack requires a buffer of 4 bytes

This looks consistent with my finding that Taskserver sends an empty response.

Behavior after commit fb5fbe9 is:

Response: No change
lauft commented 4 months ago

[!IMPORTANT] Taskserver is only compatible with Taskwarrior 2.x, and is no longer actively developed. See man task-sync for task synchronization with Taskwarrior 3