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

Tor password authentication #746

Merged
merged 24 commits into from
Jun 16, 2022
Merged

Tor password authentication #746

merged 24 commits into from
Jun 16, 2022

Conversation

MadcowOG
Copy link
Contributor

@MadcowOG MadcowOG commented May 4, 2022

Added the ability for user to authenticate tor control ports with passwords. It could be a better alternative to cookies, as passwords doesn't require changing file permissions.

@MadcowOG
Copy link
Contributor Author

MadcowOG commented May 4, 2022

I apologize for taking up so many workflow runs, I'm still trying to figure things out.

@MadcowOG MadcowOG changed the title Tor auth Tor password auth May 4, 2022
@MadcowOG MadcowOG changed the title Tor password auth Tor password authentication May 4, 2022
@nakoo
Copy link
Contributor

nakoo commented May 5, 2022

For privacy perspective, I don't think it's a good idea to implement password instead of cookie. In general, I don't understand why we need to give user permission to change the tor control port by risking server security.

@MadcowOG
Copy link
Contributor Author

MadcowOG commented May 5, 2022

I understand. I think this could provide more options to the user, as neither option is directly more or less secure than the other. With the way I'm thinking this should be set up, it would restrict whoogle's access to the file system much more.

@MadcowOG
Copy link
Contributor Author

MadcowOG commented May 6, 2022

the tor control port by risking server security.

What about this method could be a security risk?

@nakoo
Copy link
Contributor

nakoo commented May 6, 2022

the tor control port by risking server security.

What about this method could be a security risk?

I read your code. The only thing this code do is deciding whether server needs to enable Tor or not. It's unnecessary extra step to disable Tor connection. Manging password makes hard to secure Docker container too.

@MadcowOG
Copy link
Contributor Author

MadcowOG commented May 6, 2022

I apologize, I am confused.

The only thing this code do is deciding whether server needs to enable
Tor or not. It's unnecessary extra step to disable Tor connection.

Didn't the code previous to my changes do this already? I just made it so the code determines this factor through passwords as well.

Manging password makes hard to secure Docker container too.

My code doesn't change anything with Docker, since by default cookies are only enabled in torrc in Docker, it doesn't even use password authentication in Docker, correct? I think you are right though passwords shouldn't be used in Docker.

@MadcowOG
Copy link
Contributor Author

MadcowOG commented May 6, 2022

The only thing this code do is deciding whether server needs to enable
Tor or not. It's unnecessary extra step to disable Tor connection.

Sorry, do you mean the attempt to always authenticate passwords with multiple try and except blocks is unnecessary? If so, I agree, I will get on that.

@MadcowOG
Copy link
Contributor Author

MadcowOG commented May 6, 2022

The only thing this code do is deciding whether server needs to enable Tor or not. It's unnecessary extra step to disable Tor connection.

@nakoo I think I understand what you were talking about here. Did my recent commits fix the issue with unnecessary extra steps?

Copy link
Owner

@benbusby benbusby left a comment

Choose a reason for hiding this comment

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

Thanks!

@benbusby benbusby merged commit c9ee9dc into benbusby:main Jun 16, 2022
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.

3 participants