Opened 7 years ago

Last modified 5 years ago

#741 accepted enhancement

Make I2P easier to deal with with Windows firewall software

Reported by: killyourtv Owned by: str4d
Priority: minor Milestone:
Component: router/general Version: 0.9.2
Keywords: usability grandma windows firewall Cc: Zlatin Balevsky
Parent Tickets: Sensitive: no

Description

Jun   23 22:54:47 <Bry8Star> Dont want to limit i2p's peers. But since firewall has given 'inbound' (and outbound) permission to JAVA.exe, now any other unwanted connections are also getting initiated.
Jun   23 22:55:09 <zb> Bry8Star, is that what you're talking about? The windows firewall?
Jun   23 22:55:13 <zb> there's a very easy way around that
Jun   23 22:55:24 <Bry8Star> so I dont use the i2p anymore that cannot use a 2nd Java.
Jun   23 22:55:57 <zb> ok, forget everything about I said about SecurityManager? - the way around this is to launch the JVM from within a native code wrapper which will replace the process name to "i2p"
Jun   23 22:56:00 <Bry8Star> I've taken pointer from other portable I2P and using that to force to use a 2nd Java.
Jun   23 22:57:02 <zb> then on your firewall you will allow "i2p", not "java".
Jun   23 22:57:17 <Bry8Star> I2Psvc.exe uses Java.exe or Javaw.exe to do all its operations.
Jun   23 22:57:27 <zb> ok so that's the problem
Jun   23 22:58:16 <zb> the solution still requires coding but it's definitely doable
Jun   23 22:58:34 <zb> and the task manager will show "i2p" not "java", etc.
Jun   23 22:59:49 <Bry8Star> If someone installs the full i2p installer, that uses the system's default java, and since firewall is given permission to allow whatever toward/from I2Psvs.exe… Thats not good. So i2p must use a 2nd
Jun   23 23:00:15 <zb> no, it doesn't need to use 2nd java
Jun   23 23:00:27 <zb> it needs to use jni to launch the jvm from a native executable
Jun   23 23:01:15 <Bry8Star> If task-manager, and wndows treats the entire i2psvc as just 'i2psvc" then firewall rules can be created for that i2psvc only.. And that is a very good solution.
Jun   23 23:01:18 <zb> eche|on, that's windoze firewall he's talking about, they're not that "advanced" :-/
Jun   23 23:01:38 <Bry8Star> Then it can use the default java. As the firewall rules wil be for i2psvc. And not for java.
Jun   23 23:02:36 <Bry8Star> Not windows firewall, any 3rd party firewall. Comodo firewall, OutpostPro? firewall, etc.
Jun   23 23:03:09 <zb> yeah in windows world they like to use per-process permissions
Jun   23 23:03:19 <zb> so it is important to rename the process when using java
Jun   23 23:04:29 <zb> I even have the source code how to do it



Jun   24 00:02:24 <+str4d> I'm still uncertain of the exact problem here. Is the issue that the Windows firewalls allow by processes rather than ports?
Jun   24 00:02:30 <Bry8Star> I see. Then I guess, I have account in github, if you open an free account for it, and add me there, then I can try to maintain, but you wil have to be the main one.
Jun   24 00:02:52 <zb> str4d, windows users have been conditioned to think of per-process security rather than per-port security
Jun   24 00:02:55 ✽ +str4d plugs git.repo.i2p
Jun   24 00:03:15 <+str4d> Ah. That is a rather stupid model IMHO
Jun   24 00:03:27 <zb> majority of third-party firewalls are designed around this model
Jun   24 00:03:30 <zb> yeah but that's reality
Jun   24 00:03:58 <+str4d> But if that is how they think, then I can see how allowing the entire java process free-reign is an issue.
Jun   24 00:04:00 <Bry8Star> 'microsoft windows' uses port based restrictions. But in windows 7 and newer, it can do apps based restrictions as well. Other firewall can do both app & port based rules/restrictions.
Jun   24 00:05:30 <Bry8Star> Since in wondows, i2p uses the java, a separate process, and all connections initiates from/to that computer's default installed java, it can create (and did create) unwanted connections into other apps
Jun   24 00:05:52 <zb> str4d, everything about the windows ecosystem is like that. You deal with the worst design decisions daily to get basic things done.
Jun   24 00:06:39 <+str4d> Bry8Star, I doubt that it creates the unwanted connections. Rather, the firewall allows the connections in?
Jun   24 00:07:22 <+str4d> But even with app-based firewalls, it should still limit what ports are used. But it does sound like a typical Windows thing to do >_<
Jun   24 00:07:40 ✽ +str4d doesn't even want to think about IE…
Jun   24 00:09:14 <+str4d> But yes, if it is simple enough to get i2p on windows to operate as a single "separate" app for firewalling purposes, I'd be happy to help try and get that into trunk.



