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

JSON Management Not Honoring Password #1149

Open
sduensin opened this issue Sep 13, 2023 · 5 comments
Open

JSON Management Not Honoring Password #1149

sduensin opened this issue Sep 13, 2023 · 5 comments
Assignees

Comments

@sduensin
Copy link

The JSON Management API is allowing password-protected operations without a password. Example:

$ echo w 1 stop | nc -w1 -u 127.0.0.1 5644
{"_tag":"1","_type":"error","error":"badauth"}
{"_tag":"1","_type":"begin","cmd":"stop"}
{"_tag":"1","_type":"row","keep_running":0}
{"_tag":"1","_type":"end","cmd":"stop"}

This is using edge 3.1.1-239-g9624a65.

@hamishcoleman hamishcoleman self-assigned this Sep 13, 2023
@hamishcoleman
Copy link
Collaborator

That is a little embarrassing. Thanks for finding it - and a very useful minimal reproducer.
This was caused during some refactoring I did a while ago now. I'll figure out a good patch and fix it.

Additionally, since you are using the API - this api turned out to be not as usable across all the different platforms (It basically needs more code to implement clients than would be good)

I'm thinking it should be replaced with a real JsonRPC interface.

@sduensin
Copy link
Author

The client I wrote to talk to the current JSON management API is only 58 lines of code (in GDScript of all things). It wasn't terrible. 😀 However, being able to use an existing JSON RPC library would be nice as well.

@hamishcoleman
Copy link
Collaborator

Yeah, this was less about how hard it is to code a client and more about reuse of existing infra - a JsonRPC API can have a zero-line client if you serve some static HTML and simply use the existing web browser. (And there are also some handy naming + unix domain sockets with a server running multiple instances improvement that almost fall out for free)

I've pushed a patch for the auth to the same purgable branch you are already testing. However - it is worth noting that the old human-based management interface is still in the codebase and has no authentication

@sduensin
Copy link
Author

Awesome. I'll pull it and give it a go. I don't expose the management port on anything except localhost so it's not that big a deal that the old interface is still there.

@hamishcoleman
Copy link
Collaborator

This new JsonRPC API was implemented in the recently released fork n3n, if you were interested in looking at it.

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

No branches or pull requests

2 participants