The Tor Project issueshttps://gitlab.torproject.org/groups/tpo/-/issues2020-06-27T14:04:57Zhttps://gitlab.torproject.org/tpo/core/tor/-/issues/7952Control port method to get the exit policy2020-06-27T14:04:57ZDamian JohnsonControl port method to get the exit policyPresently controllers can GETCONF the ExitPolicy, but not get the actual exit policy we're running with. It would be nice if it was exposed via the control port.
Here's what I ended up writing in stem to get around this...
https://gitw...Presently controllers can GETCONF the ExitPolicy, but not get the actual exit policy we're running with. It would be nice if it was exposed via the control port.
Here's what I ended up writing in stem to get around this...
https://gitweb.torproject.org/stem.git/commitdiff/716f8a6
```
19:23 < armadev> atagar: re f0ae1eaee, where were you seeing exit policies with redundant lines?
tor tries to get rid of redundant lines too. i wonder if it doesn't do it well
enough, or doesn't do it pervasively enough?
19:24 < armadev> atagar: ...perhaps tor doesn't export its exit policy to the control port at all
19:43 < atagar> armadev: I don't think that it does, so I wrote a method for getting it...
19:43 < atagar> https://gitweb.torproject.org/stem.git/commitdiff/716f8a693e9b814da5e2c9df551dbe6768f4f324
19:43 < atagar> It would be nicer to fetch via the control port though. :)
19:43 < armadev> for these cases, it would be great if you opened a tor ticket to say what you want
exported
19:44 < atagar> ok, will do
19:44 < armadev> i guess there should be a balance between how much work tor does, and how much
work the controller does
19:44 < armadev> but in general, if the argument can go "each controller would have to implement
this thing itself, which is best done in tor", then it should go into tor
```Tor: 0.2.5.x-finalhttps://gitlab.torproject.org/tpo/core/tor/-/issues/7956Tor uses Roaming (remote) %APPDATA% instead of %LOCALAPPDATA%2021-07-09T17:21:59ZcypherpunksTor uses Roaming (remote) %APPDATA% instead of %LOCALAPPDATA%[First tor-bug post, hope I got this right...]
The Windows tor.exe -- all current versions, standalone expert bundle and TBB/etc bundles -- has outdated Windows Explorer COM interface code to get the user's %APPDATA% location. This loca...[First tor-bug post, hope I got this right...]
The Windows tor.exe -- all current versions, standalone expert bundle and TBB/etc bundles -- has outdated Windows Explorer COM interface code to get the user's %APPDATA% location. This location can be local or roaming (remote).
Remote data gives many more opportunities for adversaries to mess with Tor Windows users, especially when they are using a domained enterprise.
The solution is below. Until this is fixed, Windows Tor users should never use Tor on a Windows network.
The Windows version of Tor calls the Windows Explorer 'shell' special folder COM APIs, to obtain the location for %APPDATA%. App data can be local or roaming.
On my box, %USERPROFILE%\AppData\* has 3 directories where apps install files, Local, LocalLow, and Roaming, where about 55% of the apps use Local, 5% uses LocalLow, and 40% use Remote.
Tor uses Roaming ("%USERPROFILE%\AppData\Roaming\tor).
Roaming means that it gets remoted to other systems, which is probably a bad thing for Tor user data. The point of Roaming profiles is to let user data migrate to multiple boxes, and the remoting of that data happens with SMBs. So, Remote user data might be sniffable over the network, hopefully this is encrypted.
Depending on how enterprise is configured, Remote user data also
probably means replication/backups of that data in the domain
controller's Active Directory box. So the user's Tor data is available over multiple SMB hosts, making things easier for attackers.
This bug was probably added to Tor in 0.2.1.11-alpha - 2009-01-20:
----snip----
- Add a new --enable-local-appdata configuration switch to change
the default location of the datadir on win32 from APPDATA to
LOCAL_APPDATA. In the future, we should migrate to LOCAL_APPDATA
entirely. Patch from coderman.
----snip----
Tor explicitly checks the location. in or/config.c's get_windows_conf_root(), in it's use of the Windows Explorer COM APIs SHGetSpecialFolderLocation() and SHGetPathFromIDList(), using this definition of the APP_DATA flag:
----snip----
/* Find X:\documents and settings\username\application data\ .
* We would use SHGetSpecialFolder path, but that wasn't added
* until IE4.
*/
#ifdef ENABLE_LOCAL_APPDATA
#define APPDATA_PATH CSIDL_LOCAL_APPDATA
#else
#define APPDATA_PATH CSIDL_APPDATA
#endif
----snip----
First, this comment is wrong on many levels. It begins with "X:" (instead of "C:") which would likely be remote, not local. It talks about IE4 which shipped aeons ago, maybe time to revisit this hesitation. And you you *do* use this COM API, anyway.
I am pretty sure that it is the above definitions that're causing you to install the Roaming data location on my Win7 box.
CSIDL_LOCAL_APPDATA == FOLDERID_LocalAppData
CSIDL_APPDATA == FOLDERID_RoamingAppData
I am not great at Autotools, but I don't believe anyone is setting
ENABLE_LOCAL_APPDATA, so the directive might be 0. It is defined/used in config.ac and orconfig.h.in.
----snip----
/* Defined if we default to host local appdata paths on Windows */
#undef ENABLE_LOCAL_APPDATA
----snip----
I don't understand why this Windows-centric feature needs to be abstracted with GNU/autotools in the first place. Can Erin's Mac OSX-based, MinGW cross-compiled Autotools determine that I'm running WinXP or Win7, etc? This abstraction, using legacy APIs, is probably why this bug has been there since 2009.
Tor already abstracts this Windows-centric code into it's own function, it would be clearer if you removed all the autotools absraction to the Windows Special Folder COM API that Tor uses, IMO.
Also, this function doesn't check this return code, and assumes to be true:
strlcpy(p,get_windows_conf_root(),MAX_PATH);
Also, in this Windows-centric error codepath, there is no warning to the
user, AFAICT:
if (!SUCCEEDED(result)) {
return NULL;
}
Also, elsewhere in get_windows_conf_root(), there are 2 comments, with potentially-open questions. Here are updated comments stated more clearly. Replace:
/* Convert the path from an "ID List" (whatever that is!) to a path. */
with:
/* Windows Explorer, aka 'Windows 'shell', uses IDLists to access various things, including special folders. We need to convert this COM-based explorer-centric array list to a Windows path. */
And replace:
/* Now we need to free the memory that the path-idl was stored in.
* In typical Windows fashion, we can't just call 'free()' on it. */
with:
/* Windows explorer special folders APIs are COM APIs, not Win32 or C runtime library calls. To use special folders, we have to use the COM memory mgmt APIs, to allocate, and now release that memory. Read the SDK ShObj.h or MSDN for more info. */
Note also that you are using legacy, deprecated version of the Special folders API, there have been revisions, apparently during Vista and Win7. The newer API has more features than the one you're using, including the local/roaming granularity. I think f you explicitly stick to local data, I think you can stick with legacy APIs.
You can't just test this with one version of Windows, you'll need to test with each version you are supporting.
More info:
http://msdn.microsoft.com/en-us/library/windows/desktop/bb762181%28v=vs.85%29.aspx
http://msdn.microsoft.com/en-us/library/windows/desktop/dd378457%28v=vs.85%29.aspx
http://msdn.microsoft.com/en-us/library/windows/desktop/bb762494%28v=vs.85%29.aspx
http://technet.microsoft.com/en-us/library/cc766489.aspx
http://msdn.microsoft.com/en-us/library/windows/desktop/bb776779%28v=vs.85%29.aspx
http://www.blogtechnika.com/what-is-application-data-folder-in-windows-7/
http://superuser.com/questions/150012/what-is-the-difference-between-local-and-roaming-folders
http://superuser.com/questions/21458/why-are-there-directories-called-local-locallow-and-roaming-under-users-user
http://social.technet.microsoft.com/Forums/en/w7itprogeneral/thread/b4fd7ac2-8ad9-4a06-8c28-3f7993aa0272
http://blogs.technet.com/b/fdcc/archive/2009/07/08/fdcc-vista-application-development-requirements.aspx
http://social.msdn.microsoft.com/Forums/en/windowsgeneraldevelopmentissues/thread/8fa38285-e4f5-43f9-ab0b-8fe184d476f9
http://technet.microsoft.com/en-us/library/cc778976.aspx
Thanks,
Leehttps://gitlab.torproject.org/tpo/core/tor/-/issues/8002Mis-count of CPUs2020-06-27T14:04:55ZTracMis-count of CPUsOn start-up Tor v0.2.3.25/Linux says:
"[notice] Wow! I detected that you have 64 CPUs. I will not autodetect any more than 16, though. If you want to configure more, set NumCPUs in your torrc"
This is in a CentOS6/OpenVZ VPS with onl...On start-up Tor v0.2.3.25/Linux says:
"[notice] Wow! I detected that you have 64 CPUs. I will not autodetect any more than 16, though. If you want to configure more, set NumCPUs in your torrc"
This is in a CentOS6/OpenVZ VPS with only a single virtual CPU. Tor config param NumCPUs is not used.
It turns out that OpenVZ, since version rhel6/042stab061.2, lists all the *host* CPUs in /sys/devices/system/cpu/. It is this number of processors that is returned by the sysconf(_SC_NPROCESSORS_CONF) that Tor uses in ~/tor-0.2.3.25/src/common/compat.c to determine CPU count.
From the command line:
# getconf _NPROCESSORS_ONLN
1
Programmatically:
$ /tmp/main
sysconf() says: 64
Wow, too many CPUs: 64
I know that there's not much you can do about a sysconf() that lies. I want to get this on the record, though, for the sake of Tor's future auto-scaling.
**Trac**:
**Username**: tmpname0901Tor: 0.2.4.x-finalhttps://gitlab.torproject.org/tpo/core/tor/-/issues/8037Specialy crafter microdesc could trigger to flush up to 16MB uninited heap al...2020-06-27T14:04:54ZcypherpunksSpecialy crafter microdesc could trigger to flush up to 16MB uninited heap allocated memory to mediamicrodescs_parse_from_string() and so on func do not count string as null terminated and allows to process "string" with zero byte in middle. md->body = tor_strndup(cp, md->bodylen) duplicate only partial of such string. dump_microdescri...microdescs_parse_from_string() and so on func do not count string as null terminated and allows to process "string" with zero byte in middle. md->body = tor_strndup(cp, md->bodylen) duplicate only partial of such string. dump_microdescriptor() flushes all bodylen of md's body to disk. Specially crafted bytes append to valid md sent by directory cache could lead to flush up to 16MB of memory data to media. Tor tries to clear sensitive data on free(), but some non cleared memory still no need to write in plain text to media.Tor: 0.2.4.x-finalhttps://gitlab.torproject.org/tpo/core/tor/-/issues/8042Reloaded md never be purged for platforms with unsigned time_t2020-06-27T14:04:54ZcypherpunksReloaded md never be purged for platforms with unsigned time_tmicrodesc_cache_reload() calls microdescs_add_to_cache() with listed_at == -1, so md->last_listed was defined by annotation loaded from disk only.
```
if (listed_at > 0) {
SMARTLIST_FOREACH(descriptors, microdesc_t *, md,
...microdesc_cache_reload() calls microdescs_add_to_cache() with listed_at == -1, so md->last_listed was defined by annotation loaded from disk only.
```
if (listed_at > 0) {
SMARTLIST_FOREACH(descriptors, microdesc_t *, md,
md->last_listed = listed_at);
```
But if unsigned time_t then last_listed updated to 0xFF..FF value so that md never be purged by microdesc_cache_clean(). If such md was flushed to disk during rebuild cache then it will forever live.Tor: 0.2.4.x-finalhttps://gitlab.torproject.org/tpo/core/tor/-/issues/8051Broken check for empty "bridge-ips" line in validate_bridge_stats()2020-07-24T14:16:14ZGeorge KadianakisBroken check for empty "bridge-ips" line in validate_bridge_stats()`geoip_format_bridge_stats()` generates the `bridge-stats` file contents, and if `country_data` is NULL then it writes "bridge-ips \n" to the file.
Then when `validate_bridge_stats()` is called, if it can't find a populated `bridge-ips`...`geoip_format_bridge_stats()` generates the `bridge-stats` file contents, and if `country_data` is NULL then it writes "bridge-ips \n" to the file.
Then when `validate_bridge_stats()` is called, if it can't find a populated `bridge-ips` line it tries to match "bridge-ips\n", which should always fail because of the extra whitespace that was added in `geoip_format_bridge_stats()`. Isn't that right?
The only thing that confuses me is why this hasn't triggered so far. Maybe something in my analysis is incorrect, or the `bridge-stats` file is always generated after we accept a connection?Tor: unspecifiedhttps://gitlab.torproject.org/tpo/core/tor/-/issues/8180EntryNodes ignored when UseEntryGuards==0, warning can be overlooked2020-06-27T14:04:48ZcypherpunksEntryNodes ignored when UseEntryGuards==0, warning can be overlookedThe man page doesn't mention that the EntryNodes option only applies when Tor uses entry guards. I think this is not the expected bahaviour because the option is called "EntryNodes" and not "EntryGuards".
If you don't want to fix this, ...The man page doesn't mention that the EntryNodes option only applies when Tor uses entry guards. I think this is not the expected bahaviour because the option is called "EntryNodes" and not "EntryGuards".
If you don't want to fix this, at least let Tor exit with an error when it reads the torrc file, because if someone provides a list of EntryNodes, that can only mean he doesn't want his computer to connect to other nodes, and it might be dangerous for him if Tor does it anyway.Tor: 0.2.4.x-finalhttps://gitlab.torproject.org/tpo/core/tor/-/issues/8197Do something about policies_parse_exit_policy()'s arguments2021-09-16T14:36:00ZRoger DingledineDo something about policies_parse_exit_policy()'s arguments```
r = policies_parse_exit_policy(&line, &policy, 1, 0, 0, 1);
...
test_assert(0 == policies_parse_exit_policy(NULL, &policy2, 1, 1, 0, 1));
```
Quick quiz: which of these booleans means what? And which one isn't a boolean at all? ...```
r = policies_parse_exit_policy(&line, &policy, 1, 0, 0, 1);
...
test_assert(0 == policies_parse_exit_policy(NULL, &policy2, 1, 1, 0, 1));
```
Quick quiz: which of these booleans means what? And which one isn't a boolean at all? :)
(Don't work on this ticket until we merge legacy/trac#5528.)Tor: 0.2.6.x-finalrl1987rl1987https://gitlab.torproject.org/tpo/core/tor/-/issues/8206Check return values from fcntl and setsockopt2020-06-27T14:04:46ZNick MathewsonCheck return values from fcntl and setsockoptThese functions are allowed to fail, though in practice they won't (at least, not the way we're using them). Still, our failure to check the return values makes some tools (Coverity scan) unhappy. Let's clean up false positives by chec...These functions are allowed to fail, though in practice they won't (at least, not the way we're using them). Still, our failure to check the return values makes some tools (Coverity scan) unhappy. Let's clean up false positives by checking our 12 unchecked fcntls and our one unchecked setsockopt.Tor: 0.2.4.x-finalhttps://gitlab.torproject.org/tpo/applications/tor-browser/-/issues/8227Create descriptions for new Torbutton Security Prefs2020-06-27T14:42:50ZMike PerryCreate descriptions for new Torbutton Security PrefsWe simplified our Torbutton Security Prefs in legacy/trac#3100, but Nick pointed out that we should probably have inline descriptions of each pref and why you would want to set them. I think this is a good idea, but I suspect that it wil...We simplified our Torbutton Security Prefs in legacy/trac#3100, but Nick pointed out that we should probably have inline descriptions of each pref and why you would want to set them. I think this is a good idea, but I suspect that it will take a fair amount of time to both get the wording right and get the layout right, so I decided to push it out until we're more sure we actually have this time to spare before FF17 drops, or in case we simply decide we have to do it afterwords.https://gitlab.torproject.org/tpo/core/tor/-/issues/8278Wrap conditionally-compiled C files in #ifdefs2021-06-18T18:21:28ZNick MathewsonWrap conditionally-compiled C files in #ifdefsFor people editing Tor with an IDE, it can be helpful to have the files which won't get built surrounded with appropriate #ifdef blocks. That way, their IDE won't complain that the file is uncompileable when in fact it's not even suppos...For people editing Tor with an IDE, it can be helpful to have the files which won't get built surrounded with appropriate #ifdef blocks. That way, their IDE won't complain that the file is uncompileable when in fact it's not even supposed to get built.
This is also an issue for people writing their own build scripts/tools for Tor and getting it wrong, but I'm less interested in handling that case.
This ticket is more or less in "Lorax" status. ("Unless someone like you cares a whole awful lot / nothing is going to get better. It's not.") I'll take a clean patch for it in 0.2.5 or later if somebody writes one.https://gitlab.torproject.org/tpo/core/tor/-/issues/8297Do not start reading connection if any blocking reason still present2020-06-27T14:04:42ZcypherpunksDo not start reading connection if any blocking reason still presentTor can to start reading edge connection just because received SENDME cell, or cell queue reduced only or just refilled buckets. While all another reasons of blocking connection still present. No need to start reading edge connection if ...Tor can to start reading edge connection just because received SENDME cell, or cell queue reduced only or just refilled buckets. While all another reasons of blocking connection still present. No need to start reading edge connection if any of blocking reasons still true.Tor: unspecifiedhttps://gitlab.torproject.org/tpo/core/tor/-/issues/8415Use event_set_mem_functions2022-04-29T21:02:58ZNick MathewsonUse event_set_mem_functionsWe should use event_set_mem_functions so that Libevent also uses our memory allocation functions, as we do for openssl when we call CRYPTO_set_mem_ex_functions.We should use event_set_mem_functions so that Libevent also uses our memory allocation functions, as we do for openssl when we call CRYPTO_set_mem_ex_functions.Tor: 0.3.5.x-finalrl1987rl1987https://gitlab.torproject.org/tpo/core/tor/-/issues/8712Authorities should not vote against Fast just because they vote against Running2020-06-27T14:04:30ZRoger DingledineAuthorities should not vote against Fast just because they vote against RunningNon-active relays get stripped of their Fast flag, even if the bwauth measurements put them above the Fast threshold.
Seems to me that if enough other authorities find the relay to be Running, we shouldn't be voting against giving him t...Non-active relays get stripped of their Fast flag, even if the bwauth measurements put them above the Fast threshold.
Seems to me that if enough other authorities find the relay to be Running, we shouldn't be voting against giving him the Fast flag.
Probably same with other flags like Guard, Stable, and Exit.Tor: 0.2.7.x-finalhttps://gitlab.torproject.org/tpo/core/tor/-/issues/8727ServerTransportListenAddr validation should validate that transport-name is w...2022-02-07T19:38:03ZGeorge KadianakisServerTransportListenAddr validation should validate that transport-name is well-formedSomeone put in his torrc:
```
ServerTransportListenAddr obfs2,obfs3 0.0.0.0:56831 0.0.0.0:56832
```
inspired by the format of ServerTransportPlugin. Unfortunately, this is not the correct way to use ServerTransportListenAddr. The correct...Someone put in his torrc:
```
ServerTransportListenAddr obfs2,obfs3 0.0.0.0:56831 0.0.0.0:56832
```
inspired by the format of ServerTransportPlugin. Unfortunately, this is not the correct way to use ServerTransportListenAddr. The correct way is:
```
ServerTransportListenAddr obfs2 0.0.0.0:56831
ServerTransportListenAddr obfs3 0.0.0.0:56832
```
We should at least validate that the first argument of the line is a pluggable transport name (C-identifier) to avoid stuff like "obfs2,obfs3".https://gitlab.torproject.org/tpo/core/tor/-/issues/8749Return information about the leaking application2021-06-18T18:21:28ZbastikReturn information about the leaking applicationLog from where the leaking request came.
When Tor says (in the Vidalia log):
> Potentially Dangerous Connection! - One of your applications established a connection through Tor to "!IP:PORT" using a protocol that may leak information a...Log from where the leaking request came.
When Tor says (in the Vidalia log):
> Potentially Dangerous Connection! - One of your applications established a connection through Tor to "!IP:PORT" using a protocol that may leak information about your destination. Please ensure you configure your applications to use only SOCKS4a or SOCKS5 with remote hostname resolution.
could Tor tell what port the connection was made from? Maybe the log could include SOCKS details (like username). I don't think it isn't able to identify the application.
Sure it's bad to use random stuff with Tor, but this information makes it easier to sort out applications that leak.https://gitlab.torproject.org/tpo/core/tor/-/issues/8787Check return values for more unix functions2020-07-24T14:58:06ZNick MathewsonCheck return values for more unix functionsReportedly, we lack checks for the return values of at least munmap, lseek, unlink. We should fix that for code-quality.Reportedly, we lack checks for the return values of at least munmap, lseek, unlink. We should fix that for code-quality.Tor: unspecifiedhttps://gitlab.torproject.org/tpo/core/tor/-/issues/8791munge_extrainfo_into_routerinfo would mishandle a non-extrainfo2020-06-27T14:04:27ZNick Mathewsonmunge_extrainfo_into_routerinfo would mishandle a non-extrainfoIf you could somehow make munge_extrainfo_into_routerinfo() get a document which wasn't an extrainfo, it would be in trouble, since it assumes that the relevant history lines are NUL-terminated.
For quality-of-implementation, it should ...If you could somehow make munge_extrainfo_into_routerinfo() get a document which wasn't an extrainfo, it would be in trouble, since it assumes that the relevant history lines are NUL-terminated.
For quality-of-implementation, it should check the return value of memchr().Tor: 0.2.5.x-finalhttps://gitlab.torproject.org/tpo/core/tor/-/issues/8960replaycache_add_test_and_elapsed uses int for the length of a cache entry2020-06-27T14:04:22ZRobert Ransomreplaycache_add_test_and_elapsed uses int for the length of a cache entryThe rest of Tor uses `size_t` for buffer lengths.The rest of Tor uses `size_t` for buffer lengths.Tor: 0.2.5.x-finalhttps://gitlab.torproject.org/tpo/core/tor/-/issues/8961src/or/replaycache.c hashes entries with SHA-12020-06-27T14:04:22ZRobert Ransomsrc/or/replaycache.c hashes entries with SHA-1Tor is supposed to be moving away from SHA-1, and the replay-detection cache can be migrated _and_ protected against hash flooding at the same time (see also legacy/trac#4900) without a protocol change. Just add and use a `crypto_digest...Tor is supposed to be moving away from SHA-1, and the replay-detection cache can be migrated _and_ protected against hash flooding at the same time (see also legacy/trac#4900) without a protocol change. Just add and use a `crypto_digest_local` function which prepends a random bytestring (either 16 bytes or a full hash block), then applies either SHA-256 (if Tor was compiled for a 32-bit architecture) or SHA-512 (if Tor was compiled for a 64-bit architecture), then returns the first 160 bits.Tor: 0.2.8.x-final