passwordsafe-devel — For developers of Password Safe

RE: [Passwordsafe-devel] SecureAllocator

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

	Yikes! That's a lot more complete than mine!

	I wrote & tested it with VC6 at work, and then tested it at home
with VC7.  VC6 testing was functional, but the Vc7 testing was just to
see if it compiled.  After sending out the email I discovered that the
VC7 version didn't actually work.  (vc6 wasn't using rebind, so it
didn't care that it was missing).  I had just tracked down the error
when your email came.

    Truth, 
     James Curran


-----Original Message-----
From: pas...@li...
[mailto:pas...@li...] On Behalf Of
Edward Elliott
Sent: Friday, June 21, 2002 12:39 AM
To: 'PasswordSafe Development'
Subject: Re: [Passwordsafe-devel] SecureAllocator


Nice job, James.

First, so this information doesn't get buried in the post, here is the 
link to my SecureAllocator code:

http://www.eddeye.net/src/secalloc.html

Re: [Passwordsafe-devel] SecureAllocator

[Edward E. <pas...@ed...>]

Nice job, James.

First, so this information doesn't get buried in the post, here is the 
link to my SecureAllocator code:

http://www.eddeye.net/src/secalloc.html

It addresses all of the issues below.  I have also written a detailed
article on the design and usage of my allocator, which is currently
being considered for publication in the C/C++ User's Journal.  If
anyone is interested, I can see about getting you an advance copy of
the article.  Meanwhile, the code is well commented and the readme
file covers its usage.

Allocators have many subtle issues.  I'd just like to make a few 
observations on James's code:

1) Why subclass std::allocator?  Reimplementing all of
std::allocator's functions can be done in a few lines, and avoids any
problems with subclassing.  Notably, the C++ Standard says if two
allocators compare equal, they may be used to deallocate each other's
memory.  When std::allocator's operator= is called with your
SecureAllocator as an argument, this will happen and you lose all the
protections.  Easy enough to fix.

2) Since you don't reimplement the rebind nested template each
allocator must provide, SecureAllocator inherits std::allocators. for
Whenever a container uses rebind to obtain an allocator of a type
other than the container value type (T), it will obtain a
std::allocator and not a SecureAllocator.  This occurs with every
container other than vectors and strings; lists, maps, and sets all
store values in their own tree or node structure type.  Again, easy to
fix.

3) Clearing memory before releasing it is not the only issue.  In
fact, I seem to recall reading somewhere that some Windows OSes zero
out all memory pages that are returned to the system (though I could
be wrong about this).  In any case, what worries me more is paging out
of keys and passwords to the swap file, where they can remain on disk
much longer than the lifetime of the program.  Some more work to add,
but doable.

4) Each STL has its own quirks with regards to allocator usage.  For
example, the STL which ships with MSVC6 calls an (undocumented) method
in an allocator, _Charalloc, to allocate memory for types other than T
(the container value type).  MSVC doesn't use the rebind template as
prescribed by the C++ Standard because they haven't implement nested
templates yet.  Easy to fix.

5) Likewise, the popular STLPort implementation, which I use with
MSVC, uses some external templated functions to perform rebinding on
compilers that don't support nested templates (a much cleaner solution
than Microsoft's hack).  If you plan to use the allocator with
STLPort, these will have to be added.

I might add that in addition to solving these issues, my allocator
- is well-tested, having been used in several projects for many months
    with success.
- is cross-platform, having been tested with MSVC and gcc on 32-bit
    Windows, and gcc on linux. it should port easily to any other unix.
- uses policy classes for easy extensibility.
- is thread safe.

I leave it to others to decide what use, if any, to make of my code.


Edward Elliott

BTW, I apologize for sending several copies of my last post.  I was
having trouble with my mail server and thought the first two
submissions failed.  Nevertheless, I am embarassed by the slipup.

Re: [Passwordsafe-devel] SecureAllocator

[Jim R. <jru...@us...>]

> Ok, this mail list has been quiet for a while... 

How fortuitous.  I had a slight break in the action of my moonlighting 
work, and thought tonight would be a good night to take a look at the 
PasswordSafe code.  Then your code shows up.

> B) a subtle tweak that I was all talk, no code... 

I just figured you were busy with the theatre site (it took me a while to 
connect the dots -- I'm the one who sent you the cast list for "The 
Hobbit" last month).

Thanks, James, I'll read through it tonight!

Jim R
Itinerant coder and actor

[Passwordsafe-devel] SecureAllocator

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

Ok, this mail list has been quiet for a while... Let's see, when we last
left off, there was:
A) a plan to create a STL allocator, which handled trashMemory as memory
was freed, and
B) a subtle tweak that I was all talk, no code... 

SO, here goes... I tried a crack at the SecureAllocator.  This is what I
came up with:

