Openwall GNU/*/Linux - a small security-enhanced Linux distro for servers
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Tue, 23 Jul 2013 04:57:56 +0100
From: Rafael Waldo Delgado Doblas <lord.rafa@...il.com>
To: john-dev@...ts.openwall.com
Subject: Re: cryptocurrency mining fallback in JtR

Alexander,

> Great.  Are there any usage instructions for an advanced end-user, so
> that we could ask someone to try this out?  If not yet, then please
> write some.

I have written a draft here:
http://openwall.info/wiki/john/cgminer_integration

> OK, although it could be cleaner to turn this into an extra Makefile
> (then invoke with "make -f Makefile.miner TARGET=...") or extra targets
> inside the main Makefile (then invoke it with "make TARGET=...") or even
> have cgminer and dtach built by default after a normal build, without
> any unusual syntax on make's command line (this is what we discussed on
> IRC originally).  Do you find the latter hard to implement?

Acturally yes, the first thing that I tried to do was add to the Makefile

../run/cgminer: ../run/john
        cd cgminer; ./configure; make;.....

But the configure scrypt didn't work. Maybe there are a problem with the
make env variables.

> Shouldn't most of these (all but MinerAfterEnd) be combined into one
> option called simply Options (where cgminer's command-line options
> would be specified) and MinerAfterEnd renamed to simply AfterEnd (or we
> may invent a better name) since you already put them into their new
> section called Miner, making the Miner* prefix redundant?

> I understand that with your use of execve() you'd have to parse the
> command-line options into individual argv[] elements, yet I think you
> can do that.

> For ease of use it is sufficient to provide some sample default Options
> string.  It will illustrate proper usage just as well as the multiple
> separate options do.

OK now there 3 options:
APIPort -> Port used by JtR to connect with an existing CGMiner API.
AfterEnd -> Used to tell JtR if must to start a new CGMiner instance or not
(only starts a new one if there are none running)
AfterEndOptions -> Used to set the new CGMiner instance parameters.


> Great.  Where are the corresponding changes to cgminer code (for the API
> enhancement you mentioned)?  I tried looking at every commit you made on
> the bleeding-jumbo branch in https://github.com/LordRafa/JohnTheRipper
> and there doesn't appear to be a commit with just your cgminer changes.
> There must have been a commit like that, separate from pulling
> upstream's cgminer code in.

> We need to review those changes and then likely propose them upstream.
> (For that, we also need to forward-port them to the latest cgminer.)

As I told in my last mail, those changes were made in the repo
https://github.com/LordRafa/cgminer on branch 2.10

> You're resetting the environment.  You should preserve it.  At least the
> XAUTHORITY variable needs to be preserved for access to AMD GPUs to work.

> Why do you set TERM=xterm?  Perhaps you should not.

As we want to keep the environment I'm using execv instead of execve. Now
dtach works perfectly.

> Why "-c /tmp/cgminer"?  In general, explicit use of /tmp is considered a
> security hole, with relatively few exceptions.

I move it to run folder.

> Some other assorted comments:

> Is src/cgminer/.deps (and the files in this directory), which got into
> your git tree, part of cgminer distribution, or did you commit it by
> mistake?

Was a mistake fixed.

> Please add copyright and license statements to your added source files
> (src/miner*).

> You're assuming that cgminer and dtach are in the current directory, but
> what if john is invoked with a path to it specified?  Maybe you should
> call path_expand() on "$JOHN/cgminer" and "$JOHN/dtach", and put these
> strings in params.h along with other similar ones.

> Your source code indentation is consistently inconsistent with that of
> the rest of JtR.  We use 8 char tabs (and 4 chars for half-indentation
> on line-wrapped statements).  You use 3 char indentation.  Can you adopt
> ours?

Done.

> Why all the "#ifdef __linux__"?  I understand that you have not tested
> on other platforms yet, but let's try to keep the code available on all
> platforms anyway - and only deactivate it on platforms where it's known
> not to work.

I made some changes to allow work in windows (I didn't test it) and I
removed this directive.

> Your multiple mentions of 4096 in miner.c is an example of how not to do
> it. ;-)  Use a #define or a global constant instead, and consider
> passing sizeof(buf) as a parameter to get_data().

Improved in the new code.

> Why the /24 netmask on 127.0.0.1?  Why not /32 or /8?  /24 seems
> entirely arbitrary (/32 or /8 would not), unless there's some reason.

I copy-paste from the CGMiner Readme.

> miner.c unnecessarily exports multiple symbols to other source files.
> Please use "static" on all global symbols that are not to be exported.
> There should be very few globals without the "static" keyword on them.

> You're not checking for error returns from many of the calls, e.g. from
> fork().  Please add such error checking, and please use pexit() from
> misc.c on fatal errors and fprintf(stderr, "...\n") for warnings.

Done in the new code.

> You use blocking I/O and no timeouts for communication over the socket.
> This is fine for testing, but eventually it'd be nice to make this more
> robust.  (This is not to be worked on yet.)

To do.

> If you simply omit the option, would there really be a dependency?
> Wouldn't cgminer simply build without ncurses if the ncurses development
> library is not installed?

> As to preference, I think we shouldn't override cgminer default without
> reason.

Done.

Rafael.



2013/7/20 Solar Designer <solar@...nwall.com>

> Rafael,
>
> On Tue, Jul 16, 2013 at 12:05:01AM +0100, Rafael Waldo Delgado Doblas
> wrote:
> > I have pushed the changes to my github repo. Now is working on linux.
>
> Great.  Are there any usage instructions for an advanced end-user, so
> that we could ask someone to try this out?  If not yet, then please
> write some.
>
> > In order to build the project, cgminer and dtach, I made a bash script
> > called build.sh. To compile you need to provide the correct architecture
> as
> > parameter.
>
> OK, although it could be cleaner to turn this into an extra Makefile
> (then invoke with "make -f Makefile.miner TARGET=...") or extra targets
> inside the main Makefile (then invoke it with "make TARGET=...") or even
> have cgminer and dtach built by default after a normal build, without
> any unusual syntax on make's command line (this is what we discussed on
> IRC originally).  Do you find the latter hard to implement?
>
> > This are the new options in the config file:
> >
> > MinerAfterEnd -> launchs a cgminer instance at the end of JtR if there
> are
> > not a cgminer instance running.
> > MinerPoolURL -> pool url.
> > MinerPoolUSR -> pool user.
> > MinerPoolPWD -> pool password.
> > MinerPlatform -> cgminer platform to use.
> > MinerAPIPort -> port to check where the CGminer instance (if there are
> any)
> > is listening.
>
> Shouldn't most of these (all but MinerAfterEnd) be combined into one
> option called simply Options (where cgminer's command-line options
> would be specified) and MinerAfterEnd renamed to simply AfterEnd (or we
> may invent a better name) since you already put them into their new
> section called Miner, making the Miner* prefix redundant?
>
> I understand that with your use of execve() you'd have to parse the
> command-line options into individual argv[] elements, yet I think you
> can do that.
>
> For ease of use it is sufficient to provide some sample default Options
> string.  It will illustrate proper usage just as well as the multiple
> separate options do.
>
> > If any cgminer instance is found JtR will stop the gpus used by JtR and
> > after it finish it will start to mining again.
>
> Great.  Where are the corresponding changes to cgminer code (for the API
> enhancement you mentioned)?  I tried looking at every commit you made on
> the bleeding-jumbo branch in https://github.com/LordRafa/JohnTheRipper
> and there doesn't appear to be a commit with just your cgminer changes.
> There must have been a commit like that, separate from pulling
> upstream's cgminer code in.
>
> We need to review those changes and then likely propose them upstream.
> (For that, we also need to forward-port them to the latest cgminer.)
>
> > I'm having problems with dtach. When I do this:
> >
> >          char *envp[] = {"TERM=xterm", NULL};
> >          char *argv[] = {"./cgminer",
> >             "-o", miner_pool_url,
> >             "-u", miner_pool_usr,
> >             "-p", miner_pool_pwd,
> >             "--api-port", miner_api_port,
> >             "--api-listen", "--api-allow", "W:127.0.0.1/24",
> >             miner_platform,
> >             NULL
> >          };
> >          execve("./cgminer", argv, envp);
> >
> > cgminer works perfectly but when I try to do this:
> >
> >
> >          char *envp[] = {"TERM=xterm", NULL};
> >          char *argv[] = {"./dtach", "-c", "/tmp/cgminer",
> >             "./cgminer",
> >             "-o", miner_pool_url,
> >             "-u", miner_pool_usr,
> >             "-p", miner_pool_pwd,
> >             "--api-port", miner_api_port,
> >             "--api-listen", "--api-allow", "W:127.0.0.1/24",
> >             miner_platform,
> >             NULL
> >          };
> >          execve("./dtach", argv, envp);
> >
> > cgminer says that there no devices to use.
>
> You're resetting the environment.  You should preserve it.  At least the
> XAUTHORITY variable needs to be preserved for access to AMD GPUs to work.
>
> Why do you set TERM=xterm?  Perhaps you should not.
>
> Why "-c /tmp/cgminer"?  In general, explicit use of /tmp is considered a
> security hole, with relatively few exceptions.
>
> Some other assorted comments:
>
> Is src/cgminer/.deps (and the files in this directory), which got into
> your git tree, part of cgminer distribution, or did you commit it by
> mistake?
>
> Please add copyright and license statements to your added source files
> (src/miner*).
>
> You're assuming that cgminer and dtach are in the current directory, but
> what if john is invoked with a path to it specified?  Maybe you should
> call path_expand() on "$JOHN/cgminer" and "$JOHN/dtach", and put these
> strings in params.h along with other similar ones.
>
> Your source code indentation is consistently inconsistent with that of
> the rest of JtR.  We use 8 char tabs (and 4 chars for half-indentation
> on line-wrapped statements).  You use 3 char indentation.  Can you adopt
> ours?
>
> Why all the "#ifdef __linux__"?  I understand that you have not tested
> on other platforms yet, but let's try to keep the code available on all
> platforms anyway - and only deactivate it on platforms where it's known
> not to work.
>
> Your multiple mentions of 4096 in miner.c is an example of how not to do
> it. ;-)  Use a #define or a global constant instead, and consider
> passing sizeof(buf) as a parameter to get_data().
>
> Why the /24 netmask on 127.0.0.1?  Why not /32 or /8?  /24 seems
> entirely arbitrary (/32 or /8 would not), unless there's some reason.
>
> miner.c unnecessarily exports multiple symbols to other source files.
> Please use "static" on all global symbols that are not to be exported.
> There should be very few globals without the "static" keyword on them.
>
> You're not checking for error returns from many of the calls, e.g. from
> fork().  Please add such error checking, and please use pexit() from
> misc.c on fatal errors and fprintf(stderr, "...\n") for warnings.
>
> When you reply to e-mails on this mailing list, please quote inline if
> you don't mind:
>
> http://www.complang.tuwien.ac.at/anton/mail-news-errors.html
> http://www.netmeister.org/news/learn2quote.html
>
> Your recent reply to Lukas did not quote any portion of his message at
> all, even though it was a reply to his specific requests.
>
> You use blocking I/O and no timeouts for communication over the socket.
> This is fine for testing, but eventually it'd be nice to make this more
> robust.  (This is not to be worked on yet.)
>
> Alexander
>

[ CONTENT OF TYPE text/html SKIPPED ]

Powered by blists - more mailing lists

Your e-mail address:

Powered by Openwall GNU/*/Linux - Powered by OpenVZ