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 crypto explanation for the stream gateway #123

Merged
merged 1 commit into from
Aug 29, 2024

Conversation

morphis
Copy link
Collaborator

@morphis morphis commented Aug 27, 2024

No description provided.

@morphis morphis requested a review from keirthana August 27, 2024 20:38
@morphis morphis force-pushed the add-stream-gateway-crypto branch from e7af0a7 to 5412f53 Compare August 27, 2024 20:39
@morphis morphis force-pushed the add-stream-gateway-crypto branch from 5412f53 to f17feed Compare August 28, 2024 06:49
Copy link
Collaborator

@keirthana keirthana left a comment

Choose a reason for hiding this comment

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

Minor comments, otherwise looks good

explanation/cryptography/crypto_stream_gateway.md Outdated Show resolved Hide resolved
explanation/cryptography/crypto_stream_gateway.md Outdated Show resolved Hide resolved
explanation/cryptography/crypto_stream_gateway.md Outdated Show resolved Hide resolved
Copy link
Contributor

github-actions bot commented Aug 28, 2024

Everyone contributing to this PR have now signed the CLA. Thanks!

@morphis morphis force-pushed the add-stream-gateway-crypto branch from d55e349 to 19ab117 Compare August 28, 2024 18:36
@morphis morphis requested a review from keirthana August 28, 2024 18:36
Copy link
Collaborator

@keirthana keirthana left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@jat-canonical jat-canonical left a comment

Choose a reason for hiding this comment

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

One questions otherwise LGTM


## Token based authentication

Users can generate API tokens to authenticate with the HTTP API provided by the Anbox Stream Gateway. For the API tokens, a scope-limited [Macaroon](http://theory.stanford.edu/~ataly/Papers/macaroons.pdf) is used. The token is signed with a [HMAC](https://www.okta.com/identity-101/hmac/) using SHA-256 (HS256) and a 64 byte secret key. The [`macaroon.New`](https://pkg.go.dev/gopkg.in/[email protected]#New) method is used internally to generate the [JWT](https://jwt.io/) token.
Copy link
Contributor

Choose a reason for hiding this comment

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

The macaroon.New method is used internally to generate the JWT token.

I wonder if this is necessary to include as this leaks internal implementation detail which might not be suitable for the nature of our product ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

That's fine. We reference the package we use already and drawing a conclusion to what method is used from it is straightforward.

Copy link
Contributor

@ajanon ajanon left a comment

Choose a reason for hiding this comment

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

LGTM

@keirthana keirthana merged commit 322d6c3 into canonical:main Aug 29, 2024
3 checks passed
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.

4 participants