//---------------------------------------------
#include <memory>
template <typename T>
class SecureAllocator : public std::allocator<T>
{
//	typedef T* pointer;
//	typedef unsigned int size_type;
public:
void deallocate(pointer p, size_type n)
	{
		trashMemory((unsigned char*)p, n * sizeof(T));
   	#if defined(_MSC_VER) && _MSC_VER <= 1200
		operator delete(p);
   	#else
		std::allocator<T>::deallocate(p,n);
   	#endif

	}
};
#include <string>
typedef std::basic_string<char, std::char_traits<char>,
SecureAllocator<char> >   SecString;
typedef std::basic_string<wchar_t, std::char_traits<wchar_t>,
SecureAllocator<wchar_t> >   SecWString;
//---------------------------------------------
That's it.  The #if is needed because VC6 didn't like the
fully-qualified call to the base class. VC7 accepted it, as did Comeau
C++, although Comeau needed the commented-out typedef's

It also appears to work for any other STL collection which takes an
allocator:

#include <vector>
std::vector<int, SecureAllocator<int> > secvector;

Note that this works:

	SecString	secstr = "Abcedfg";
	cout << secstr << endl;

But this does not:

   	string regstr = "Hijkl";
	regstr += secstr;
	string w = secstr + regstr;




Seeing that that was so easy, I tried a crack at making it fully
generic:
As a version which compiled on both VC6 & vc7 required way to many #ifs,
I've include separate versions below, and the combined version at the
very end.
//---------------------------------------------
// Standard C++ version

template <typename T>
inline void trashMemory2(T* buffer, long length)
{		trashMemory((unsigned char*) buffer, length); }


template <typename T, typename Pr>
class beforeDeallocate : public std::allocator<T>
{
public:
//	typedef T* pointer;
//	typedef unsigned int size_type;
void deallocate(pointer p, size_type n)
	{
		Pr	pred;
		pred(p, n * sizeof(T));
		std::allocator<T>::deallocate(p,n);
	}
};
#include <functional>
template <class T, class U, class R, R (*F)(T,U)>
struct some_binary_function: public std::binary_function<T,U,R> 
{
	R operator()(T p, U n)
	{	return F(p, n); }
};
template <typename E>
class SecureAllocator : public beforeDeallocate<E,
some_binary_function<E*, long, VOID, trashMemory2> >
{
};

//---------------------------------------------
/// VC6 version 
template <typename T>
inline int trashMemory2(T* buffer, long length)
{		trashMemory((unsigned char*) buffer, length); 
		return 0;
}


template <typename T, typename Pr>
class beforeDeallocate : public std::allocator<T>
{
public:
//	typedef T* pointer;
//	typedef unsigned int size_type;
void deallocate(pointer p, size_type n)
	{
		Pr	pred;
		pred(p, n * sizeof(T));
		operator delete(p);
	}
};
#include <functional>
template <class T, class U, class R, R (*F)(T,U)>
struct some_binary_function: public std::binary_function<T,U,R> 
{
	R operator()(T p, U n)
	{	return F(p, n); }
};
template <typename E>
class SecureAllocator : public beforeDeallocate<E,
some_binary_function<E*, long, int, trashMemory2> >
{
};
//---------------------------------------------

Most of the differences are because VC6 can't handle a return void.
Also, trashMemory2 is needed because some_binary_function requires a
function that takes exactly two parameter (with an optional third).


    Truth, 
     James Curran



#if defined(_MSC_VER) && _MSC_VER <= 1200
	#define VOID	int
#else
	#define VOID	void
#endif

template <typename T>
inline VOID trashMemory2(T* buffer, long length)
{		trashMemory((unsigned char*) buffer, length); 
#if defined(_MSC_VER) && _MSC_VER <= 1200
		return 0;
#endif
}


template <typename T, typename Pr>
class beforeDeallocate : public std::allocator<T>
{
public:
//	typedef T* pointer;
//	typedef unsigned int size_type;
void deallocate(pointer p, size_type n)
	{
		Pr	pred;
		pred(p, n * sizeof(T));
#if defined(_MSC_VER) && _MSC_VER <= 1200
		operator delete(p);
#else
		std::allocator<T>::deallocate(p,n);
#endif

	}
};
#include <functional>
template <class T, class U, class R, R (*F)(T,U)>
struct some_binary_function: public std::binary_function<T,U,R> 
{
	R operator()(T p, U n)
	{	return F(p, n); }
};
template <typename E>
class SecureAllocator : public beforeDeallocate<E,
some_binary_function<E*, long, VOID, trashMemory2> >
{
};

FW: [Passwordsafe-devel] Design Issues - Env class & Storage class

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

>> You volunteering for coding these classes, James?  <<

	Actually, I'd planed to.  I was bringing them up here to get a
