Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Wed, 13 May 2015 10:18:15 +0300
From: Shinnok <admin@...nnok.com>
To: john-dev@...ts.openwall.com
Subject: Re: [core john] [Johnny] Windows event loop


> On May 12, 2015, at 9:25 PM, Mathieu Laprise <mathlaprise@...il.com> wrote:
> 
> Wow, my tests on CTR+C were done on magum/master, congratulation for finding the 1-2 lines at the right position in john core that made all the differences. I'm really surprised, I've seen nowhere that we had to remove the null handler before adding a new handler. Congrats on finding that !

Here's the tricky part, I didn't remove the handler, a full blown exercise of attention comes next. :) 

Pasting the documentation bit again:
"• Calling SetConsoleCtrlHandler with the NULL and TRUE arguments causes the calling process to ignore CTRL+C signals. This attribute is inherited by child processes, but it can be enabled or disabled by any process without affecting existing processes."
https://msdn.microsoft.com/en-us/library/windows/desktop/ms686016%28v=vs.85%29.aspx <https://msdn.microsoft.com/en-us/library/windows/desktop/ms686016(v=vs.85).aspx>

Read the piece a couple of times. You'll realize than, that this SetConsoleCtrlHandler function is a "super" function, like many others in WinApi. What it can do with just 2 args is:
1. Install a handler: SetConsoleCtrlHandler(HandlerRoutine, TRUE)
2. Remove a handler: SetConsoleCtrlHandler(HandlerRoutine, FALSE)
3. Ignore CTRL_C event processing: SetConsoleCtrlHandler(NULL, TRUE)
4. Stop ignoring CTRL_C event processing: SetConsoleCtrlHandler(NULL, FALSE)

Yes this is counterintuitive, cheap and really bad API design, that's WinApi. What I do in JtR patch is just make sure that we don't ignore the CTRL_C events, keeping in mind that the attribute is inheritable and will be set for Johnny so it doesn't kill itself.

> 
> Shinnok said :
> As you probably have figured out by now, this took me a considerable amount of time to do, so Mathieu, next time around, please strive a bit more when I tell you that you haven't been exhaustive enough regarding an approach.
> I'm sorry you invested so much time redoing it. Since there were no objections when I asked, I sincerely thought you'd be happy with the working solution CTRL_BREAK even if it wasn't done your way. Unfortunately, you were not so I apology. I'm used to work in environments where time is money and if we have something 100% working and OK code-wise, we are not encouraged to spend a lot of time rewriting something that already took a lot of time to do unless it has important benefits. But now, I understand more how it works in this company.

I think we do value quality over anything else for most cases. We also value asking for help when stuck or undecided, instead of hanging on the edge of procrastination for fear of doing so. I do appreciate the preliminary effort and research you put in though.

> 
> I expect you to take an hour or two to figure out if all of this is confirmed on your system too, then I'd like you to proceed to the comments task. My system was a Windows 7 VM. Cygwin latest as of today with gcc version 4.9.2.
> On windows 8.1,  1.7.9, core john and jumbo executables work. But I have the unknown type u_int error for jumbo source code. 
> Aleksey said :
> Though my question was about WM_CLOSE exactly. Is WM_CLOSE still
> needed?
> No, WM_CLOSE was a complete alternative to generating CTRL_XXX events. 
> 

WM_CLOSE was only the second best choice after direct CTRL_C events.

Shinnok
Content of type "text/html" skipped

Powered by blists - more mailing lists

Confused about mailing lists and their use? Read about mailing lists on Wikipedia and check out these guidelines on proper formatting of your messages.