search

granoff / Lockbox

Objective-C utility class for storing data securely in the key chain.
MIT License
847 stars 87 forks source link

Remove "key" property from console log. #23

Closed 0xjmp closed 10 years ago

0xjmp commented 10 years ago

@discussion just to be safe, it's not necessary to log the "key" property's value as anyone who profiles your app could potentially get the value by watching the console

rivera-ernesto commented 10 years ago

Well they could get the key but not the value. Anyway I think it makes sense.

Maybe better, all log messages could be surrounded by:

#ifdef DEBUG
    NSLog(...);
#endif
granoff commented 10 years ago

You've really touched on a larger issue in the class, which is that there ought to be no non-DEBUG logging. That is to say, in any production code, NSLog ought to be a no-op. Cleaner than what @rivera-ernesto suggested would be to do something like the well known DLog() macros:

#ifdef DEBUG
#define DLog(fmt, ...) NSLog((@"%s [Line %d] " fmt), __PRETTY_FUNCTION__, __LINE__, ##__VA_ARGS__);
#else
#define DLog(...)

and then replace occurences of NSLog with DLog. I'd entertain a pull request like this. :-)

rivera-ernesto commented 10 years ago

You're right!

0xjmp commented 10 years ago

I figured why show the key if it's not necessary? Figuring out what key is returning this error is as easy as setting a breakpoint on the log :)

This approach prevents any dev from accidentally giving away their keys, subsequently making Lockbox even more secure. I'd consider it as a best practice then because you shouldn't ever assume that the dev will know to hide their logs in production.

That DLog function is slick @granoff - I'm definitely using it in the future +1

granoff commented 10 years ago

PR 24 took care of this, so closing. Thanks though!