consensus on their design.  (Right now, I'm playing with the Blowfish
class, but on that, I can make changes to the class without effecting
the API, so I can work "behind the scenes")

    Truth, 
     James Curran


-----Original Message-----
From: pas...@li...
[mailto:pas...@li...] On Behalf Of Jim
Russell
Sent: Thursday, May 30, 2002 12:44 AM
To: PasswordSafe Development
Subject: Re: [Passwordsafe-devel] Design Issues - Env class & Storage
class

[Passwordsafe-devel] Re: Passwordsafe-devel digest, Vol 1 #7 - 5 msgs

[Steve L. <st...@is...>]

> > 1. (installer)
> > I agree in principle about it being OpenSource & portable --
> > except by nature, installers are OS specific.  And the current method of
> > a "not install beyond unzipping" is the best, and we should try our
> > hardest to preserve this.
>
> Truth be told, I agree with this.  That came from Bruce, who's looking
> for Password Safe to be mainstream enough for normal people (not like
> us) to use.  I dunno -- self-extracting zip file, anyone?


nullsoft, makers of the illustrious winamp, give out their installer as
source for anyone to use:

http://www.nullsoft.com/free/nsis/


-steve

RE: [Passwordsafe-devel] TODO list comments

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

>> There is no way in hell I'm changing that.  The word "undone" with
the 
meaning "not done" was around long before Ctrl-Z.  <<

	Well, not that long...  From the Oxford English Dictionary:
Undone: not done, unaccomplished, uneffected (earliest citation c1300)
Undone: brought to decay or ruin (earliest citation 1340)

    Truth, 
     James Curran

Re: [Passwordsafe-devel] Design Issues - Env class & Storage class

[Jim R. <ji...@ru...>]

> code, it looks a lot like it was designed by a C programmer who learned
> C++ by running VC++'s ClassWizard.

Oh, there's no doubt about that.  You should have seen it before I did
enough clean-up to make it readable.  On the other hand, it does, for 
the most part, work.  Although some of the encryption code seems to be 
too complex for the task at hand (all that 'stuff' stuff and 1000-times 
loops).

> First, I suggest we create a global singleton call Env or Config, which
> manages all user options.  Every choice should be exposed as a property,


> 	The next class we should define I'll simply call "Storage".
> It's will be completely in charge of reading & writing the PasswordDb.

You volunteering for coding these classes, James?

Jim R

Re: [Passwordsafe-devel] TODO list comments

[Jim R. <ji...@ru...>]

> 1. (installer)
> 	I agree in principle about it being OpenSource & portable --
> except by nature, installers are OS specific.  And the current method of
> a "not install beyond unzipping" is the best, and we should try our
> hardest to preserve this.

Truth be told, I agree with this.  That came from Bruce, who's looking 
for Password Safe to be mainstream enough for normal people (not like 
us) to use.  I dunno -- self-extracting zip file, anyone?

> 4. (.DAT)
> 	In addition to merely allowing extensions besides .dat, we
> should move to using a unique extension for our standard.  I suggest
> ".pwsafe"

Yeah, I know.  Whose idea was .DAT as the default extension?

> 14. (resize & safe position)
> 	Note that it's far easier to make a dialog box resize, than it
> would be to convert Pwsafe into a non-dialog based application.  

I don't know why I didn't think of that.  Perfect.

I'm not going to comment on each of your notes, other than to say 
you've got some good comments that I will attempt to merge into 
the current TODO.

> Porting:
> 	I'd particularly like to see a PocketPC version.  I'd be willing
> to work on that port.

Seems to me that the first order of business is to separate the 
'engine' from the 'frontend', and then use the 'engine' for 
ports.  This way the current MFC code can be isolated (far away
from decent code <ahem>).

> General TODO comments:
> 	In the status column, "undone" should be "not done".  "Undone"
> implies that you implemented it, realized a problem, and backed the
> change out.   

There is no way in hell I'm changing that.  The word "undone" with the 
meaning "not done" was around long before Ctrl-Z.  Have you never heard 
that some things are best left undone?  What about 'unappreciated'?  
Does that mean 'was appreciated, but then the appreciation was removed'?
Or how about 'unequal'?  'Undying'?

> 	Also, the status of #13 (XP) should be listed as "Closed" or
> "not a bug", since nothing was actually "Done".

Ah, well there you have me, but I'll probably not change that either.


Jim R

Re: [Passwordsafe-devel] SecString class

[Jim R. <ji...@ru...>]

> --- James Curran <Ja...@No...> wrote:
> > 	I've just join in, and figure that I'd throw in my two cent on
> > the recent conversations:

Welcome, James.  Nice to have you.

> --- Edward Elliott
> That's what we're here for :).  Thanks for taking time to contribute.
> I haven't heard from Jim in months, I don't think he's actively
> working on the project anymore.  Not that I've done anything either...

