Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Add Authlib backend and test compatibility with Askar #14

Merged
merged 7 commits into from
Nov 13, 2023

Conversation

dbluhm
Copy link
Member

@dbluhm dbluhm commented Nov 12, 2023

This PR adds an Authlib CryptoService backend.

To be perfectly frank, I'm not a fan of Authlib. For the functions we need from it, it is poorly documented and difficult to follow. My main goal in adding this backend is to (1) exercise the dependency inversion by having more than one backend around and (2) diagnose and debug compatibility issues we're seeing between this library and didcomm-python.

And add a failing test for further debugging

Signed-off-by: Daniel Bluhm <[email protected]>
Signed-off-by: Daniel Bluhm <[email protected]>
@dbluhm
Copy link
Member Author

dbluhm commented Nov 13, 2023

Figured out what the issue was after much trial and tribulation: there were values that were b64 encoded and the ascii bytes of the encoded value were being used as part of the key derivation process instead of the decoded bytes. Specifically, apu and apv in ECDH-1PU and apv in ECDH-ES. Correctly decoding the bytes solved our interop issues (see passing tests).

Copy link
Contributor

@TheTechmage TheTechmage left a comment

Choose a reason for hiding this comment

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

Aside from the broken import, I think this looks good overall

didcomm_messaging/crypto/backend/basic.py Outdated Show resolved Hide resolved
Failing at the moment

Signed-off-by: Daniel Bluhm <[email protected]>
Copy link
Contributor

@TheTechmage TheTechmage left a comment

Choose a reason for hiding this comment

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

LGTM

@dbluhm dbluhm merged commit 675a4c6 into main Nov 13, 2023
5 checks passed
@dbluhm dbluhm deleted the feature/authlib branch November 13, 2023 16:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants