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: Re-issue/send PDAs when first-party endpoint certificates are renewed #212

Conversation

gnarea
Copy link
Member

@gnarea gnarea commented Apr 12, 2022

Fixes #185. It's also worth highlighting the following:

  • It introduces the instance method FirstPartyEndpoint.authorizeIndefinitely(), which is like FirstPartyEndpoint.issueAuthorization(), except that it tracks which 3rd party endpoints are authorised in order to renew their PDAs automatically.
    • For example, Awala Ping will continue to use FirstPartyEndpoint.issueAuthorization() but Letro will use FirstPartyEndpoint.authorizeIndefinitely().
    • To stop renewing an indefinite PDA, it's enough to delete the respective 1st or 3rd party endpoint.
  • This PR introduces the concept of "channel" (e.g., ChannelManager), which exists in the Awala protocol suite and refers to an established communication channel between two endpoints (which is end-to-end encrypted and where both are mutually authorised to send messages to each other).
  • Because Re-register first-party endpoints when the private gateway's certificate changes #175 isn't implemented yet, I haven't actually integrated the function to reissue+send PDAs (FirstPartyEndpoint.reissuePDAs()). This should be done as part of Re-register first-party endpoints when the private gateway's certificate changes #175.
  • This PR integrates CertificationPath from the core Awala lib, which effectively duplicates the existing AuthorizationBundle data class. Once this PR is merged, I'll create a separate PR to replace the remaining uses of AuthorizationBundle.

Review notes

Of things that I'm likely to have got wrong here, I'd rank highly the use of SharedPreferences and coroutines (esp. where I have to specify a context explicitly), so please pay special attention to those.

kodiakhq bot pushed a commit to relaycorp/awala-jvm that referenced this pull request Apr 13, 2022
With its own serialisation.

Needed in relaycorp/awala-endpoint-android#212

TODO:

- [x] Implement serialisation
- [x] Define `CertificationPath` in PKI spec.
- [x] Implement `validate()` method, which uses `path.leafCertificate.getCertificationPat(emptySet(), listOf(path.chain.last()))`
@gnarea gnarea marked this pull request as ready for review April 13, 2022 16:58
kodiakhq[bot]
kodiakhq bot previously approved these changes Apr 13, 2022
@gnarea gnarea requested a review from sdsantos April 13, 2022 16:58
@gnarea
Copy link
Member Author

gnarea commented Apr 13, 2022

Hmm, one thing I just realised is that FirstPartyEndpoint.authorizeIndefinitely() and FirstPartyEndpoint.issueAuthorization() must actually return ByteArrays -- otherwise we'd be leaking the Awala lib to the apps using this lib.

sdsantos
sdsantos previously approved these changes Apr 13, 2022
Comment on lines 37 to 38
val channelPreferences =
context.getSharedPreferences("awaladroid-channels", Context.MODE_PRIVATE)
Copy link
Collaborator

Choose a reason for hiding this comment

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

This triggers a disk read immediately, we should delay it until we're inside a suspend function.

Copy link
Member Author

Choose a reason for hiding this comment

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

Right, we don't want that! Good catch


internal class ChannelManager(
sharedPreferences: SharedPreferences,
coroutineContext: CoroutineContext
Copy link
Collaborator

Choose a reason for hiding this comment

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

You could default the context here instead, but that's a small detail.

coroutineContext: CoroutineContext
) {
internal val flowSharedPreferences: FlowSharedPreferences =
FlowSharedPreferences(sharedPreferences, coroutineContext)
Copy link
Collaborator

Choose a reason for hiding this comment

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

It seems you are never using the Flow part of the FlowSharedPreferences, just using it to run things on a certain coroutine context. You could switch it for a normal SharePreferences + withContext() {} around the calls.

But maybe in the future we will need it.

Copy link
Member Author

Choose a reason for hiding this comment

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

👍🏾 I like that! That's really the only reason why I integrated FlowSharedPreferences

@gnarea gnarea dismissed stale reviews from sdsantos and kodiakhq[bot] via 1dbba32 April 14, 2022 11:35
@gnarea gnarea requested a review from sdsantos April 14, 2022 11:35
@gnarea gnarea added the automerge Allow kodiak to automerge commit when all checks pass label Apr 14, 2022
@kodiakhq kodiakhq bot merged commit 6d62572 into main Apr 14, 2022
@kodiakhq kodiakhq bot deleted the 185-re-issue-and-send-pdas-when-first-party-endpoint-certificates-are-renewed branch April 14, 2022 12:09
@github-actions
Copy link

🎉 This PR is included in version 1.8.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
automerge Allow kodiak to automerge commit when all checks pass released
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Re-issue and send PDAs when first-party endpoint certificates are renewed
2 participants