Menu ▾ ▴

#2654 Cannot open Twofish encrypted db written by Keepass2Android

KeePass_2.x
closed
nobody
None
5
2021-07-27
2021-07-16
gwashburn
No

I'm running into an issue where databases using Twofish encryption when saved or exported by Keepass2Android (KP2A) are not able to be opened by KeePass2 with the TwofishCipher plugin. This is a known issue with KP2A, and is because KP2A is using TwofishCipher version < 1.4, which had a padding bug. If KP2A fixes the bug as was done for the plugin, then on upgrade Twofish KDBX files will not be readable.

When opening the db an error dialog pops up saying the file failed to load and when using the --debug option, shows a stacktrace with an XML error (which threw me off on a much larger database). See attached screenshot. Also attached is an kdbx file that triggers this exception, the password is asdf.

There is no reason for KeePass2 to need to care about the padding bytes. The hashed block stream format ensures that the padding data will not be confused with data we care about. After reading through the source, I can't find where keepass2 would even be using data past the last hashed block. However, it must be so because I've constructed kdbx files that are identical except for the padding and one successfully loads while the other does not.

I know this is not strictly a keepass2 issue, however, it seems to me the best place to resolve this is here.

2 Attachments
keepass2-twofish-exception.png
test.kp2a.kdbx

Discussion

  • Paul

    Paul - 2021-07-17

    This is not fixable in KeePass because the encryption is done by a 3rd party plug-in.
    http://gogogadgetscott.info/keepass/twofishcipher/

    The only solution I can see is to stop using Twofish.

    cheers, Paul

     

  • gwashburn

    gwashburn - 2021-07-21

    Can you please help me to understand why what you say is true? Why isn't the exception coming from the Twofish plugin, instead of the XML parser?

    It is my understanding that the encryption plugin only decrypts the hashed block stream. From there KeePass2 code reads the XML database from the hashed block stream. Since the exception is being thrown in the XML parser, that says to me that the decryption done by the plugin has been successful and that its KeePass2's interpretation of the decrypted data that is causing problems.

    Also no exception was thrown from HashedBlockStream::ReadSafeBlock, which says to me that there is no problem with the hashed block stream (unless that exception is getting ignored somewhere).

    Since its only a change in padding that is causing this error, my guess has been that KeePass2 is appending the padding data to the XML database data for some reason. Another explication now occurs to me. Perhaps an exception in the Twofish plugin or in ReadSafeBlock is being ignored and the XML parser is being given a truncated version of the XML data.

    I do not have a development environment for mono setup, nor an I familiar with debugging C# programs. I have provided a KDBX file that causes this issue. Can you or another developer tell me what is the string being passed to the XML parser? If its truncated XML, then I would suspect that an exception is getting swallowed, in which case we should not do that so that errors are not covered up. If the string is a complete XML database with appended padding bytes, then there's an issue in KeePass2 in how its getting data from the hashed block stream.

    Either way, there's an issue in KeePass2 in that it is showing a confusing error to the user. How is the user to know the issue is actually with a plugin and not their database? I see potential for some code improvements here.

     

  • Paul

    Paul - 2021-07-22

    KeePass doesn't need to have error messages that show 3rd party plug-in errors. It is up to the plug-in author to produce valid data and manage error messages.

    This doesn't help you with the issue of opening old databases.
    I would keep an old KeePass / plug-in as a portable setup to enable you to recover the data.

    cheers, Paul

     

    • gwashburn

      gwashburn - 2021-07-22

      I believe there must be a miscommunication here. I am not asking for "error messages that show 3rd party plug-in errors". I have reviewed the code based, but I'm still not that familiar with it, so there may be an error in my logic. My understanding is that if the Twofish plugin threw an exception that it would bubble up to KeePass. The question then becomes, what would KeePass do with it? I would expect it to not just swallow/ignore it, but to in some way bubble it up to the UI. Are you suggesting that it is up to the plugin to catch all exceptions and send a mesage to the UI independently from KeePass?

      In order for the Twofish plugin to properly validate the decrypted HashedBlockStream it would need to implement essentially the same code, which makes no sense, when KeePass already has that code.

      The problem isn't old databases, its current databases written by Keepass2Android that use the old Twofish plugin. I have a work around where I can recreated the database with correct padding using the python libkeepass library. Its not for the faint of heart though, and it a pain in the ass. The fact that I can do it though, means that so can KeePass, which is what this issue is about.

       

  • Dominik Reichl

    Dominik Reichl - 2021-07-22
    • status: open --> closed
    • Priority: 3 --> 5
     

  • Dominik Reichl

    Dominik Reichl - 2021-07-22

    Padding is handled by the encryption stream. So, in the case of Twofish, it's handled by the plugin, not by KeePass itself.

    Trying to not read streams to the end is not a proper solution in my opinion. Although this might work for KDBX 3.1 files (not sure, haven't checked it in detail), it's unlikely that it would work for KDBX ≥ 4 files, because there we have an Encrypt-then-MAC construction (i.e. the decryption happens after verifying the blocks). KeePass uses classes of the framework (XML parser, GZip implementation, etc.) and we have no control about whether these read streams to the end; they may check for invalid trailing data or use buffering.

    One solution would be that the Twofish plugin provides two different encryption algorithms: one ('Twofish') with the original cipher UUID and another one ('Twofish (New)' or something like that) with a different cipher UUID that uses the new padding. With this, it would be clear whether/which padding is used. Of course, given that the Twofish plugin version 1.4 has been released a while ago, it might be a bit late for this solution, but you could certainly ask the plugin developer.

    Thanks and best regards,
    Dominik

     

    • gwashburn

      gwashburn - 2021-07-22

      Hi Dominik, first, thanks for all the work on KeePass2.

      I understand that padding is handled by the encryption stream, ie the Twofish plugin. What I'm not asking is for KeePass2 to do anything with the padding. The real issue here is that KeePass seems to not handle garbage at the end of the HashedBlockStream, which in this case are 1-15 NULL bytes. What I'm saying is that for KDBX 3.1 , there is no reason for trailing garbage in the HashedBlockStream to affect anything, and yet it currently does. I assert this because each block in the HashedBlockStream specifies its length and there's and ending NULL block, so we know when the stream ends. Anything after the NULL block should be ignored.

      I'm unsure if this also applies to KDBX ≥ 4 . The HmacBlockStream also specifies the length for each block, but I'm not sure if there is an end of stream block or a way to determine that the rest of the stream need not be read.

      So instead of talking about the encryption, which is a red herring here, can we just talk about how KeePass (ungracefully) handles hashed block streams with trailing garbage? Why is it that KeePass throws and exception coming from the XML parser? Isn't that an issue in and of itself?

      The alternative solution you provide seems like a good idea, though it doesn't solve this particular issue (only doesn't trigger it). Perhaps a version 1.5 should be released with a new UUID and everyone with 1.4 should upgrade to that.

      Thanks for you consideration,
      Glenn

       

  • gwashburn

    gwashburn - 2021-07-27

    Looking into this further, what's happening is that the encryption plugin is throwing an exception based on the incorrect padding and the exception is being swallowed in KdbxFile:CloseStreams, which is why the actual exception bubbled up to the UI is the xml parse error. Since we now know that the problem is originating from an exception in the TwofishCipher plugin due to incorrect padding, then I agree there's no change in KeePass that will allow KeePass to read the database. So this feature request is indeed resolved.

    However, the issue of the misleading error message remains and I'll file a bug.

     

Log in to post a comment.

MongoDB Logo MongoDB