-
Notifications
You must be signed in to change notification settings - Fork 72
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
create_config, not .config #215
Comments
Hmm, there might be a couple problems with that:
So perhaps we really do want 2 variants of But perhaps a different alternative is to just |
Other options:
Yet another option that (maybe) directly addresses the use-case:
It would be useful to have some feedback from e.g. @adrelanos or other Whonix (or SubGraph) contributors on what would actually make |
meejah:
- have a "don't query all values" option to `TorConfig`.
Sounds good!
Yet another option that (maybe) directly addresses the use-case:
- `try/except` around the get_conf() and if we get a 500 error,
assume it's filtered and mark that option as "not really available".
So then it would be an error to ask for it, and an error to set it.
I like that less since that would still create lots of noise in
control-port-filter-python logs.
|
Thanks for the feedback @adrelanos :) I'll have to explore the "don't query everything" option a bit further since there might be a lot of sharp corners. Essentially: txtorcon does manipulate the config for certain things. I might just get rid of the "pass any SOCKS port you want and we'll create one" convenience-thing, but it still needs to ask for available SOCKS ports etc. Additionally: is there any way for txtorcon to reliably tell "I'm running against a filtering proxy"? Then e.g. I could trigger the "minimal GETCONF" mode automatically and/or provide better error-messages when things do go wrong (e.g. "option not available" vs. "we think we're talking to a filtering proxy and it won't let us do 'GETCONF whatever'") |
One suggestion: the answer to e.g. something like:
It should be easy for a filtering-proxy to add the |
I agree, that's a good idea. Do you think we can solve this issue mostly by changing the config code? I mean, could we just give
I think that, right now, the first option could be implemented as it does not requires the filters to change. However, I do think that having such a convention for the filters is desirable and having txtorcon adapting to such situation would be great!
I think these would be nice improvements/optimizations to have... which maybe both configs can benefit from?
@adrelanos would probably be able to tell us if these changes would work for other cpfs. I would like to mention that finding a solution to this will also make it possible to use txtorcon on Tails with its cpf, which is the one Whonix forked from. I hope to dedicate more time to this issue this weekend. Thanks @adrelanos and @meejah! |
Felipe Dau:
> So perhaps we really do want 2 variants of TorConfig: one that
> tries to do "minimal" GETCONF queries and one that doesn't.
I agree, that's a good idea. Do you think we can solve this issue
mostly by changing the config code?
I mean, could we just give `Tor`
a different config and it might not even notice because it behaves
like the regular one?
I don't understand. Could you rephrase that please?
@adrelanos would probably be able to tell us if these changes would
work for other cpfs. I would like to mention that finding a solution
to this will also make it possible to use txtorcon on Tails with [its
cpf](https://git-tails.immerda.ch/tails/tree/config/chroot_local-includes/usr/local/lib/tor-controlport-filter),
which is the one Whonix forked from.
Tails and Whonix filter are only different wrt how cpfpy configuration
files are parsed. Not noticeable from application level.
|
meejah:
Additionally: is there any way for txtorcon to reliably tell "I'm
running against a filtering proxy"?
I don't think there currently is, but would probably not be hard to
implement.
However, I guess it's best if at application level you won't have to
distinguish talking to real Tor control port vs tor-controlport-filter.
That would add another level of complexity.
Ideally we could solve all in the filter. By asking you "please don't
ask for all existing GETCONFs even if you do not need them" as well as
"ideally one Tor control protocol command at once (GETINFO version
followed by GETINNFO circuit-established is better than a combined
GETINFO version circuit-established because then the filter rules look
cleaner" is already a lot to ask. And nice of you considering to
implement that!
"ask for all existing GETCONFs" seems most useful for Tor controllers
such as 'arm' / 'nyx'. Not so much for applications with a more limited
scope as in creating, deleting onions and looking up related status's.
//cc @fred-a-kemp (author of tor-controlport-filter by Tails)
|
Sure. The
Oh, good to hear that! Thanks :) |
What if we came up with a command that is not part of the protocol which we would get
What if there was a |
I still think detecting the filter and acting different is the wrong
approach.
Why not go minimal by default, and ask for other stuff when needed? Then
all the filtering could remain abstracted in the filter.
Another idea I had in mind is tricking txtorcon. When it asks
config/names, only reply with a limited amount of names. Do you think
that would work or is a sane approach?
Felipe Dau:
> Additionally: is there any way for txtorcon to reliably tell "I'm
> running against a filtering proxy"? Then e.g. I could trigger the
> "minimal GETCONF" mode automatically and/or provide better
> error-messages when things do go wrong [...] One suggestion: the
> answer to PROTOCOLINFO could include something in the VERSION lines
> without violating the control-spec I think.
What if we came up with a command that is not part of the protocol
which we would get `Unrecognized command` without a filter but `OK`
with a filter and then enable the "minimal" mode?
Sure.
You'd probably also wanted to make sure it also works when using
tor-controlport-filter --complain.
|
That breaks the existing API of This is why I was presenting it as "maybe I could make a different implementation" because current users of
Hmm, very interesting! Yes, that should work (at least as far as: txtorcon will then only ask for those config-items). However, some things will break if certain items aren't in there (e.g. without This is definitely way better than "special case" behavior in txtorcon (i.e. "am I connecting to a filter"). It also seems "more correct" to me as far as the control-spec. |
meejah:
> Why not go minimal by default, and ask for other stuff when needed?
> Then all the filtering could remain abstracted in the filter.
That breaks the existing API of `TorConfig`.
This is why I was presenting it as "maybe I could make a *different*
implementation" because current users of `TorConfig` would all break
if I made the API a deferred-using one. That said, some people would
prefer a "more explicit" API like this.
I see. Yes, if you could implement a new "minimal" `TorConfig` that
would be awesome. Next step would then be contacting the application
developers who use the "full" `TorConfig` and ask them to port to the
new "minimal" `TorConfig` if that is possible in their use case.
(I imagine porting to a new "minimal" `TorConfig` will be possible for
most if not all applications that matter in case of Whonix, i.e.
@unmessage, @globaleaks are not Tor controllers so they need fewer Tor
control protocol commands, therefore could use the "minimal" `TorConfig`.
That seems to me on a coding level like a clean implementation. Thank
you for your interest to implement this!
> Another idea I had in mind is tricking txtorcon. When it asks
> config/names, only reply with a limited amount of names. Do you
> think that would work or is a sane approach?
Hmm, very interesting!
Yes, that *should* work (at least as far as: txtorcon will then only
ask for those config-items).
Great!
However, some things will break if
certain items aren't in there (e.g. without `SOCKSPort` it can't ask
for that, and does do that sometimes).
We could leave `SOCKSPort` inside config/names, no problem. We can even
rewrite the reply that the client is getting for `SOCKSPort`.
Example of doing that here:
https://github.com/Whonix/control-port-filter-python/blob/master/etc/tor-controlport-filter-merger.d/30_whonix.yml
Btw other tor controlport filter examples here:
https://github.com/Whonix/control-port-filter-python/tree/master/usr/share/tor-controlport-filter-merger/examples
I guess you would just make up
this list from any whitelisted `GETCONF` items?
Probably yes. Whitelisting all `GETCONF` items that are really required.
And then manually add them to a list for config/names and add that to
tor controlport filter profile.
(I mean, I wouldn't auto generate the config/names list within the
python code. That level of complexity should not be needed.)
I'll try hacking the
filter to do this and see how it goes. Great idea!
This is definitely way better than "special case" behavior in
txtorcon (i.e. "am I connecting to a filter"). It also seems "more
correct" to me as far as the control-spec.
Great!
|
That's a great idea! Would that be the default behavior of the filter? Do you think people might not like this feature because they would be confused thinking configs are not available, but are actually just "pre-filtered"? Well, that until they learn to deal with cpfs...
And do you think it is worth supporting the minimal/deferred approach? I think it is and agree with @adrelanos that minimal should be used and request on demand, but maybe too much work while the trick presented by him solves the problem (for now?). If the "minimal" feature should proceed, then it could be optional while full would still be the default mode and announce that txtorcon x.y.z will swap them and full would become optional and minimal the default. And as suggested by @adrelanos, contact the people we know are using it. |
The default filtering approach in Whonix will be to enable the minimum for client use only (no hidden services). i.e.: Users that wish to use applications that require other Tor control protocol commands such as onionshare, unMessage etc. will have to follow Whonix documentation and enable the profile as of Whonix 14. For Non-Qubes-Whonix 15 I am thinking about a first boot whonix setup wizard question "allow onions?" (phrasing this is still todo) and then allow all applications we have profiles for (copy them for the user from the examples to the actual config folder) to simplify things. Qubes-Whonix 15 I am thinking about an shell wrappers in front of onionshare, unMessage etc. that on start of onionshare etc. ask "enable onionshare profile in sys-whonix" and therefore simplify that for the user. Btw that's one reason for the stackable wrappers draft.
The behavior when the unMessage tor controlport filter profile got enabled.
For any application that needs more GETCONFS, they'd need to enable a tor controlport filter profile for that application. That should then be a higher priority configuration file (lexically later file name) and overwrite the small config/names list with a longer one. Exotic combinations of random profiles may not be possible without profile modifications. Profiles will always be per application. A one fits all may not be possible without sacrifice of security. Perhaps we could move away from per-application profiles and make that
Or simpler, just two modes:
|
It would be great to have an app for that!
I really like this idea! A great UX improvement.
The behavior I meant was that the filter's reply to
I also like this idea - and customizing it with an application as you suggested above would be awesome. The only difficulty I see would be handling the different ports each app uses. How would that work? Thanks! |
Felipe Dau:
The reply to Here is the pseudo profile part for
No more automatic than that. Then indeed multiple txtorcon based applications requiring different replies for (Such a conflict could of course be manually solved by adding yet another higher priority profile that allows the merged set of Perhaps it would indeed be a nice feature to automatically reply to What do you think about this, @fred-a-kemp? (anonym, author of
In Tails: a lot better than in Whonix. They can use
etc. Something that cannot be used in Whonix due to gateway / workstation split model, where the exe-paths information gets lost. (That's why it's set to In Whonix: where we merge all profiles from onionshare
and zeronet
get merged to
Lucky we these won't conflict. But yeah, if in future loads of applications do all sorts of things, we could end up in a situation where one |
Yeah, this makes sense. IMHO we should only special-case exactly So, let's only special-case exactly |
To me this sounds like a good way as well. Similarly, I think you should always allow "GETINFO info/names" and auto-reply with anything whitelisted under 'GETINFO' For txtorcon specifically, everything is built up from |
fred-a-kemp:
So, let's only special-case **exactly** `GETINFO config/names` (and
always allow it, no matter the filter file). Sorry for my hostility
to GitHub, but you'll have to fetch my fix from the
`feature/getinfo-config-names` branch in the new upstream repo (note
the name change!) found at: https://git-tails.immerda.ch/onion-grater
(web interface is broken, but `git clone` works). Let me know what
you think!
Great!
Looks like you branched off an outdated branch? (missing python -u,
--listen-interface, etc.)
|
Patrick Schleizer:
fred-a-kemp:
> So, let's only special-case **exactly** `GETINFO config/names` (and
> always allow it, no matter the filter file). Sorry for my hostility
> to GitHub, but you'll have to fetch my fix from the
> `feature/getinfo-config-names` branch in the new upstream repo (note
> the name change!) found at: https://git-tails.immerda.ch/onion-grater
> (web interface is broken, but `git clone` works). Let me know what
> you think!
Great!
Looks like you branched off an outdated branch? (missing python -u,
--listen-interface, etc.)
Right. I force pushed the branch rebased on master, so please fetch and adjust!
|
Your patch seems to work, thank you @fred-a-kemp.
However, either txtorcon gets confused by that or unMessage does not use add_onion due to some bug? |
I just tried the above config (using 'carml') after a couple hacks to the filter code, but: ADD_ONION isn't in the yaml above. Adding the below config allows this command to succeed:
|
Hmm, it looks like "something is hanging" when doing I think it's actually simple to fix: control-port lines must end in |
You also need to allow the |
Okay, with those changes (including |
Following @meejah's suggestions allowed me to run unMessage on Whonix. It was possible to send and receive connections. I just pushed a change (felipedau/unmessage@c51b981) that properly maps the hs to the correct (external) IP address. The tests we ran previously worked because we only tested making the connections from the Whonix VM - not receiving! Thanks guys! |
Does Whonix/onion-grater@30c1de5 look alright? If it is not too much to ask, while you are at it, could you please have a brief look if the filter otherwise looks alright wrt \n vs \r\n as well as other things? |
|
@adrelanos yes that change looks good for onion-grater |
p.s. related to this original issue, PR #220 changes this API to "get_config". I'm still a little conflicted over calling this "create_config" or "get_config". Reasons: get_config:
create_config:
Of course, I could make it |
I agree with the name you chose to use and IMO you shouldn't rename it. As a user, I would expect I am not sure if people would need to create new configs, so I would not bother about changing the behavior as well. Can you think of an example that would be used? Maybe Thanks! |
@felipedau Thanks for the feedback :) No, I can't think of any reason someone would need more than 1 |
The code changing the |
Thanks @meejah, with these changes I was also able to use unMessage on both Whonix 14 (Debian 9) and non-Whonix (Debian 8) systems and successfully make and receive connections. |
Great, thanks for testing @felipedau ! |
We should make the API for "get a TorConfig instance" async and call it .create_config, not .config on the
Tor
class.Also,
.connect
and.launch
should not useTorConfig
at all. It is already "not really" exposed in the API (there is a_tor_config
for tests (and.global_tor()
?) but it shouldn't be used and the docs say that).The use-case for this is cutting down on the number of GETCONF calls for "filtering proxies" (such as Whonix and SubGraph use to limit access to the tor control port). This is also the only "implied" object created, so it would be nice to eliminate it, making all additional "helper" object things that are created from
Tor
methods.Thanks to @felipedau for bringing this use-case up. See also: AnemoneLabs/unmessage#4 (comment)
The text was updated successfully, but these errors were encountered: