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

Rest gateway #1355

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

DiegoTavares
Copy link
Collaborator

@DiegoTavares DiegoTavares commented Apr 12, 2024

Summarize your change

Create a service to expose a REST endpoint for the grpc interface. The motivation behind having a REST endpoint is to create a web version of cuegui (coming soon).

How does it work

See the module README for a full documentation.

@ddneilson
Copy link

ddneilson commented Apr 24, 2024

Hi there. A drive-by comment prompted by this PR getting surfaced in the TSC meeting. I understand that this REST gateway is being designed without authorization/authentication, with an expectation that perimeter/firewall security is sufficient protection. As a point against this posture, please do consider the possibility of phishing attacks, or internal threat actors.

For example, it is possible for an attacker to send someone inside of the network a phishing email that contains code or clickable links that send a web request to the webservice that's, otherwise, only accessible within the network. This becomes relatively trivial if the webservice is completely unauthenticated, and opens the render farm up to remote exploits. Similarly, an internal threat actor could send whatever requests they would like to the gateway to affect behavior of the farm.

Render farms are remote execution platforms, by definition, so an attacker gaining access to the webservice could submit work (mining bitcoin, for instance), or exfiltrate information about secret projects that are running on the farm. I would suggest a threat modelling exercise be undertaken to understand the risks inherent in a component such as this, and the implementation be modified to mitigate the threats uncovered. Some things to think about include: How would the threat be mitigated? How would an attack be detected? What audit trail exists to discover the source of the attack? etc.

@DiegoTavares
Copy link
Collaborator Author

Hi there. A drive-by comment prompted by this PR getting surfaced in the TSC meeting. I understand that this REST gateway is being designed without authorization/authentication, with an expectation that perimeter/firewall security is sufficient protection. As a point against this posture, please do consider the possibility of phishing attacks, or internal threat actors.

For example, it is possible for an attacker to send someone inside of the network a phishing email that contains code or clickable links that send a web request to the webservice that's, otherwise, only accessible within the network. This becomes relatively trivial if the webservice is completely unauthenticated, and opens the render farm up to remote exploits. Similarly, an internal threat actor could send whatever requests they would like to the gateway to affect behavior of the farm.

Render farms are remote execution platforms, by definition, so an attacker gaining access to the webservice could submit work (mining bitcoin, for instance), or exfiltrate information about secret projects that are running on the farm. I would suggest a threat modelling exercise be undertaken to understand the risks inherent in a component such as this, and the implementation be modified to mitigate the threats uncovered. Some things to think about include: How would the threat be mitigated? How would an attack be detected? What audit trail exists to discover the source of the attack? etc.

Thanks for the feedback Daniel, I acknowledge the risks involved in exposing a REST API, even on an internal network environment. I'm working on adding oauth authentication to the REST gateway module to ensure any non-authorized access gets blocked.

@DiegoTavares DiegoTavares changed the title Rest gateway Draft: Rest gateway May 21, 2024
DiegoTavares and others added 2 commits August 9, 2024 15:38
Merge changes from local fork into ASWF repo

---------

Signed-off-by: Diego Tavares <[email protected]>
…it testing (AcademySoftwareFoundation#1453)

Features and improvements

1. JWT authentication for gRPC REST gateway
- Integrated JWT authentication for secure communication.
- Implemented middleware to validate JWT tokens from HTTP requests'
authorization headers.
2. Include unit testing
- Added Go tests for the REST gateway to ensure authentication
middleware functionality and error handling.

Co-authored-by: Zachary Fong <[email protected]>
Co-authored-by: Ramon Figueiredo <[email protected]>
Co-authored-by: Diego Tavares <[email protected]>
@ramonfigueiredo
Copy link
Collaborator

Hi @DiegoTavares

Remember to merge the branch https://github.com/AcademySoftwareFoundation/OpenCue/tree/rest_gateway into your rest_gateway again.

See PR: [rest_gateway] Add authentication to gRPC REST gateway and include unit testing
#1453

…ion, and examples (AcademySoftwareFoundation#1470)

- Improved Introduction for better clarity on the Rest Gateway's
purpose.
- A detailed guide on JWT authentication and its usage was added.
- Included a table of contents for easy navigation.
- Provided clearer examples of how to interact with the REST interface.
- Updated section on unit testing.

Co-authored-by: Ramon Figueiredo <[email protected]>
Co-authored-by: Zachary Fong <[email protected]>

---------

Co-authored-by: Zachary Fong <[email protected]>
@DiegoTavares
Copy link
Collaborator Author

@ddneilson A solution for authentication has been implemented as suggested on your previous comment.
See the README for more details on how it works. It would be great to hear your opinion on this.

@DiegoTavares DiegoTavares changed the title Draft: Rest gateway Rest gateway Sep 11, 2024
@DiegoTavares
Copy link
Collaborator Author

@bcipriano Can you review this PR anytime this week? As both Ramon and myself have been involved in the development, it makes sense to have somebody else as the reviewer.

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