RE: [Passwordsafe-devel] SecString class

[James C. <Ja...@No...>]

>>  Say you have a set< smart_ptr<std::string> >, you could wrap a
smart_ptr around a SecString and place it in the container too.  Only
now the smart_ptr will call the std::string destructor on the SecString.
<<

	Actually, no you can't do that, since smart_ptr<std::string> &
smart_ptr<SecString> are distinct types regardless of how they are
defined.  They cannot be placed in the same set<> container.


>> There are plenty of reasons besides polymorphic behavior to use
pointers: reducing object copying, improving locality of reference,
adding another level of indirection.  So no, the use of a string* does
not automatically imply that we are dealing with subclasses of string.
<<
	
	But, it's not the use of a pointer that's a problem.  It's the
delete via the pointer to the base class, by one agent which knowingly
accepted a pointer new'd by another agent. It's hard to justify that
particular behavior for any reason other than polymorphic use.   I would
not use an OS, compiler, or  library which did such a thing unless the
receiving pointer class had a virtual dtor.  

>>  Almost all functions handle their data very carelessly (from a
security standpoint): making copies, passing it around, outputting the
contents to logs or consoles. <<

	OK, we'll give you that one......

>> there's no reason functions which handle std::string's can't be
compatible with a SecString instantiated from basic_string. <<

	Well, actually there is.... "[U]nless we're writing a compiler,
an operating system, and all the libraries from scratch, there will be
code which we don't control. " 


	On the whole, I'm thinking you're probably right (I meant to say
that in the first message), but I thought a good philosophical
discussion could get this mail list moving again.	


    Truth, 
     James Curran


-----Original Message-----
From: Edward Elliott [mailto:pas...@ed...] 
Sent: Wednesday, May 29, 2002 3:41 PM
To: James Curran; PasswordSafe Development
Subject: Re: [Passwordsafe-devel] SecString class




I agree that with sufficient attentiveness you can avoid doing these
things.  But why require the extra effort when you can just as easily
ensure it can *never* happen by not subclassing std::string?  


> 	Further, we must have the ability (and desire) to change
> GetANewString() (to return a subclass of std::string), but (and here's
> the key), we must be UNABLE to change Func().  However, considering
> that
> code, could there be any possible reason for writing that idiom,
> *unless* it was to use pStr polymorphically?  In which case, the
> programmer, *expects* a subclass of string there, and so the code is
> broken before you touched a thing.  
>
> In other words, we'd be making design decisions based on the 
> theoretically possibility of breaking code that's already broken.

There are plenty of reasons besides polymorphic behavior to use 
pointers: reducing object copying, improving locality of reference,
adding another level of indirection.  So no, the use of a string* does
not automatically imply that we are dealing with subclasses of string.

Additionally, unless we're writing a compiler, an operating system, and
all the libraries from scratch, there will be code which we don't
control.  If your Func above resides in one of those places, we wouldn't
be able to change its code.  There could well be a Func that takes a
std::string* as an argument.

However, even if you're above argument were true, I don't think it's
good justification for subclassing std::string.  Programmers forget
things.  Programmers make changes without fully understanding the code.
Programmers make mistakes.  I do these things frequently, and I consider
myself an above-average programmer.

By not subclassing std::string, we prevent anyone from ever committing
one of these mistakes.  Many more programmers besides you and I will be
working on the project code in its lifetime.  The reliability and
maintainability of the code should be primary concerns.


> 	The problem with the alternative you suggest of create a new 
> instantiation of basic_string with a specific allocator, to that you 
> lose the IS-A relationship of inheritance.  For example, an existing 
> function which took a std::string& parameter would work perfectly with

> a string-derived class, but not with a new basic_string type, since it
> would have not relationship with std::string.

Exactly, that's the point.  A SecString is *not* a std::string.  It
contains data which the program must control extremely carefullly. You
don't want a SecString being passed to any function that handles
std::strings.  Almost all functions handle their data very carelessly
(from a security standpoint): making copies, passing it around, 
outputting the contents to logs or consoles.  You don't want any of
these things happening to a SecString.

Compatibility shouldn't be an issue anyway.  There should be very few
places which need access to SecString's data.  Look at PasswordSafe. Say
a SecString holds a password.  How many places need to set the string
data?  Two - one when the user first inputs the password, another when
decrypting it in from the password storage file.  How many places need
to read the data?  Two - one for displaying the password on screen,
another to encrypt the password for storage on disk.  I have a hard time
thinking of any library functions you would want to pass a password to
that only accept std::strings.

As an aside, there's no reason functions which handle std::string's
can't be compatible with a SecString instantiated from basic_string. If
instead of this
    void foo(std::string& s)
you write this
	template <class _Char, class _Alloc>
    void foo(basic_string<_Char, _Alloc>& s)
then foo will accept any instantiations of basic_string, including
std::string.

Now forget eveything I've just said.  Suppose there are no problems with
the lack of a virtual destructor.  I can still think of at least two
crucial reasons not to subclass std::string.

The first problem is slicing.  If SecString is a subclass of
std::string, and you pass SecString *by value* to a function which
accepts a std::string, that function will get a copy of SecString's data
in a std::string object.  Now all the protections built into SecString
go right out the window, because the function isn't using a SecString
object anymore.

Similarly, you have problems of assignment between string classes. If I
have string s and SecString t, and SecString subclasses string, then I
can assign t to s: s = t.  Again we have the same problem as
before: SecString's data residing in a std::string object.  The data has
no more protection than if we used std::string in the first place.

All in all, I think an allocator is the safest way to go.  That still
lets us use all the STL algorithms on SecString, which should be more
than sufficient for PasswordSafe's needs.

Regards,
Edward Elliott