search

purejava / keepassxc-proxy-access

A Java library to access KeePassXC via its build-in proxy
MIT License
13 stars 5 forks source link

Reading messages for generate-password request is not robust enough #4

Closed purejava closed 2 years ago

purejava commented 2 years ago

Please agree to the following

Summary

In case multiple messages are returned at once by the KeePassXC proxy, keepassxc-proxy-access does not distinguish clearly between them

What software is involved?

Steps to Reproduce

  1. connect
  2. associate
  3. send the generate-password request and press Apply password in the KeePassXC password generator popup

Expected Behavior

keepassxc-proxy-access should process each returned message separately

Actual Behavior

keepassxc-proxy-access does not distinguish clearly between the returned messages, reads two or more messages at once and processes these as one message

Reproducibility

Intermittent

Relevant Log Output

[main] TRACE org.keepassxc.Connection - Send - encrypting the following message: {"action":"generate-password","clientID":"HXkLwnQ7wFpM5SwCrvnbwZXBEBJ2L410"}
[main] TRACE org.keepassxc.LinuxMacConnection - Sending message: {"action":"generate-password","clientID":"HXkLwnQ7wFpM5SwCrvnbwZXBEBJ2L410","message":"qhhti1eH6I1bIyi+qbF7310QTTaNJQnk+XMuS9Sx3N4CSAJA2dvll+wUAypbU5jD00Novu/Fq9FtqqD9cdb06fA1rkZVr7oATq4eIruGKYkig1iMrel5JYCwQeY=","nonce":"/bKDHxbAklMKhei1+QHOVD1hmJYB7wTk"}
[pool-1-thread-1] TRACE org.keepassxc.LinuxMacConnection - Reading message: {}
[pool-1-thread-2] TRACE org.keepassxc.Connection - KeePassXC send an empty response: {}
[pool-3-thread-1] DEBUG org.purejava.KeepassProxyAccess - Attempting to save credentials
[pool-3-thread-1] DEBUG org.purejava.KeepassProxyAccess - Credentials saved
[pool-1-thread-1] TRACE org.keepassxc.LinuxMacConnection - Reading message: {"action":"generate-password","message":"n4aSiH6byrgeZVixT6TnQpfD8unZ2C/SOmWgtkI9VRR3d73FMz0I40+ufFFV7DkuwQGDZ+uHvoeV4ipMrOsqhB/gl3gi+1Q2XPlppojpceruHPgjQPMZvB4OPi1lJ5xusEF+uNkZV75V8L8azyrogKQ1efBNEN4sKaKmU3S2uDaQtUkHhmtx9BOnonqY5tQ6UotM7YqZ10VdcyPgZTpIJ71tUH7w","nonce":"/rKDHxbAklMKhei1+QHOVD1hmJYB7wTk"}
[pool-1-thread-1] TRACE org.keepassxc.LinuxMacConnection - Reading message: {"action":"generate-password","message":"o+80N2eiBE80QpFqOSO7wBFTVb6LlZq9GNxU3Tu1AxlN91W3VqaNC8Ro0P7YBee1uEK7qWcskK9eEzHTy12MvRomrXzhy2NqlyHRUuM6IA86pyXLsxgo+ZsTH3PcSLlpXLqw7eGLslwnswof4RjJH1s/WCb5lLmO9CaqdTmLJSSbkhhd53Lz2/dTN+wT+l4pVfh0rgQkr/5GkvnIn3IKU1BdVlam","nonce":"/rKDHxbAklMKhei1+QHOVD1hmJYB7wTk"}
[pool-1-thread-1] TRACE org.keepassxc.LinuxMacConnection - Reading message: {"action":"generate-password","message":"47xqKdyW+24nyZF4epZxD5dbCNt7Z8KuIJu1kv8YocN8bghY/Xp9CIZgU2r8bsieGQ1pguG/ZKLijYgd8WAm5ppN9sJ91lY2kmUu3s3GoNo4q3f62thgiFNs82SZTRHmEYZRs6iYhZk3xxWNJqdkf19MwVhB3C8CwXWpcBVPka4SW6IbjzEUvrZK0SuEw+SMXOnNyrsaH60J3x6Q+USDAPbsuJXj","nonce":"/rKDHxbAklMKhei1+QHOVD1hmJYB7wTk"}{"action":"generate-password","message":"4Dnm5sdfDMvjgL2luRMeRjwrj/xTtv7n83+HTg3TcCHJ4XPawPkcCGqr+bf85jNClSgzYW1X7UkdRUcVgXJdUOhNx9mq71dC+oa8S3etIBRajU1oE2MSDp/65N7iALACxsGPrb+hHOpWeuXLP/lfZSQd2FfFcGWsoto7+ftJWjGHMMjvNHQGVE39eRk5yZ/SK/rjS4LNVuT8sJcid9UAnGP2fq5X","nonce":"/rKDHxbAklMKhei1+QHOVD1hmJYB7wTk"}

Anything else?

This is also discussed upstream.

purejava commented 2 years ago

I dug into the transferred bytes. KeePassXC sends messages one by one, but the keepassxc-proxy-access side of the socket has no info about the length of a message transferred and so depends on reading what is transferred over the socket.

Reading is done with a ByteBuffer, so in case KeePassXC sends a lot of messages at once, this leads to appended messsages for one read operation on the reading side.

This behaviour of KeePassXC violates the keepassxc-browser protocol, so in the end there is no sense in splitting up the messages and processing them separately.

Instead, this case needs to be treated as an error and handled accordingly. This is implemented by 1fa01c600763d9e7bcac4ffd1df90eab5e1fc74c.