Now, Edward, I'm still here.  Swamped as hell, but I'm still here.  
I've just started a new full-time job, while still trying to finish 
off the contracting job I had to pay the bills until I *got* the 
full-time job, and I still managed to get the v1.90 beta posted on 
sourceforge, didn't I?

As I mentioned the *first* time the SecString discussion came up, 
don't forget that that particular scrap of code is *not* part of 
the current build of Password Safe.  It was just me making notes 
to myself as to what we might do to get rid of CMyString, which we 
want to get rid of because it's a subclass of CString, and we're 
never gonna port this thing to other platforms unless we wean it 
away from MFC.

> > this, you just have to look at the one (inconceivable) scenario,
> > where it is a problem:
> 
> It appears such a scenario is conceivable, given that you conceived of
> one :).

"I do not think that word means what you think that word means."  [Inego Montoya]


> 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.

When Edward and I last discussed this, James, I was leaning in the 
same direction (the allocator).  At that time, he promised me that 
he would write one.  <g>

Jim R
The Phantom Maintainer

[Passwordsafe-devel] Design Issues - Env class & Storage class

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

OK, How 'bout we discuss some real design issues.  Scanning the source
code, it looks a lot like it was designed by a C programmer who learned
C++ by running VC++'s ClassWizard.  Nearly every class is derived from
CDialog.  The two main one not derived from CDialog (BlowFish &
CItemData) have serious problems (next email).

First, I suggest we create a global singleton call Env or Config, which
manages all user options.  Every choice should be exposed as a property,
and it will be completely responsible for the persistence of them.  That
way we can isolate the means of persistence (which presumably is
platform dependant: Registry, INI file, a file in the /etc directory or
whatever) from the rest of the code.

Presently, we have code like this:

CConfirmDeleteDlg::CConfirmDeleteDlg(CWnd* pParent)
   : CDialog(CConfirmDeleteDlg::IDD, pParent)
{
   m_dontaskquestion =
      app.GetProfileInt("", "deletequestion", FALSE);
}

// :
// :

      CConfirmDeleteDlg deleteDlg(this);
      if (deleteDlg.m_dontaskquestion == FALSE)
      {
         int rc = deleteDlg.DoModal();
         if (rc == IDOK)

After this change, we'll have:

CConfirmDeleteDlg::CConfirmDeleteDlg(CWnd* pParent)
   : CDialog(CConfirmDeleteDlg::IDD, pParent)
	{  }

// :
// :

	if (Env.confirm_deletion())
	{
	      CConfirmDeleteDlg deleteDlg(this);
            int rc = deleteDlg.DoModal();
      	if (rc == IDOK)		


Note also, that the sense of the flag has been reverse to the positive.
The current way, TRUE indicates a negative, so here, FALSE means "Yes,
do it".

---------------------------------

	The next class we should define I'll simply call "Storage".
It's will be completely in charge of reading & writing the PasswordDb.
Basically, it takes a filename, and returns a vector<CItmeData> (It also
takes the vector and writes it out).  Again this is to let us separate
the platform specific areas for the generic data (For the PocketPC, I've
been thinking about using the systems internal database instead of a
flat file).


    Truth, 
     James Curran

Re: [Passwordsafe-devel] SecString class

[Edward E. <pas...@ed...>]

--- James Curran <Ja...@No...> wrote:
>   I've just join in, and figure that I'd throw in my two cent on
> the recent conversations:

That's what we're here for :).  Thanks for taking time to contribute.
I haven't heard from Jim in months, I don't think he's actively
working on the project anymore.  Not that I've done anything either...

There are more reasons not to subclass std::string than just (lack of)
virtual destructors, but first let me respond to a few of your
comments.


>   Actually, there is NO problem here, because there is no
> conceivable situation where the dtor would not be called.  To prove
> this, you just have to look at the one (inconceivable) scenario,
> where it is a problem:

It appears such a scenario is conceivable, given that you conceived of
one :).  Seriously, I admit it's not gonna be a common occurence, but
the fact is it *can* happen.  Secure software has to consider every
possibility, not just the likely scenarios.


> void Func()
> {
>   std::string* pStr = GetANewString();
>   // use pStr
>
>   delete pStr;   // ~std::string() called, not subclass dtor
> }

That is one way it could happen.  Also consider smart pointers.
It's common to use smart pointers for holding objects in STL
containers to reduce copying overhead.  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.

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

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

Re: [Passwordsafe-devel] SecString class

[Edward E. <pas...@ed...>]

--- James Curran <Ja...@No...> wrote:
> 	I've just join in, and figure that I'd throw in my two cent on
> the recent conversations:

That's what we're here for :).  Thanks for taking time to contribute.
I haven't heard from Jim in months, I don't think he's actively
working on the project anymore.  Not that I've done anything either...

There are more reasons not to subclass std::string than just (lack of)
virtual destructors, but first let me respond to a few of your
comments.


