Implement framework for flexible 2FA #373

BryanJacobs · 2024-01-15T05:27:55Z

This adds support for using the hmac-secret FIDO extension to contribute keying material for a KeePass 4 file.

It does this by storing an additional XML statekeeping blob in the outer ("public") header. This blob is designed to hold a variety of different authentication factors, such as passwords, key files, and Yubikey challenge-response devices.

The implementation here has support for passwords (primitively - no brute-force resistance), key files, and FIDO2 authenticators. It contains documentation about the changes to the KeePass file format, and tests covering the basic functionality.

It adds a dependency on python-fido2 to perform actual FIDO2 operations with connected PC/SC or USB-HID authenticators.

BryanJacobs · 2024-01-15T05:28:50Z

.gitignore

@@ -9,3 +9,4 @@ Pipfile.lock
 *.kdbx
 *.kdbx.out
 .idea
+.venv


Not using an isolated Python environment rapidly leads to a mess. I've marked the demi-standard .venv directory as ignored to make it easier to keep a virtual environment inside the project folder.

BryanJacobs · 2024-01-15T05:30:15Z

pykeepass/multifactor_format.rst

This file isn't just for pykeepass. I'm going to implement support for this in other projects next, and figured it would be good to have a reference, since the whole KeePass file format isn't centrally owned.

BryanJacobs · 2024-01-15T05:30:47Z

pykeepass/__init__.py


 from .version import __version__

-__all__ = ["PyKeePass", "create_database", "__version__"]
+__all__ = ["PyKeePass", "create_database", "__version__", 'FactorInfo', 'FactorGroup', 'FIDO2Factor', 'KeyFileFactor']


As these classes are necessary to make a database start using multifactor auth, I figured it made sense to expose them at the top level.

BryanJacobs · 2024-01-15T05:31:18Z

pykeepass/fido2.py

This file - and only this file - contains code for talking to FIDO2 authenticators.

The two operations we perform are, of course, make-credential and get-assertion.

BryanJacobs · 2024-01-15T05:31:51Z

pykeepass/kdbx_parsing/common.py

@@ -105,7 +104,49 @@ def aes_kdf(key, rounds, key_composite):
    return hashlib.sha256(transformed_key).digest()


-def compute_key_composite(password=None, keyfile=None):
+def compute_keyfile_part_of_composite(keyfile):


Factored out of compute_key_composite

BryanJacobs · 2024-01-15T05:32:35Z

pykeepass/kdbx_parsing/common.py

@@ -173,6 +181,30 @@ def compute_master(context):
    return master_key


+def populate_custom_data(kdbx, d):


I found working with construct extremely difficult.

BryanJacobs · 2024-01-15T05:33:43Z

pykeepass/pykeepass.py

@@ -152,14 +176,19 @@ def save(self, filename=None, transformed_key=None):
        if not filename:
            filename = self.filename

+        if hasattr(self.kdbx.header, 'data'):


I noticed that if you called save() after changing anything in the header it didn't really save because the binary representation was already there...

BryanJacobs · 2024-01-15T05:34:40Z

pykeepass/pykeepass.py


-    keepass_instance.save(transformed_key)
+    keepass_instance.save(transformed_key=transformed_key)


I think this was bugged before? If I understand correctly it would pass transformed_key as the filename. So I guess nobody really used this function that way?

BryanJacobs · 2024-01-15T05:34:50Z

pyproject.toml

