Protect Profile with Password (#23) #63

angus · 2018-06-24T19:15:30Z

angus commented

2018-06-24 19:15:30 +00:00

Apologies for the amateurish errors, hopefully fixed here and a little cleaner. Test script ran cleanly.

Getting the below error in runtime when running new-profile or load-profile:
"could not connect to Tor Control Port 9051 "

Not one I've seen before the rebasing, so not sure if it's at my end or not? Wanted to get this code in front of you ASAP just in case it's a separate issue being worked on/a unique config thing at my end.

Apologies for the amateurish errors, hopefully fixed here and a little cleaner. Test script ran cleanly. Getting the below error in runtime when running new-profile or load-profile: "could not connect to Tor Control Port 9051 <nil>" Not one I've seen before the rebasing, so not sure if it's at my end or not? Wanted to get this code in front of you ASAP just in case it's a separate issue being worked on/a unique config thing at my end.

sarah commented

2018-06-24 19:18:43 +00:00

Ah that's because you need to be running a Tor daemon with the following torrc file

    SOCKSPort 9050
    ControlPort 9051

Cwtch uses that to set up and communicate with onion services.

Ah that's because you need to be running a Tor daemon with the following torrc file SOCKSPort 9050 ControlPort 9051 Cwtch uses that to set up and communicate with onion services.

angus commented

2018-06-24 19:46:36 +00:00

I could have sworn that’s my config! Has the required config changed in the past week? I’ll confirm I’ve got that when I get back to my laptop

angus commented

2018-06-26 02:08:06 +00:00

Confirmed, my config has those two lines... My browser can use Tor as a SOCKS proxy no problem and is browsing around. Weird thing is, Wireshark is showing the exchange is

Request: GET / HTTP/1.1\r\n
Return: HTTP/1.0 501 Tor is not an HTTP Proxy

Seems odd it's making HTTP requests over SOCKS? I always assume I'm missing something but I've changed nothing at my end and the request itself seems to be off. Sorry if I have missed something though.

FWIW the code seems to work fine, even if I can't establish a network connection.

Confirmed, my config has those two lines... My browser can use Tor as a SOCKS proxy no problem and is browsing around. Weird thing is, Wireshark is showing the exchange is Request: GET / HTTP/1.1\r\n Return: HTTP/1.0 501 Tor is not an HTTP Proxy Seems odd it's making HTTP requests over SOCKS? I always assume I'm missing something but I've changed nothing at my end and the request itself seems to be off. Sorry if I have missed something though. FWIW the code seems to work fine, even if I can't establish a network connection.

sarah commented

2018-06-26 04:21:05 +00:00

Cwtch tried to open a tor connection as HTTP to check that it is in-fact speaking to the Tor daemon (https://git.openprivacy.ca/cwtch.im/cwtch/src/master/connectivity/tor/tormanager.go#L42) which is why you are seeing HTTP there.

The code will run but it won't actually do any networking stuff or load a Cwtch peer properly without the control connection.

Cwtch tried to open a tor connection as HTTP to check that it is in-fact speaking to the Tor daemon (https://git.openprivacy.ca/cwtch.im/cwtch/src/master/connectivity/tor/tormanager.go#L42) which is why you are seeing HTTP there. The code will run but it won't actually do any networking stuff or load a Cwtch peer properly without the control connection.

sarah commented

2018-06-26 04:29:52 +00:00

Ah nm, there is a bug in the control port connect code, it outs an error even if it is nil. I'll fix that!

sarah commented

2018-06-26 04:45:18 +00:00

As for the code, this looks good and I've run it and it works but has 1 big bug:

Steps:

Create a new profile [alice]
Quit
Load Profile [alice]
Save
Quit

At this point if you xxd alice's profile file you will find that the salt value has been nulled out,so the first 128 bytes are 0x00. So it's now impossible to load Alice's profile again. Looks like LoadProfile needs a copy(cpc.salt,salt)` or something similar.

It would also be good if you could expand come of the documentation around the code - particularly the comments and parameter names e.g. CreateKey currently has the parameter key but this should probably be called password. CreateKey, Encrypt and Decrypt shouldn't be exportable (they should being with lower case letters) as we don't expect users to call them.

Otherwise this is looking really cool!

As for the code, this looks good and I've run it and it works but has 1 big bug: Steps: - Create a new profile [alice] - Quit - Load Profile [alice] - Save - Quit At this point if you `xxd` alice's profile file you will find that the salt value has been nulled out,so the first 128 bytes are `0x00`. So it's now impossible to load Alice's profile again. Looks like `LoadProfile` needs a copy(cpc.salt,salt)` or something similar. It would also be good if you could expand come of the documentation around the code - particularly the comments and parameter names e.g. `CreateKey` currently has the parameter `key` but this should probably be called `password`. `CreateKey`, `Encrypt` and `Decrypt` shouldn't be exportable (they should being with lower case letters) as we don't expect users to call them. Otherwise this is looking really cool!

angus commented

2018-06-27 00:39:29 +00:00

Ok, remerged the bug fix, everything seems to work fine from my end on the correction.
Added the salt back into the cwtchPeer in the LoadCwtchPeer function and made other changes as noted. Fixed the nonsense comments I had in there. Hopefully we're there!

Ok, remerged the bug fix, everything seems to work fine from my end on the correction. Added the salt back into the cwtchPeer in the LoadCwtchPeer function and made other changes as noted. Fixed the nonsense comments I had in there. Hopefully we're there!

sarah closed this pull request

2018-06-27 01:15:03 +00:00

sarah commented

2018-06-27 01:15:18 +00:00

Awesome! And thanks!

Sign in to join this conversation.

No reviewers