Groundwork to support flexible multifactor database authentication and FIDO2

[BryanJacobs]

Screenshots

No user-visible changes.

Testing strategy

Unit tests for parsing of the header included, and a Kdbx test opening a database secured using header-described password authentication.

Type of change

• ✅ New feature (change that adds functionality)

Relates to https://github.com/keepassxreboot/keepassxc/discussions/9506 .

Description

This PR allows KeePassXC to read files containing an XML blob in their header describing multifactor authentication for the database. This blob can describe a replacement for the existing composite key, or append to it. One or more "groups" can be used, and each group is one-of-N.

Either way, the multifactor methods' output goes through the EXISTING key derivation function - it's just input to the KDF.

The target use case here is to support using one-of-N FIDO authenticators: for that use case, the header would describe a single group with N-many Factors in it.

I have implemented end-to-end support for FIDO2 in this format in KeePassPy at https://github.com/libkeepass/pykeepass/pull/373 , including human-readable documentation of the file format. But this repository is generally cleaner than that one so I figured I'd raise an intermediate PR rather than a big-bang here.

What is Done

1. Parsing an authentication_factors blob from the KDBX4 outer header if present
2. Error handling for the parser
3. On read, using a "sha256 password" type authentication factor if present in the header

What is Not Yet Done

1. Saving files in this format: writing the database clears the header
2. User interface, either CLI or GUI. Note that the existing UI will work if a password is entered and it is valid for one password factor in each group due to how the code is implemented...
3. FIDO2 factor support, which is a significant amount of code on top of this groundwork

Please let me know if this is going in an acceptable direction. If so, I'll keep going, but I'd rather not have a PR with 3000-plus lines in it.

Note that this PR contains two cleanly separated commits and it may be easier to look at the first one first.

[BryanJacobs]

If this general feature works okay I'll implement support for FIDO2 in KeePassDX next, so we'll have compatible implementations everywhere relevant.

[droidmonkey]

Oh this is fun! Will comb through this soon.

[Cyanogenbot]

I am willing to provide my design skills for some of the UI parts, although I'm definitely not a king at programming!

[BryanJacobs]

I've added a commit to attempt to address build failures. I think your test coverage agent is running a different compiler (likely gcc) than I am (clang). It appears there are some minor differences in how the QT libraries work between the two.

If it fails again please forgive me - on my local machine, make coverage passes fine.

[BryanJacobs]

I believe the MacOS build failure is test flake, rather than a problem with this PR.

04:50:16 
  process_add_identity: parse: bignum is negative

04:50:16 
  FAIL!  : TestSSHAgent::testKeyGenRSA() 'agent.addIdentity(key, settings, m_uuid)' returned FALSE. ()

04:50:16 
     Loc: [/Users/KPXC/buildAgent/work/c401303cba1b4098/tests/TestSSHAgent.cpp(242)]

I don't have permission to rerun it, but other than that the automated checks now pass.

[kgraefe]

I believe the MacOS build failure is test flake, rather than a problem with this PR.

04:50:16 
  process_add_identity: parse: bignum is negative

04:50:16 
  FAIL!  : TestSSHAgent::testKeyGenRSA() 'agent.addIdentity(key, settings, m_uuid)' returned FALSE. ()

04:50:16 
     Loc: [/Users/KPXC/buildAgent/work/c401303cba1b4098/tests/TestSSHAgent.cpp(242)]

I don't have permission to rerun it, but other than that the automated checks now pass.

I just stumbled over that myself: https://github.com/keepassxreboot/keepassxc/issues/10320

[BryanJacobs]

I just stumbled over that myself: #10320

There is another piece of flake in TestPassphraseGenerator: sometimes the title-case word generator doesn't produce a string matching the test regex. Overall the tests seem reliable to me though.

[BryanJacobs]

@droidmonkey I think I've addressed all the feedback. I also checked that botan uses PKCS7 padding. PKCS7, when you need N bytes of padding, puts the number N N times.

So a 15-byte message gets 0x01 added to it, a sixteen-byte message gets 0x10 (binary 16) 16 times, and a seventeen-byte message gets 0x0F (binary 15) 15 times. There is always at least one byte of padding, so we either need an unpadded mode here or to use some well-defined padding in the header (for, to my mind, no particular benefit and a file size and complexity increase).

Are you waiting for something else from me here? If this direction is okay I can proceed to implement the saving and FIDO2 parts of the change.

[droidmonkey]

I'm good, will review as I can