> 	Actually, there is NO problem here, because there is no
> conceivable situation where the dtor would not be called.  To prove
> this, you just have to look at the one (inconceivable) scenario,
> where it is a problem:

It appears such a scenario is conceivable, given that you conceived of
one :).  Seriously, I admit it's not gonna be a common occurence, but
the fact is it *can* happen.  Secure software has to consider every
possibility, not just the likely scenarios.


> void Func()
> {
> 	std::string* pStr = GetANewString();
> 	// use pStr
> 
> 	delete pStr;   // ~std::string() called, not subclass dtor
> }

That is one way it could happen.  Also consider smart pointers.
It's common to use smart pointers for holding objects in STL 
containers to reduce copying overhead.  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.

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

Re: [Passwordsafe-devel] SecString class

[Edward E. <pas...@ed...>]

--- James Curran <Ja...@No...> wrote:
> 	I've just join in, and figure that I'd throw in my two cent on
> the recent conversations:

That's what we're here for :).  Thanks for taking time to contribute.
I haven't heard from Jim in months, I don't think he's actively
working on the project anymore.  Not that I've done anything either...

There are more reasons not to subclass std::string than just (lack of)
virtual destructors, but first let me respond to a few of your
comments.


> 	Actually, there is NO problem here, because there is no
> conceivable situation where the dtor would not be called.  To prove
> this, you just have to look at the one (inconceivable) scenario,
> where it is a problem:

It appears such a scenario is conceivable, given that you conceived of
one :).  Seriously, I admit it's not gonna be a common occurence, but
the fact is it *can* happen.  Secure software has to consider every
possibility, not just the likely scenarios.


> void Func()
> {
> 	std::string* pStr = GetANewString();
> 	// use pStr
> 
> 	delete pStr;   // ~std::string() called, not subclass dtor
> }

That is one way it could happen.  Also consider smart pointers.
It's common to use smart pointers for holding objects in STL 
containers to reduce copying overhead.  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.

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

Re: [Passwordsafe-devel] SecString class

[Edward E. <pas...@ed...>]

--- James Curran <Ja...@No...> wrote:
> 	I've just join in, and figure that I'd throw in my two cent on
> the recent conversations:

That's what we're here for :).  Thanks for taking time to contribute.
I haven't heard from Jim in months, I don't think he's actively
working on the project anymore.  Not that I've done anything either...

There are more reasons not to subclass std::string than just (lack of)
virtual destructors, but first let me respond to a few of your
comments.


> 	Actually, there is NO problem here, because there is no
> conceivable situation where the dtor would not be called.  To prove
> this, you just have to look at the one (inconceivable) scenario,
> where it is a problem:

It appears such a scenario is conceivable, given that you conceived of
one :).  Seriously, I admit it's not gonna be a common occurence, but
the fact is it *can* happen.  Secure software has to consider every
possibility, not just the likely scenarios.


> void Func()
> {
> 	std::string* pStr = GetANewString();
> 	// use pStr
> 
> 	delete pStr;   // ~std::string() called, not subclass dtor
> }

That is one way it could happen.  Also consider smart pointers.
It's common to use smart pointers for holding objects in STL 
containers to reduce copying overhead.  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.

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

[Passwordsafe-devel] Import / Export

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

I'd like to suggest adding Import/Export in encrypted form, particular
of subsets of a database, using a different password. 

Here's the scenario I envision: 

	A company agrees to use PasswordSafe as the corporate standard.
All employees use it for all their passwords (personal & work). The
Network administrator decides one day to change all the passwords for
the common systems. He enters them into his personal PasswordSafe file.
He then selects just the relevant entries, clicks File/Export, and
enters a "previously agreed upon distribution password"  and filename. 

	He can then email that new file to all the affected employees.
They start their own personal copies of PasswordSafe (with their own
password), then click File/Import, entry the password to merge the file,
and the deletes the email--- (The export password needs to be a closely
guarded secret) New passwords widely distributed in a secure manner. 


    Truth, 
     James Curran

[Passwordsafe-devel] Synchronization

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

This was posted, in a slightly different form, on the SourceForce
PasswordSafe Forum, in response to another message.