@@ -14,6 +14,7 @@ dependencies = [
    "argon2_cffi",
    "pycryptodomex>=3.6.2",
    "lxml",
+    "fido2[pcsc]"


New dependency

BryanJacobs · 2024-01-15T05:35:11Z

requirements.txt

@@ -1,5 +1,6 @@
-lxml==4.8.0
+lxml==5.1.0


Version bump to work with fido2. Doesn't seem to break things.

BryanJacobs · 2024-01-15T05:35:50Z

pykeepass/kdbx_parsing/factorinfo.py

This file contains most of the implementation heavy lift.

BryanJacobs · 2024-01-15T07:05:56Z

pykeepass/kdbx_parsing/factorinfo.py

+            digest = hmac.new(decrypted_key, self.group.validation_in, 'SHA-512').digest()
+        else:
+            # Can't verify, we don't know how
+            pass


In the event it's wrong we'll just end up failing to open the database later, as the constructed composite key won't pass the header checksum.

BryanJacobs · 2024-01-15T07:08:18Z

pykeepass/kdbx_parsing/factorinfo.py

+
+        next_challenge = random.randbytes(32)
+
+        if len(fido2_factors) > 0:


Rather important (and very inconvenient to implement...) feature here. We make one FIDO2 call per authenticator per group.

If there are ten Factor entries, each one of which is a FIDO2 authenticator... that means for each authenticator one would succeed and nine would fail.

If there's one authenticator plugged into the system it would not be great for the user to have to type their PIN ten times.

So instead we gather all the credentials in the group and send them in one call. The authenticator will accept the credential it created, and ignore the others.

BryanJacobs · 2024-01-15T07:10:12Z

pykeepass/fido2.py

+        ],
+        extensions={
+            "hmacCreateSecret": True,
+            "credentialProtectionPolicy": CredProtectExtension.POLICY.REQUIRED,


This forces the user to provide "verification" (a PIN, a fingerprint, etc - not just a touch) every time the credential is used.

This is important because the hmac-secret extension says that the keys used are silently different for UV (user verification) vs non-UV scenarios. It would not be great if the key were created with non-UV, and then later the authenticator is set to always-UV mode and so it becomes impossible to get the original key again.

BryanJacobs · 2024-01-15T07:12:05Z

pykeepass/fido2.py

+            PublicKeyCredentialDescriptor(
+                type=PublicKeyCredentialType.PUBLIC_KEY,
+                id=credential_id
+            ) for credential_id in already_enrolled_credentials


Passing the credentials already in the group here prevents adding the same authenticator to the same group twice.

You can still add the same authenticator to two different groups, and of course to two different files. But I don't think it makes sense to have two credentials from the same authenticator in one group...

Evidlo · 2024-02-29T06:44:01Z

Closes #311

This looks good so far. I'll ask to move the contents of multifactor_auth.rst into a new section in the README as that's where our examples currently live.

I'll hold off on merging until the thread over at keepassxc is mostly resolved.

BryanJacobs · 2024-03-02T23:55:42Z

Sorry, accidentally auto-closed as I updated my branch. I will reopen this momentarily with your requested changes.

BryanJacobs commented Jan 15, 2024

View reviewed changes

pykeepass/kdbx_parsing/factorinfo.py Outdated

Copy link

Author

BryanJacobs Jan 15, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This file contains most of the implementation heavy lift.

BryanJacobs commented Jan 15, 2024

View reviewed changes

BryanJacobs mentioned this pull request Feb 16, 2024

Groundwork to support flexible multifactor database authentication and FIDO2 keepassxreboot/keepassxc#10311

Open

Evidlo mentioned this pull request Feb 29, 2024

Support for Yubikey / Hardware Key #311

Open

BryanJacobs closed this Mar 2, 2024

BryanJacobs force-pushed the master branch from 8391f91 to 769ee25 Compare March 2, 2024 23:54

BryanJacobs mentioned this pull request Mar 3, 2024

Implement framework for flexible 2FA #379

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement framework for flexible 2FA #373

Implement framework for flexible 2FA #373

BryanJacobs commented Jan 15, 2024

BryanJacobs Jan 15, 2024 •

edited

Loading

BryanJacobs Jan 15, 2024

BryanJacobs Jan 15, 2024

BryanJacobs Jan 15, 2024 •

edited

Loading

BryanJacobs Jan 15, 2024

BryanJacobs Jan 15, 2024 •

edited

Loading

BryanJacobs Jan 15, 2024

BryanJacobs Jan 15, 2024

BryanJacobs Jan 15, 2024

BryanJacobs Jan 15, 2024

BryanJacobs Jan 15, 2024

BryanJacobs Jan 15, 2024

BryanJacobs Jan 15, 2024

BryanJacobs Jan 15, 2024

BryanJacobs Jan 15, 2024

Evidlo commented Feb 29, 2024

BryanJacobs commented Mar 2, 2024

		@@ -173,6 +181,30 @@ def compute_master(context):
		return master_key


		def populate_custom_data(kdbx, d):


		keepass_instance.save(transformed_key)
		keepass_instance.save(transformed_key=transformed_key)


		next_challenge = random.randbytes(32)

		if len(fido2_factors) > 0:

		@@ -1,5 +1,6 @@
		lxml==4.8.0
		lxml==5.1.0

Implement framework for flexible 2FA #373

Implement framework for flexible 2FA #373

Conversation

BryanJacobs commented Jan 15, 2024

BryanJacobs Jan 15, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

BryanJacobs Jan 15, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

BryanJacobs Jan 15, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Evidlo commented Feb 29, 2024

BryanJacobs commented Mar 2, 2024

BryanJacobs Jan 15, 2024 •

edited

Loading

BryanJacobs Jan 15, 2024 •

edited

Loading

BryanJacobs Jan 15, 2024 •

edited

Loading