passwordsafe-devel — For developers of Password Safe

[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

[Passwordsafe-devel] SecString class

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

Hi all,

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 suggested a better approach would be to write an allocator that
clears memory on release, then make SecString an instantiation of
the std::string template using this allocator.  The beauty of this
solution is you can create any STL container that clears its memory on 
release.  I mentioned I would be writing such an allocator class myself
soon for another project and would gladly provide a copy of the source.

Well, as I was browsing the Crypto++ library I came across an allocator
class (template, actually) written by a Denis Bider that does exactly
that.  On Windows platforms, it also attempts to lock the pages in
memory with the VirtualLock() system call.  I was contemplating adding
this functionality to my allocator as well, although the MSDN 
information is conflicting as to whether VirtualLock actually locks the
pages in memory or just hints to the OS that is should do so.  Anyway,
the allocator is available (no license restrictions) as part of the
BSL library at

    http://www.bitvise.com/bsl.html

I don't know this Denis Bider and I can't vouch for the quality of
the code.  But on first pass it looks reasonably solid.  My one 
complaint would be the simplistic memory overwriting which merely
zeroes the memory out one time, but this can easily be changed.  I
hope to give the code a more thorough review in the near future.  At
that time I will inform this list of my findings and provide and code
changes I make.  Thanks,

Edward Elliott


__________________________________________________
Do You Yahoo!?
Yahoo! Greetings - Send FREE e-cards for every occasion!
http://greetings.yahoo.com

Re: [Passwordsafe-devel] Development source

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

On or about Thu, Jan 31, 2002 at 08:41:29PM -0000, Martyn Pa...@Ma... wrote:

> Does anyone have a source set for a win32 platform?

As it stands now, the *only* source set is the one for the win32 
platform, especially considering the fact that it's currently
MFC-based.

> I have taken a look at the source available from the CVS system and have a number of comments to make but would like to base my comments of a solid foundation.

The source in CVS is what I checked in.  It consists of the v1.7 
source as provided to me by Bruce S. (the Counterpane Source) with 
the fixes marked on the TODO page at passwordsafe.sourceforge.net 
already applied.  One or two of the modules that I've started to 
write to separate the backend engine from the GUI are also in 
CVS, but are not in the Makefile yet, and are not yet used in 
the application.

I have a few more changes that have not been checked in yet, but 
the last few weeks have been a bit of a speed bump in Password 
Safe development (the company I worked for dissolved, and I've 
been spending a lot of time job-hunting).

Anyway -- Martin, the CVS source is about as solid a foundation 
as we have right now, so comment away based on that.


Jim R

[Passwordsafe-devel] Development source

[Martyn <Pa...@Ma...>]

Dear All

Does anyone have a source set for a win32 platform?

I have taken a look at the source available from the CVS system and have =
a number of comments to make but would like to base my comments of a =
solid foundation.

Martyn Pavey

[Passwordsafe-devel] The Mailing Lists are up and running

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

UPFRONT APOLOGY -- You may get more than one copy of this email.  I'm 
posting it to three mailing lists, and BCC'ing to a number of addresses. 
We'll get it all sorted out soon enough. <g>

Now that that's out of the way...

Thanks to everyone for their messages of congratulations/condolences 
on my appointment as maintainer of PasswordSafe.  You are receiving 
this message either because you subscribed to one of the mailing lists 
I've set up, or because I'm inviting you to do so.  PasswordSafe from 
this point forward will be, I hope, a collaborative effort, and 
mailing lists are a nice public way of hashing out where the 
development effort should go.

The mailing lists are pretty much what you would expect:

passwordsafe-devel -- subscribe to this one if you want to do some 
coding or documentation

passwordsafe-users -- subscribe to this one if you want to submit 
bug reports, feature requests, or complain about the user interface

passwordsafe-announce -- subscribe to this one if you only want to 
know when the next version is released


So subscribe if you haven't yet, and lets get rolling.  Point a 
browser to:

    https://sourceforge.net/mail/?group_id=41019


Jim R