Jun   24 00:30:42 <zb> Bry8Star, str4d : turns out I had uploaded it as part of an old side project 2 years ago - http://downloads.sourceforge.net/project/muwire/muwire/0.2.0/muwire-src-0.2.0.tar.bz2
Jun   24 00:30:50 <zb> you only care about what is under limewireExe
Jun   24 00:31:27 <zb> visual studio project and the minimal code necessary to rename launch the jvm maintaining parent process image
Jun   24 00:31:47 <zb> enjoy

Subtickets

Attachments (1)

i2pExe.zip (38.2 KB) - added by str4d 7 years ago.

Download all attachments as: .zip

Change History (25)

comment:1 Changed 7 years ago by killyourtv

Summary: Show I2P process in task manager as I2P.exe instead of java.exe (or javaw.exe)Make i2P easier to deal with with Windows firewall software

comment:2 Changed 7 years ago by zzz

<zzz> hmmmmmmmmmmm
<zzz> we already have a 'native executable to launch the jvm'
<zzz> it's called the wrapper
<zzz> perhaps there's some wrapper config options that would do the trick?

comment:3 Changed 7 years ago by Zlatin Balevsky

Cc: Zlatin Balevsky added
Keywords: usability grandma windows firewall added

comment:4 in reply to:  2 Changed 7 years ago by Zlatin Balevsky

Replying to zzz:

<zzz> it's called the wrapper
<zzz> perhaps there's some wrapper config options that would do the trick?

The tanukis wrapper does some nice things. Unfortunately I couldn't see any option to embed the jvm inside the wrapper process. However, it looks like it should be possible to set

wrapper.java.command=our_custom_i2p.exe

on windows and that might just work out of the box. In fact, with this approach the code for our_custom_i2p.exe would be much simpler than what I had in muwire.

comment:5 Changed 7 years ago by zzz

I really don't want to maintain and ship another chunk of native code.

What I am looking for in the wrapper options is simply a way to _name_ java something else, at least to fool Windows. There's some interesting options listed in the advanced section. Also some for Windows service.

comment:6 Changed 7 years ago by Zlatin Balevsky

I couldn't find it in the 10 or so minutes I spent on their site.. if it turns out tanukis doesn't have such option, the custom wrapper is one of those things which are compiled once and never changed. I may be able to take care of it in the near future if it becomes necessary

comment:7 Changed 7 years ago by str4d

From http://wrapper.tanukisoftware.com/doc/english/faq.html#9

I have several Wrapper's and JVMs running, how can I tell which is which in the Windows Task Manager?

When running several copies of the Wrapper under Windows, it is not easy to tell which application is which in the Task Manager because they all show up as wrapper.exe and java.exe. If is not possible for the Wrapper to implement a feature to change this name because Windows does not allow that for security reasons.

The workaround is a bit of a hack. But it works great. Rename the wrapper.exe file to wrapper-myapp.exe. Then modify the batch files so they use this new wrapper-myapp.exe. For the Java process, you have to do the same thing. Go into the %JAVA_HOME%/bin directory and simply copy java.exe to java-myapp.exe. Then modify the wrapper.conf file so that new java-myapp.exe is used.

