in reply to RFC: SecureString - Obfuscated / masked strings exept when you need them

Conceptually, I love it. Codewise, I've not looked :-)

Basically, I have code like this at the top of every function:

my $self = shift; $self->trace_entry(@_);
(trace_entry jsonishly serialises everything to the trace file.) Without this, I have to pull the password out of the tracing, and do other funky things just to ensure that the password doesn't show up. In fact, I've gone and reversed the whole thing by pushing the password off into another module altogether, and doing non-standard tracing there, just so that passwords don't end up in the trace. The downside is that the password object is global/a singleton. And, while doing this might be doable, it's a bit more painful when I pass around a hash of options through multiple layers between the part that starts with the password, and underlying code that might need it.

Something like this would be extremely useful, I think. But putting it in the top-level of namespaces seems a bit off :-) Also, calling it "Secure" might be misleading as well. Perhaps "Text::Hidden"?

Looking a bit more at the code, I see you've gone a little more over the top than I would have thought. Just reading the intro, what I had originally thought was "return the X's for stringification, and the real value for ->get". Because, honestly, this would likely solve nearly the whole problem anyway. Code that just treats it as a string (such as logging) would get the X's, but code that needed the value would already know it's a password (hidden object) and use the ->get. Restricting which pieces of code that actually realise it's a password object and call it correctly from being able to get the value is more of a straight-jacket than I'm used to in Perl. (But about right for Java or C++.) Basically, it's just saying "if you know what you're doing, you get it, but if you're just blindly blasting things around, you're not going to get it." The override ability where modules you may not control get access to it is nice, but, again, you then have to worry about them logging it, which now they can do automatically again.

  • Comment on Re: RFC: SecureString - Obfuscated / masked strings exept when you need them
  • Download Code

Replies are listed 'Best First'.
Re^2: RFC: SecureString - Obfuscated / masked strings exept when you need them
by duelafn (Parson) on Jul 20, 2011 at 01:34 UTC

    Thanks for your comments, concept is most of my concern.

    I do tend to agree with you about the "allow" parameter, it fell in by accident while I was exploring some possibilities. I will take your comment as at least partial support for considering removal of that (mis)feature.

    The "auto_get" option is a different kettle of fish though. That can allow you to pass a "Text::Hidden" object to a module that needs to actually use the data at some point (but may attempt to log its value at other points - I can restrict auto_get to a specific method or even a specific line of code). Using "auto_get" does break open the black box of the external module and may break things if the module reorganizes itself too much. On the one hand, it makes me a bit nervous to use such a feature, but on the other hand, I don't see how "Text::Hidden" could be useful if it did not allow passing usable instances into external code.

    Update: On an additional reading, I do see that you had distinguished "allow" from "auto_get", but I still want to point out that access can be restricted more finely than by class name and that the only alternative to such an "auto_get" feature is passing the raw unmasked string to the external module. So, I guess my question for you (and others) is whether you would use the "auto_get" option at the method (or line number) level?

    Good Day,

      I agree with Tanktalus' comments. I wanted to read the code but it is quite hard to find the code with the POD all mixed up with it. Please consider making the code easier to read and the POD easier to read (and both easier to maintain) by keeping them separate.

      I would rename the get() method to something very distinctive. It would be useful to be able to search for all instances of, for example, unhide_string() to audit the places where the sensitive data is being exposed to ensure they are all appropriate.

      I can see the motivation for auto_get() but I also don't think it will be enough of a solution.

      I'd probably go for a more direct approach at preventing specific leakage. The first thing is to prevent the sensitive string from being logged.

      So provide a class that you can tie the log file handle to such that any uses of the file handle set a global "unsecure" flag:

      sub Text::Hidden::Handle::PRINT { local( $Text::Hidden::SECURE ) = 0; ... } sub Text::Hidden::as_string { my( $self ) = @_; return $self->{value} if $SECURE; return $self->get_obscured_value(); }

      Another place you don't want to leak such information is into a database. I'd be tempted to walk caller() information looking for DBI or DBD::* modules, though I somewhat worry about the efficiency of that. Perhaps one only need walk a couple of levels up for such a check, though.

      So, there will be some situations where you can unhide the sensitive string only a couple of places where it is actually used. There will be other situations where the value is used in in a bunch of code, some of which you have no control over and you just want to identify the few places where information can escape the process and block those exits. auto_get() may be sufficient for many of these second cases but I also think it will be harder to get working that way.

      And that last point means that you should provide a debug option that logs the places where the string value was asked for and whether a hidden or unhidden version was provided.

      - tye        

        Pod moved. I can agree that a grepable accessor name is a good idea.

        auto_get does already support code references for the purpose of walking the stack looking for specific things. I imagine a ::Util package with some convenience functions for building such callbacks would be possible - along with some tweaks to the implementation to make subclassing and/or applying roles easier (to permit Text::Hidden::HideFrom::DBI)

        The major take-home that I am getting from the tied filehandle and DBD/DBI examples is that I should not completely abandon the "Default Allow" camp... I think I could get the "unsecure" filehandles, ::HideFrom::* roles, and auto_get lists to work together in a secure and predictable fashion. Am I properly interpreting your suggestions?

        Note: The debug option is currently spelled "cluck => 1" but does currently lack a note specifying whether the string was or was not unmasked. I agree that "debug" is probably a better name.

        Thank you for your comments... I guess I will have to clean this up and upload it now.

        Good Day,

        Revised the synopsis in the original post (well... now it is entering into tutorial territory)

        I don't see how a tied filehandle with a default unhidden policy is going to be a good idea at all. Consider:

        open my $LOG, ">", "/var/log/my_app.log"; tie $LOG, "Text::Hidden::Handle", force => "hidden"; my $ccn = Text::Hidden->new( "1234567887654321", default => "unhidden" + ); print $LOG $ccn; # OK print $LOG "Got CCN: $ccn"; # Oops! - premature stringification

        Sure, interpolating a default unhidden string is always going to be "dangerous", but the fragility near a filehandle that pretends to force the values to hidden seems too far over the top. Unless I misunderstood your suggestions.

        I have however, added default unhidden and hide_from options as well as manual mask forcing. Additionally, I have added localized policy setting so that one need not globally choose default unhidden (see examples in OP). Do you think that the revised synopsis addresses your concerns / describes a potentially usable tool?

        Good Day,