Presently I "synchronize" it (using the term loosely) between my home
and work PCs, by copying the .DAT file via my PocketPC (using
ActiveSync). This uses merely the single file date for synchronization.
This is a just barely workable solution. (If I add one entry at work,
then later a different one at home between syncs, the home file ends up
overwriting the work file, losing that update.

So, I agree that we need a modified date field for each item (TODO #27),
but that's not far enough. 

What I'd really like to be able to do is to create a ActiveSync module
for PasswordSafe, which can handle syncing on an entry level, not just
on the file level (And I assume a similar facility is possible for Palm
and other systems that can be synced). An important part is that the
syncing tool should be able to work *without* knowing the file password 

To accomplish that, we'd need each entry to have both a last modified
date, and a (permanent) GUID-like unique identifier. These must be in
clear text, and each entry (encrypted & clear text portions) must be a
fixed size. This, along with a "last synced" date, would allow a full
automatic two-way sync for adds, deletes, and modify (detecting items
modified independently on both side between sync).  

More importantly, each entry would have to be individually encoded
(instead of have the entire file encrypted at once, which I assume is
the way it's done now). This lead to very short runs of encrypted text,
which I think makes it easier to crack (though I'm not sure on this
point), which could be a problem. 

Actually, the entries needed completely fixed length ---the cleartext
portion (which would actually be fairly opaque binary data) needs to be
fixed long, and include a offset and length of the variable length
encrypted entry.  Further, now that I've looked at the code, it seems
that each entry IS being encode individually.



The alternative would be have the syncing tool decrypt the file before
it goes to work (and then recrypt it later). That would require the user
either manually entering his password whenever he synced (A major pain
-- I sync my PocketPc at least twice a day, once at home & once at the
office), -OR- the sync tool would need to store the password locally,
which would be a security leak by itself. 

    Truth, 
     James Curran

[Passwordsafe-devel] TODO list comments

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

1. (installer)
	I agree in principle about it being OpenSource & portable --
except by nature, installers are OS specific.  And the current method of
a "not install beyond unzipping" is the best, and we should try our
hardest to preserve this.

4. (.DAT)
	In addition to merely allowing extensions besides .dat, we
should move to using a unique extension for our standard.  I suggest
".pwsafe"

14. (resize & safe position)
	Note that it's far easier to make a dialog box resize, than it
would be to convert Pwsafe into a non-dialog based application.  

19. (copy with Ctrl-C)
	Yes, it should be automatic with Windows, but it's not, and it's
annoying as hell! I believe the problem is that the edit dialog had the
DLGC_WANTCHARS flags set somewhere. (It tells Windows that that the
control itself will handle all special characters)

22 & 23. (export as text, print)
	In addition to the honkin' big warning, it should require
reentry of the file password.  If a walk up to someone's unattended desk
& find PWS open, I shouldn't be able to print out the whole file (force
me to copy'n'paste each entry manually).  Further, neither the CSV file
nor the printout should have any column headers or any indication that
it came from PWS.  It a find a stray printout lying around, it shouldn't
scream "These are someone's passwords".

24. (prevent second user from modifying).
	It can be done internally.  We'd have to open the file with the
DENY_WRITES sharing option, and keep the file open as long as the
program is running.  Additionally, we'd have to detect a failure to open
to file in read/write mode, and switch to read only (probably after
offering the user of (a) opening this read-only, (b) opening another
file read/write or (c) close this instance of the program entirely, and
activating the other instance (for people, like me, who accidentally
start PWS a second time when it's already running)

26. (configure random passwords generation)
	Additionally, it like an option for "first character always
alphabetic"  (no particular reason -- it just seems "right" that way)

31. (start PWS on Windows startup)
	I'm not sure what they want here.  A tutorial on how to put a
shortcut in the Startup folder?  Further, it begs the question, in this
case, should it popup to the "Entry Password" window on Windows startup
(which will get very annoying very quickly), or do they want an
auto-sign in feature (which is a whole different and much bigger issue)

32. (hotkey)
	Like 31, this is more a personal Windows configuration issue,
rather than something internal to the program.  (It's set on the
properties of the shortcut in the Start Menu)

37. (always on top)
	No! Please no!  "Always on top" is an abomination before the
Lord!  Ok, Ok... I guess I could deal with it, if (a) it's is an OPTION,
(b) it is OFF by default, (c) the option is sticky (so when I turn it
off, it STAYS off.).  Also, always-on-top is of little usefulness for a
program which takes up as much screen real estate as PWS, so we should
add the windows resize feature first.


Porting:
	I'd particularly like to see a PocketPC version.  I'd be willing
to work on that port.

General TODO comments:
	In the status column, "undone" should be "not done".  "Undone"
implies that you implemented it, realized a problem, and backed the
change out.   
	Also, the status of #13 (XP) should be listed as "Closed" or
"not a bug", since nothing was actually "Done".

(If you want to use any of these comments in an updated, TODO list, feel
free....)


    Truth, 
     James Curran

[Passwordsafe-devel] Hello all

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

	Now that I've already posted my first comment to the mail list,
I might as well introduce myself.   I've been waiting for the OpenSource
project to start for about 2 years now.  Apparently you started it right
after the last time I check CounterPane's web page (when I noticed they
had removed the notice the v2.0 would be open source).

	I have a few recommendations & suggestions which I'll post in
separate messages (yeah, you're about to get a flood from me).


    Truth, 
     James Curran

[Passwordsafe-devel] SecString class

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

	I've just join in, and figure that I'd throw in my two cent on
the recent conversations:

>> I noticed the SecString class at that time was subclassing
std::string, which is a big no-no since std::string has no virtual
destructor.  In some instances, this can lead to the SecString
destructor never being called, and hence the memory buffer with the
SecString contents never getting cleared. <<

	Actually, there is NO problem here, because there is no
conceivable situation where the dtor would not be called.  To prove
this, you just have to look at the one (inconceivable) scenario, where
it is a problem:

void Func()
{
	std::string* pStr = GetANewString();
	// use pStr

	delete pStr;   // ~std::string() called, not subclass dtor
}

	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.

Now, if we are able to change Func() (and, since we have the complete
source code at our disposal, we would be able to, if such a function
existed), then fixing it is trivial:

class vstring : public std::string
{	
	vstring(const char* _Ptr, size_type _Count = npos) 
		: std::string(_Ptr, _Count) {}

	// similarly duplicator other ctors

	virtual ~vstring() = 0;
};

class SecString : public vstring
{   // etc
};

void Func()
{
	vstring* pStr = GetANewString();
	// use pStr

	delete pStr;   // subclass dtor called
}


--------------

	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.



    Truth, 
     James Curran

[Passwordsafe-devel] secure memory question

[Edward E. <ed...@ya...>]

Jim,

I haven't seen any list traffic in quite a while.  I take it this means
development on PasswordSafe has stalled or at least tailed off.  That's
ok, I'm not trying impugn anyone, just saying it's part of the reason I
never posted my code for the secure allocator I mentioned a few months
ago.

I've been working on getting my secure allocator published, which will
not only make the code available to everyone but also the allocator
will (hopefully) end up being more robust.  While working on this task,
I ran into a question I'm hoping you can help me with.

In our brief discussion of the SecString class and secure allocators, I
seem to recall you mentioning that a secure memory wipe should do more
than just zero out the memory block once. (I can't find the email now,
the sourceforge archives don't seem to have it). E.g. if I have a key
or password in some memory block, I should overwrite it multiple times
with different bytes (0x00, 0xFF, 0xAA, 0x55, and such).

I am curious, what was the basis for this statement?  After reading
Peter Gutmann's excellent article "Secure deletion of data from
magnetic and solid-state memory”, that strategy seems to work well for
magnetic media (hard disks), but apparently doesn't do much for
solid-state memory (RAM) where the "imprint" left by data is a function
of how much time the data was there.  Did I miss something?  Is there
more recent work which indicates several quick overwrites of RAM really
is beneficial?

Thanks for your help,

Edward Elliott

__________________________________________________
Do You Yahoo!?
Yahoo! Health - your guide to health and wellness
http://health.yahoo.com

[Passwordsafe-devel] Re: secure allocator (addendum)

[Edward E. <ed...@ya...>]

Just to clear up a mistake I made, point 3 in my last email should
have said _dynamic_ initialization not static initialization.  Also
the three variables it applies to are g_lockDH, g_DHBlock, and
g_dhinitializer, which are classes with constructors.  The global
variable g_dwPageSize, a primitive type, is guaranteed to be set to 0
during static initialization before any program code executes.  But
since g_dwPageSize only gets loaded with a useful value in the
constructor of g_dhinitializer, it is indirectly affected by this
problem as well.  To recap, point 3 should read as below:

 3) Thread synchronization is provided by a global lock variable 
    (g_lockDH).  Initialization of this lock therefore occurs at an
    unspecified time during the program's dynamic initialization.
    If a method called elsewhere during dynamic initialization
    attempts to use CDHAllocator, the lock may be uninitialized and
    the results undefined (but almost certainly bad).  This may
    occur for instance in the constructor of a 
    vector<char, CDHAllocator<char> > declared at global scope in
    another translation unit.  The same applies to the global
    variables g_dhinitializer and g_DHBlock, and indirectly to 
    g_dwPageSize.

Sorry for the confusion.

Edward Elliott

__________________________________________________
Do You Yahoo!?
Yahoo! Sports - live college hoops coverage
http://sports.yahoo.com/

[Passwordsafe-devel] secure allocator

[Edward E. <ed...@ya...>]

As promised, I have performed a more thorough review of the "secure" 
CDHAllocator in the BSL library at http://www.bitvise.com/bsl.html.
I'm somewhat embarassed by my earlier assessment that it looked 
"pretty good", even if it was only based on a five-minute perusal.
After only a couple hours studying the code, I've found a half-dozen 
serious flaws and can't recommend its use to anyone.

Here is a summary of the problems I encountered:

 1) CDHAllocator subclasses std::allocator.  I see two potential
    problems with this.  One, std::allocator provides no virtual 
    destructor.  I can't think of a situation where this would cause 
    problems; I haven't seen a situation where allocators are used 
    through pointers or passed by value such that slicing can occur.
    Allocators are not supposed to have non-static member variables 
    anyway (so that two allocators of the same type are always 
    equal), so destruction may be irrelevant.  However no good can
    come of it.

 2) (subclassing, continued) CDHAllocator inherits the operator== 
    from std::allocator.  Comparing a CDHAllocator and a
    std::allocator will therefore be possible (which it shouldn't), 
    since a CDHAllocator is in fact a "const std::allocator&".
    Moreover, this comparison will return true, indicating one type
    of allocator can be used to destroy the other's memory.  I don't 
    know when this would happen but if it did the results would be
    disastrous.  Just the possibility makes me very uncomfortable.

 3) Thread synchronization is provided by a global lock variable 
    (g_lockDH).  Initialization of this lock therefore occurs at an 
    unspecifed time during the program's static initialization.  If a 
    method called elsewhere during static initialization attempts to 
    use CDHAllocator, the lock may be uninitialized and the results 
    undefined (but almost certainly bad).  This may occur for instance
    in the constructor of a vector<char, CDHAllocator<char> > declared 
    at global scope in another translation unit.  The same applies to
    the global variables g_dwPageSize and g_DHBlock.

 4) The global lock provides Lock() and Unlock() methods to acquire
    and release the lock.  If an exception is thrown between these
    two calls, the lock will never be released.  DieHardAlloc, 
    DieHardFree, and DieHardVerifyClean are all subject to this 
    problem. What's worse, CDHBlock explicitly throws exceptions in
    the Allocate and Free methods which DieHardAlloc and DieHardFree 
    call.  For a reliable allocator, especially a secure one where 
    an adversary may be actively trying to cause exceptions, 
    disregarding exception safety and deadlocks is not an option.  

 5) It uses a map and a multimap to keep track of free blocks, a 
    structure far too inefficient for fundamental operations like 
    memory allocation.  Every memory allocation requires at least a
    binary search and two erases, usually accompanied by two inserts
    into the maps as well.  Maps guarantee logarithmic element access
    times but have large constant factors and do not perform well on
    small data sets.  

 6) On memory allocation failure, CDHAllocator throws its own 
    CDHAllocationFailed exception instead of the accepted 
    std::bad_alloc.  This applies to the CAllocator class as well.

 7) On deallocation, it only overwrites each byte a single time with
    0x00.  A safer approach would be to overwrite memory several times
    with different values, including 0x00 and 0xff.

 8) Uses std::fill_n to zero out the memory on deallocation.  Because
    DieHardFree passes unsigned char *'s as iterators, std::fill_n
    loops over the memory one byte at a time.  This is horribly 
    inefficient.  memset (for zeroing out) and memcpy (for overwriting
    with a string of random bytes) are much better.