Now when you look at your Windows Task Manager, it will be easy to tell which exe is which.

Note, you can also set the wrapper.pidfile and wrapper.java.pidfile properties in the wrapper.conf file. These will create files containing the pids. These pids can then be compared with those shown in the Task Manager. Note that you need to configure the Task Manager to display process pids.

So it looks like the custom wrapper is the only feasible option at this time.

Changed 7 years ago by str4d

Attachment: i2pExe.zip added

comment:8 Changed 7 years ago by str4d

Milestone: 0.9.6
Owner: set to str4d
Status: newaccepted
Summary: Make i2P easier to deal with with Windows firewall softwareMake I2P easier to deal with with Windows firewall software

The attached i2pExe.zip contains the Java-renaming wrapper (migrated from zab's limewireExe). It has been built using Visual Studio 2008 and tested on Windows 7 with a clean install of I2P 0.9.5-0, and is functioning perfectly - the only change required was in wrapper.config line 30:
-wrapper.java.command=java
+wrapper.java.command=I2P

Next step: further tweaks so that this wrapper can replace the existing launch4j-based i2p.exe (i.e. I2P.exe could be run on its own to emulate runplain.sh or launched via I2Psvc.exe to use the Tanuki wrapper).

Any comments or suggestions on the above/code, before I commit it? I'm slightly perturbed by the strongly-worded header comments in java.c etc., but I'm guessing those are just old headers? I think those files are only there so the whole JDK is not required to build the wrapper (I didn't have any JDK installed when I built it, just the JRE) - zab? We already use the JDK, so I don't think there should be any license issue.

comment:9 Changed 7 years ago by zzz

review as requested:

my previous comments were skeptical but if you're going to do it that's a lot better… also, a wrapper replacement is probably better than a wrapper-wrapper.

Not against you checking in if you clarify the license info of all you've copied. Also would be better if you check in limewire's stuff as a baseline before modding.

Of course would need testing with all versions of windows we support, 32 and 64 bit, before we start using it. And it would only be for new installs.

Having said all that, I'm not really qualified to review windows C code, but I'll take another look at it late this week when I have more time.

comment:10 in reply to:  9 Changed 7 years ago by str4d

Replying to zzz:

my previous comments were skeptical but if you're going to do it that's a lot better… also, a wrapper replacement is probably better than a wrapper-wrapper.

To clarify, i2pExe is not really a "wrapper" as such; in its current state (in the attached .zip) it is equivalent to the Tanuki workaround in comment 7 of copying java.exe to i2p.exe. It just has the added advantage that we can use it for two purposes (renaming the java process, and replacing the launch4j-based i2p.exe).

Not against you checking in if you clarify the license info of all you've copied.

Of course. The "base" license for limewireExe is GPL (as per http://sourceforge.net/projects/muwire/ ), so in theory at least we can use all of limewireExe without concern, including the java.c etc. files included in the source.

That said, I have done some searching and while I have not yet found the exact files from limewireExe, I have found the java.h and java.c for OpenJDK 7 which are largely identical:

http://hg.openjdk.java.net/jdk7/jdk7//jdk/raw-file/jdk7-b24/src/share/bin/java.h
http://hg.openjdk.java.net/jdk7/jdk7//jdk/raw-file/jdk7-b24/src/share/bin/java.c

These files are GPLv2, so I am reasonably confident that the copies of the files from limewireExe can be used (or failing that, replaced later with a GPLv2 equivalent).

Also would be better if you check in limewire's stuff as a baseline before modding.

Okay. I will commit all the original files from limewireExe first and then apply the mods to make i2pExe.

Of course would need testing with all versions of windows we support, 32 and 64 bit, before we start using it. And it would only be for new installs.

Of course. I assume that all such OS-specific install-related files are not in the updates, as with the Tanuki wrapper? It's simple enough for existing Windows users to gain the benefits of this file though - just download it to their I2P install directory (overwriting the existing i2p.exe), and make the required wrapper.config line change.

Having said all that, I'm not really qualified to review windows C code, but I'll take another look at it late this week when I have more time.

Thanks :)

comment:11 Changed 7 years ago by str4d

i2pExe has been checked into the repo in changeset:bad3a7c1558dcc70f995ed5cb4d31f747a9063ec (look in installer/c/i2pExe).

comment:12 Changed 7 years ago by zzz

re: "wrapper", correct, that was bad terminology on my part, what I meant was it's a launch4j replacement, not a thing layered on top of launch4j. I really only had time for a quick "drive-by" 5-minute review and comment, sorry about that, hope to get my head back into it soon.

In general what gets replaced in the updates are the jars and the theme files, no executables or os-specific things. Take a look at the contents of i2pupdate.zip.

comment:13 Changed 7 years ago by Zlatin Balevsky

zab: I'm gonna ask him for diff
[10:31pm] zzz: pretty sure his first checkin was unmodified from limewire (I asked him to do that), so you can just use mtn
[10:41pm] zab: go over installer/c/i2pExe/I2P.rc there are some potentially questionable choices
[10:42pm] zab: i.e. "anonymity for the masses" in the comments section
[10:43pm] zab: also VALUE "LegalCopyright?", "Public Domain" isn't exactly true
[10:52pm] zab: there's some #ifdef _WIN32 that I don't understand
[10:57pm] zab: also, he commented out -Djava.net.preferIPv4Stack=true and that needs to be passed to the jam somehow
[11:02pm] zab: One thing I don't like is hardcoding all the library jars in the executable. That means rebuilding the launcher every time a library is added or removed.

comment:14 in reply to:  13 Changed 7 years ago by str4d

Replying to zab:

zab: I'm gonna ask him for diff
[10:31pm] zzz: pretty sure his first checkin was unmodified from limewire (I asked him to do that), so you can just use mtn
[10:41pm] zab: go over installer/c/i2pExe/I2P.rc there are some potentially questionable choices
[10:42pm] zab: i.e. "anonymity for the masses" in the comments section
[10:43pm] zab: also VALUE "LegalCopyright?", "Public Domain" isn't exactly true

That's all placeholder stuff - I was just removing all LimeWire-specific references. Of course the actual content would be decided after discussion, but with both zzz and yourself away and few others around, I was more focused on getting something working.

[10:52pm] zab: there's some #ifdef _WIN32 that I don't understand

Those are there in preparation for potentially making this cross-platform (or rather, able to be built for multiple platforms). The #ifdef _WIN32 sections surround Windows-specific code, and don't actually affect the Windows build at all; the goal is that there would be additional #ifdef sections adding in code for other systems for notification boxes etc.

[10:57pm] zab: also, he commented out -Djava.net.preferIPv4Stack=true and that needs to be passed to the jam somehow

I commented it out because the only options I have active in the default settings section are those which are in the existing I2P.exe (see below) (so taken from both installer/i2pstandalone.xml and the doBuildEXE target in build.xml). Also, I recall from IPv6 discussions that this is currently set in Router.java (or somewhere else in-code).

[11:02pm] zab: One thing I don't like is hardcoding all the library jars in the executable. That means rebuilding the launcher every time a library is added or removed.

I agree that hardcoding the library jars isn't nice, but the exact same hardcoding currently occurs in the existing I2P.exe built by launch4j (the library paths are in a .jar but the .jar is then bundled inside I2P.exe, so it also requires rebuilding every time a library is added or removed).

What I am trying to achieve in the defaults section of i2pExe is a replacement for the existing I2P.exe; so when called by the Tanuki wrapper it just uses the provided arguments (so none of the default settings), but when called with no arguments (as is the case for the "No Window" option for starting I2P in Windows), the internal defaults are used.

One way around the hard-coded defaults issue would be to provide a launch.properties; this would be used in preference (I left all that code in from LimeWireExe?). We should still have hard-coded defaults in the event that launch.properties does not exist for some reason, but that way i2pupdate.zip could contain launch.properties so default library paths etc. could be updated. That *would* mean that non-Windows installs would also get a launch.properties, but if the multi-platform idea above is realized then we would want that to be the case.

Another idea is to extend i2pExe to look for launch.properties, and then launch.default.properties if the former does not exist (and then use the hardcoded defaults if neither exist). That way, we can provide "our" defaults, while giving users a file they can tweak which will not be overwritten on updates.

comment:15 Changed 7 years ago by zzz

I don't see how it finds the launch.properties file. Where is it looking? $PWD? $I2P? How does it find a configuration?

Seems like a chicken/egg problem.

You may be trying to bite off too much here. Not only both the wrapper and no-wrapper cases, but also the discussion on IRC about converting existing installs to i2pexe, and serving updates for it. Maybe just tackle the wrapper case, first-install only?

OTOH, skimming thru what you've checked in, the size and complexity of it concerns me. 1685 lines of .c and 2093 lines of .h files. That's a lot to maintain. Would we get it right the first time, or would we have to serve updates for it?

I'm also concerned about testing and the combinatorial problem of all the Windows versions, and 32/64/WOW64, JVMs, etc. The I2P dev team is primarily linux-based and none of us have a decent collection of Windows boxes. We routinely miss Windows-based installation bugs and problems. See the recent 0.9.5 issues #912 #919 #920 for just the most recent batch. Are we getting too far out of our core competency here?

I'm sure it's still a lot simpler than launch4j… but launch4j has "just worked" for us over the years.

So is this the right way to go? Or are we taking on a lot of complexity and risk to solve a minor problem? Just asking. Don't know the answer. But I think a conservative approach is warranted.

comment:16 Changed 7 years ago by Zlatin Balevsky

1685 lines of .c and 2093 lines of .h file

most of that is boilerplate win32 code that is generated automatically and copy/pasta from java.c

Maybe just tackle the wrapper case, first-install only

+1

comment:17 Changed 7 years ago by Zlatin Balevsky

@str4d: get rid of the ifdefs. Even if we decide we need a renamer on other platforms, there are much better and cleaner ways than doing it with ifdefs.

As it stands now, the ifdefs only inflate the size of the diff, which means more code to review and more potential bugs.

comment:18 in reply to:  17 Changed 7 years ago by str4d

Replying to zab:

@str4d: get rid of the ifdefs. Even if we decide we need a renamer on other platforms, there are much better and cleaner ways than doing it with ifdefs.

As it stands now, the ifdefs only inflate the size of the diff, which means more code to review and more potential bugs.

Done. The diff of the code relative to the LimeWireExe? original ( mtn diff -r a8bf31a931109891801d02b7ad32a436d3116954 installer/c/i2pExe/i2p.c ) does look simpler now - just string changes, changes to the default parameters (which need further fixing) and provided parameter handling changes (provided parameters are used instead of the defaults, rather than appended to them like LimeWireExe?).

comment:19 in reply to:  15 Changed 7 years ago by str4d

Replying to zzz:

I don't see how it finds the launch.properties file. Where is it looking? $PWD? $I2P? How does it find a configuration?

Same way it did for LimeWireExe. zab can confirm, but my understanding is that it looks in the same directory as the i2pExe binary, which would be $I2P. What I *haven't* worked out yet is its format, so I have not tested it.

Seems like a chicken/egg problem.

Not that I can see; if no command-line parameters are provided (i.e. the Tanuki wrapper is not launching it), and the launch.properties file is missing, it will fall back to the internal defaults.

You may be trying to bite off too much here. Not only both the wrapper and no-wrapper cases, but also the discussion on IRC about converting existing installs to i2pexe, and serving updates for it. Maybe just tackle the wrapper case, first-install only?

I think I've confused things by talking, as you are misunderstanding me. I have no intention about converting existing installs to i2pExe, or pushing updates to the i2pExe binary via i2pupdate.zip; only new installs would get it by default.

However, if we do launch i2pExe as a replacement for the existing I2P.exe, then existing installs can very easily upgrade manually to it (similar to upgrading their Tanuki wrapper):

  1. Replace I2P.exe in $I2P with i2p.exe downloaded from us.
  2. Edit wrapper.config line 30:

-wrapper.java.command=java
+wrapper.java.command=i2p

Done.

OTOH, skimming thru what you've checked in, the size and complexity of it concerns me. 1685 lines of .c and 2093 lines of .h files. That's a lot to maintain. Would we get it right the first time, or would we have to serve updates for it?

See comment 16. The only important file is i2p.c; everything else is either maintained/generated by VisualStudio or Java, or copied (java.[c|h] etc).

On that note, however: where did you copy those files from zab? Are there updated versions we should be using?

I'm also concerned about testing and the combinatorial problem of all the Windows versions, and 32/64/WOW64, JVMs, etc. The I2P dev team is primarily linux-based and none of us have a decent collection of Windows boxes. We routinely miss Windows-based installation bugs and problems. See the recent 0.9.5 issues #912 #919 #920 for just the most recent batch. Are we getting too far out of our core competency here?

The majority of i2pExe is pre-existing and unchanged from LimeWireExe, so I'm sure zab can elaborate on its version compatibility. But I agree that we shouldn't rush this out before adequate testing has been done.

I'm sure it's still a lot simpler than launch4j… but launch4j has "just worked" for us over the years.

Once the current problem with the default parameters is fixed (likely a mistake by me in determining what those defaults are in launch4j), then if it works with the Tanuki wrapper it should work on its own. However, I do understand the testing problem (and the earlier biting problem), so how about the following compromise:

Rename the launch4j I2P.exe to I2P-standalone.exe (and adjust the "No Wrapper" shortcut accordingly), and bundle i2p.exe for use with the Tanuki wrapper. If we have no reported problems with the "Restartable" shortcut then we just inform Windows users (via News) to delete I2P-standalone.exe and adjust the "No Wrapper" shortcut to point to i2p.exe if they want the standalone process to be renamed (and we can do the same in a subsequent release). If there *are* problems, then users can still use the "No Wrapper" shortcut as before, and we can fix things.

So is this the right way to go? Or are we taking on a lot of complexity and risk to solve a minor problem? Just asking. Don't know the answer. But I think a conservative approach is warranted.

I wouldn't call it a "minor" problem, really - as long as Windows firewall software uses a by-process approach, we should have a way to distinguish ourselves from other Java software on Windows computers.

comment:20 Changed 7 years ago by Zlatin Balevsky

There is an #ifdef ALPHA which turns on some debugging options and loads the YourKit agent. I don't think i2p needs such functionality so please remove the ALPHA ifdef. (you removed it from one place, but it's used in two places)

Regarding compatibility, this same code was run on 70 million windows boxes. It definitely works on XP/Vista/7; it has not been tested on windows 8.

Regarding severity, this is not just a security issue. It will help us when we debug on windows.

comment:21 Changed 7 years ago by Zlatin Balevsky

I lost my commit key, so here is the removal of the ALPHA ifdef

http://pastethis.i2p/show/3105/

comment:22 in reply to:  21 Changed 7 years ago by str4d

Replying to zab:

I lost my commit key, so here is the removal of the ALPHA ifdef

http://pastethis.i2p/show/3105/

Missed that, thanks. [3cff53ae6e9e995199ab4c7c4ce5fa78ec417768] removes that as well as the (commented-out) code to copy the provided args after the default/launch.properties ones (which is unnecessary for i2pExe).

comment:23 Changed 6 years ago by str4d

Milestone: 0.9.60.9.11

Aiming to restart this soon.

comment:24 Changed 5 years ago by str4d

Milestone: 0.9.11

Heh, that went well. I will get to this eventually, but I need to set aside some time to get my I2P Windows toolchain back in order.

Found this sitting in my notes store:
<sponge> str4d: re: 741 I have an idea that will work a whole lot better for you.
<sponge> str4d: if file == NULL, feed your parser a text blob. This will do the right thing then in all cases.

Note: See TracTickets for help on using tickets.