There may well be other problems with CDHAllocator; I stopped short
of examining CDHBlock's Allocate and Free methods in detail.  I 
decided at this point nothing was salvageable and I needed to write
my own secure allocator from scratch.  I should have it done within
a few days, at which time I'll pass it along.

Edward Elliott

__________________________________________________
Do You Yahoo!?
Yahoo! Sports - live college hoops coverage
http://sports.yahoo.com/

Re: [Passwordsafe-devel] SecString class

[Jim R. <jru...@us...>]

> A few weeks ago, ignorant of this list, I emailed Jim Russell about a
> problem with the PWSafe source.  I noticed the SecString class at that
> time was subclassing std::string, which is a big no-no since
> std::string
> has no virtual destructor.  In some instances, this can lead to the
> SecString destructor never being called, and hence the memory buffer 
> with the SecString contents never getting cleared.

I should point out to the list what I mentioned to Edward when he 
wrote me.  First, SecString is experimental code that is *not* currently 
being used by the Win32 Password Safe executable.  It is basically 
"notes to myself", and may or may not compile as is.  We are looking 
for ways to free PWSafe from the CString class of MFC.  Second, the 
'string' class that I was inheriting from in my experiments is my 
own 'stl-like' string class, whose destructor is virtual.

Okay, disclaimers out out the way, Edward makes an excellent point. 
Any "secure string" class that gets used here should do its security 
magic with its own allocator.  Edward, thanks for following up on 
that, and I too will take a look at Denis Bider's code.  The current 
PWSafe code (the real code that goes in the build) does multiple 
overwrites, but doesn't go to the kind of extremes that Peter Gutmann 
recommends in his excellent paper whose title I have forgotten at 
the moment.

By the way, apologies to all for my quietness here of late.  I've been 
a victim of the current economic climate (translation -- I'm job 
hunting), and I'm doing hourly consulting to pay the mortgage.  
I've been hacking at PWSafe, but I've let updates to the sourceforge 
page slide.  I'm going to try to rectify that in the next few days.

Jim R