For my Project I'm currently at work to get the KeePass 1.27 sources compiled using VC++ 7.1 (aka "Visual Studio .NET 2003"). I know that this is an outdated and unsupported compiler in the KeePass context, but I'm really really really close to finish, and I honestly believe that my current stumbling blocks in the sources are
either nuisances and trivial to fix,
or legitimate problems (or errors) in the source code,
which need attention anyway and independend from my situation. So please read on while I report what errors and warnings I get.
A mere nuisance: Double template brackets ">>"
The files StrUtil.h and StrUtil.cpp in KeePassLibCpp/Util have nested template instances which end in the token ">>". While I know that a modern and fancy compiler should and will see two tokens, a ">" and another ">", my compiler does not (as I guess will several other compilers out there, too); instead this gives an error and I have to insert the space to separate the two characters by myself.
But hey -- this is probably the easiest-to-solve source code problem report ever!
More serious: Illegal use of a copy constructor.
In the file FullPathName.cpp (directory WinGUI/Util/CmdLine), I get the following warning:
FullPathName.cpp(53) : warning C4239: nonstandard extension used : 'argument' : conversion from 'std::auto_ptr<_Ty>' to 'std::auto_ptr<_Ty> &' with [...] A reference that is not to 'const' cannot be bound to a non-lvalue; assigment operator takes a reference to non-const`
which does in fact involve a copy constructor of SmartCharPtr, but this class is a template instance of auto_ptr, and the copy constructor of auto_ptr is by definition different from all other conventional copy constructors: it modifies it's argument, which is therefore a non-const reference. (This is to implement the special ownership semantics of auto_ptr.)
Now the target variable, heapFullPathName, is default-constructed immediately above in line 49:
SmartCharPtr heapFullPathName;
So my interim "solution" is to move the definition of this object down into line 53, and make it an explicit initialization:
SmartCharPtr heapFullPathName (new TCHAR[length]);
The compiler is happy with this, but not me: the lifetime of this SmartCharPtr now ends on exiting the if statement, but fullPathName will then still point into it's -- now invalid -- allocated string memory. I'm sure there is a better solution to this; but if the code as it is now compiles cleanly, then IMO the compiler is in error. I guess in this case the invocation of the copy ctor gets optimized away, and the compiler "forgets" to diagnose that invoking the ctor had been illegal in the first place. (But I'd have to look it up if the compiler is allowed to act like this or not.)
A curious nuisance: The intractable icon
Close to the end of the build process, when rc processes the PwSave.rc resource file (in WinGUI/res/), it throws up the final diagnostic I get:
PwSafe.rc(80) : error RC2176 : old DIB in res\PwSafe.ico; pass it through SDKPAINT
I know what a DIB is, but I have no idea what's wrong with this one, what SDKPAINT is (in my Windows "Platform SDK 2003 R2" -- yes, as outdated as the compiler! ;-) -- there is no such tool). And I even don't know what it all has to do with an icon file (do .ico files use the DIB format internally?) ...
But on this one I'm probably on my own, because the rest of the world (with it's sleek development environments) seems to have no problem with this icon file -- which is the KeePass Icon itself, btw!
And that's it already. I'm glad I could solve the other problems, like
the use of "SAL Annotations" (eg _In_),
the use of MS's (and ISO's) "safer" library functions (eg strcpy_s),
the use of SDK features that were introduced in Vista (Task Dialogs).
But these all are of course problems in my outdated environment, and not in the source code.
And now only the three hickups keep my venerable compiler from building a current KeePass executable -- too bad! Of course I could patch the source by myself, but I really would like to see these problems addressed in the KeePass source code itself (at least the first two of them) ...
Thank you for your attention, and for your responses!
PS: I did forget to mention that I build using the unmodified KeePass source distribution (freshly extracted from KeePass-1.27-Src.zip). Or rather: I would like to. Because for these remaining three problems I can't create a workaround from "outside", ie without editing the sources. That's the reason why I bother you, the KeePass developer community, in the first place: If I already had applied patches over patches to the source distribution, I wouldn't have dared to come up with these problems after all.
2014-04-11T06:04:59Z: The repository is now up to date and reflects the situation I reported about. Only those (SDK) files of the KeePass 1.27 sources are included which are needed to build the plugin. To build both KeePass and the plugin, you need to "fill in" the rest of the KeePass sources. See the README.html (or README.md) file(s) if you would like to build the project (for the purpose of reproducing my results - in the current stage it's useless otherwise!).
Or if you are interested in how I adapt my build process to cope with the "outdatedness" of my compiler and environment, (rsp with platform differences when compiling "Linuxish" source code): see the 'README.html' and 'POSC.html' files in the 'POSC/' directory; the latter text is also available online.
I am the author of the FullPathName class that you have a problem with. I've been following your proposal to use chipcards as a replacement for keyfiles with considerable interest. That's because, as I see it, the primary security weakness in password managers such as KeePass is the difficulty of keeping the master password and/or keyfile strong (random), secret (uncompromised), and secure (difficult to lose). Your proposal is an attempt to address these issues. I also like that you focus on KeePass 1.x rather than 2.x, because using C# as the basis for a security-critical program makes me uncomfortable. That said, your decision to attempt to build KeePass 1.27 under VC++ 7.1 makes no sense to me.
The FullPathName class was developed under VC++ 8.0, which was the compiler KeePass used at the time. FullPathName.cpp compiles cleanly under that compiler and, as far as I know, under all Visual C++ compilers released since then. It is not clear to me why you want to compile and execute KeePass under a compiler that was obsolete before this code was even written. Such an undertaking seems risky at best, since most of the code has not even been tested with this old compiler.
Your analysis of a supposed “fix” is correct: Moving the definition and initialization of heapFullPathName down into the “else” clause avoids the compiler warning, but at the cost of a crash at run-time, undetected by the compiler. It is important to understand that in C++, a clean compile is no guarantee that the program will not crash. This is not an error in the compiler -- it's a property of the language. Just as in C, one can write programs that will crash at run time without the compiler having any way to anticipate this.
At the time I wrote the FullPathName class, auto_ptr was the only smart pointer in the standard library, so I used it. Since then, better smart pointers have been introduced into C++. Auto_ptr is now considered a failed experiment, and will be deprecated in the next C++ standard. If I were to modify FullPathName now, I'd probably just change the SmartCharPtr typedef from auto_ptr to unique_ptr. Of course, compiling that modification would require a modern C++ compiler – or at least a modern class library.
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
The main reason for my attempt to build using VC++ 7.1 Std is simply that this is the compiler I use personally, at home, in my free time - for work on projects like that (at work - where I'm sitting right now - the IDE is VS 2008 Pro). But I see that maybe it's getting time to churn out some money to MS again ...
(The reason that I stick with KeePass 1.x - for everyday use as well and therefore in development, is mainly to stay compatible with KeePassX on FreeBSD and Mac - ie with a native password manager on these systems.)
By the way, I had successfully used a VC++ 15, the newer one, to built KeePass.exe and KeePass.pdb files and to afterwards load my VC++ 13.1 built plugin into this KeePass executable, and then, still inside VS 2003, the outdated one, debug and trace through the plugin and KeePass inside my outdated/venerable IDE.
So that's an alternative I still have.
The other reason is that I do this as a kind of "sport", or concept proof. As I said, I'm very close to being able to build the whole KeePass application, from its unmodified source distribution, using my old compiler (and said "POSC" scaffold). Using this built for "production" purposes, is obviously a whole different question!
But you might as well call this "stubbornness" ;-)
Back to the C++ questions: That "a clean compile is no guarantee" that a program will work is a mild euphemism ... :-) Most of the time I get rather nervous if a source file compiles clean on the very first attempt - that's often a sign that something totally strange is happening (like you did in fact compile a different file than you thought :-). And I hope I didn't make the impression that I would blame the compiler for this little difficulty I had surrounding heapFullPathName.
I know that auto_ptr is about to be deprecated (wasn't it already in C++ 2011?). And yes, "upgrading" to unique_ptr is an option, maybe the best option.
And I have just looked up that boost did have the unique_ptr template already in ver 1.42 (which I'm using) and even before:
Icon: The PwSafe.ico file contains a high-resolution image in PNG format, which the old resource compiler most likely doesn't know yet. You could replace the PwSafe.ico file by a version that doesn't contain the high-resolution image, like the file Ext\Icons\Finals2\plockb.ico. Obviously, I'm not going to downgrade the icon file just for VC++ 7.1 compatibility :-)
Best regards,
Dominik
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
Bill, I just saw that I was completely silent on your remarks regarding my project as a whole - sorry about that!
Of course I'm very happy to read about your interest in my project (or proposal, as there isn't yet much of a project ...). Even more so given that you are a KeePass "insider", so to say.
I have seen that there were several attempts before mine, that more or less stalled (the KeeCard project is a good example).
So I will very much appreciate any suggestions, comments, questions and help from the people around KeePass - both users and developers.
You are right in pointing out the aspect of convenience: that's my main reason to try out chip cards (beside the pure geek value in it!). I have a rather long password for my main KeePass database, and typing that again and again at different computers is really tiresome.
As I have written from the start, my first goal is to use chip cards as a replacement for key files (an interchangeable one, that is). It is for now not my goal to provide higher security than these key files with passwords. But I'm open for a second round-project that uses "smart cards" with crypto features, and this could indeed improve security beyond what can be done using passwords and key files alone.
I you haven't seen it yet, you are welcome to browse through my project's wiki and blog, where I collect technical and planning details as I go.
it's great to have these pesky ">>" out of my way, a big "thank you" for your quick action!
I don't know much about the different variants of icon files and their internal formats - what you write is quite plausibly the reason for my issue with the icon file.
As I have told Bill, I'm not intending to put the resulting executable into "production" use - it is solely for the purpose of hosting the plugin during development and debugging. So the icon is really not that important ...
(But I wouldn't dare to request back-porting or dumbing-down (re-)source files for my old compiler anyway!)
I wasn't aware (any more) how contentious the auto_ptr class template was from its very beginning, and what a complicated history with multiple differing specifications it had. As I understand it, the difficulties were mostly about pushing around auto_ptr objects, specifically as function arguments and return values. And I think the assignment in line 53 is an example of this: the RHS is a auto_ptr r-value object, while the assignment operator expects a non-const reference to auto_ptr.
But this problem (or sould I say "my" problem?) with auto_ptr at this point can be solved - in a ridiculously easy manner (I'm embarrassed that I didn't realize this sooner), which is at the same time conforming to all ISO 14882 editions I can get a hand on. And has, of course, the same semantics in all of them :
Change the assignment in line 53 to this:
heapFullPathName.reset(new TCHAR[length]);
This makes my compilers - VC++ 7.1 and 9.0 - happy, and I would be very surprised if any other compiler would complain about it.
These claims can be justified in the following way:
(1) In line 49, the pointer object gets default-constructed. Any half-usable implementation of a half-decent specification for auto_ptr sets the "dumb" pointer inside to NULL - I think there is no doubt about this.
(2) Depending on the decision in line 50, there are two cases: either the array stackFullPathName - in the auto storage class - is big enough to hold the pathname, or it is not.
(2.1) If the automatic array is sufficient, the auto_ptr object will not be accessed at all any more. When its scope is up, it will leave this world without ever having owned some free store (poor thing! ;-). The destructor will do nothing in this case, and again this is so in all auto_ptr variants I can read or think of.
(2.2) If the automatic array is too small, the free store is used (or the "heap", if you prefer this term).
(2.2.1) In the modified line 53, the new expression yields a pointer to the allocated storage, and this is taken over by the reset() member function - and the allocated memory is owned by the auto_ptr object hereafter: Again, in all standards and variants.
(2.2.2) The next operation on the pointer object - if any - is the invocation of the get() member function in line 59. (If the first alternative of the if in line 55 is taken, the game is over anyway.) The get() returns a pointer to the owned storage of the pointer object, but does not change it's state. (Again, in all ...)
(2.2.3) This pointer into free store remains valid until either the pointer object changes state to own another piece of storage, or the memory is released explicitly or implicitly. As there are no more accesses to the auto_ptr object, this pointer actually remains valid until the end of the pointer object's lifetime - which is the end of the function scope. And as far I can tell or even imagine, this is again the case in all ...
Thus you can safely go on using the auto_ptr template in this case (which was invented for simple memory management purposes like this), and only need to consider the switch to unique_ptr or so when the compilers starts nagging you about the deprecated auto_ptr template :o) (It is already relegated to appendix D "Compatibility Features" in ISO/IEC 14882:2011, at least in the N3242 draft I use.)
OK, it's fine with me if Dominik is willing to change the assignment of heapFullPathName to an invocation of reset() instead, as you suggest. This gives the same intent as the original code. Thanks to Dominik for bringing me into this discussion.
I have to say, I still think building KeePass with an antique compiler is pointless at best, and just asking for trouble at worst. You're going to have to adopt a modern compiler if you want your code to be taken seriously by others.
Regarding the goals of your project as a whole, I'm actually much more interested in the possibility of a "second-round project" using smart cards than I am in your initial project. I've been using something like that for years with KeePass: The Sony Puppy FIU-810. It's not exactly a smart card; it's sort of a USB flash drive with a microcontroller and a fingerprint reader. The idea is that the keyfile is stored in the Puppy, but is not accessible to the PC until the storage is unlocked by my fingerprint. The PC has no ability to hack the system because the unlocking decision and the reference fingerprints are self-contained in the Puppy. Well, it's definitely not a perfect system, but at least it has the advantage that I can leave the Puppy lying around all the time without compromising my keyfile -- unlike with an ordinary USB flash drive.
The problem is that the Puppy is a very old technology and is limited in many ways. (like VC++ 7.1 :-) I'm looking for a better system, possibly involving biometrics. NYMI? Anyway, I'd be interested to be involved in a project that might lead to smart-device protection for keyfiles.
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
Thank you guys for your swift answers and support!
I go on now and work with the two source code fixes. And I use the "old" icon, but that's indeed my own problem.
@Bill: I don't know how much longer I'm willing to cope with VC++ 7.1 (at home), so an upgrade in the near future is quite possible (or I try out gcc / MinGW first!). Lately I have instead spent the money for a Gimpel PC-Lint license ... So far I'm content writing and compiling code that conforms to ISO/IEC 14882:2003, the second edition of the standard. I believe my compiler (maybe with the help of some libraries like boost) is usable for this.
And yes, I do hope that my code is "taken seriously" by others :-)
The thing I miss right now of the new (for me! ;-) C++ 2011 features is "move semantics", something that I think is really useful.
On the other hand, how many of the C++ compilers out there do conform to the 2011 C++ standard? Visual Studio 2008 for sure does not.
I chose to use "dumb" cards (memory cards, or "storage cards" to contrast against Compact Flash cards etc) for the first incarnation of the project mostly for practical reasons: They are ubiquitous and cheap, and I hope easer to access (I might be wrong on this).
You are absolutely right that this gives away some security advantages that "smart" cards can have. But I expect that the modifications needed to use these are confined to a small part of the plugin, namely in the card reader access (protocol) and likely in the derivation of the key to be provided to KeePass.
The use of USB tokens, biometric devices etc is - while interesting - clearly out of scope for my project, which deals explicitly with chip cards.
So for the time being I'll stick to my plans, but maybe I use ISO "smart cards" (asynchronous, T=0 / T=1 protocol, ISO MF/DF/EF file system) earlier on, in addition to "storage cards" (synchonous, I²C / Two Wire protocol, flat array of bytes).
Only after the basic infrastructure is in place will it make sense to grapple with smart card authentication using the cryptographic features of such cards.
PS: This thread now veers away form discussions about source code to discussions about project goals and features. I'd like to propose to have discussions about that in the project's forum over there.
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
Conformance to the C++11 standard is not an all-or-nothing thing. Every compiler since VC++8.0 has added useful language features that now appear in C++11, even though no compiler provides full C++11 support.
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
Yes, that's certainly true. In general practice however, I prefer to write (and borrow!) C and C++ code conforming to a specific language specification (and if required to a platform specification like POSIX or LSB or the Win32 API) -- not to a specific set of features that my compiler happens to support and that may or may not be missing in the next person's favourite compiler.
Compare for example the implementation status of C++ 2011 features in
I concede that in our case things look different: KeePass is bound to the Windows platform, it is even bound to the Visual C++ family of compilers - version 9.0 or later (what I respect!).
But also note the number of "No" entries for Visual Studio 2010 -- the successor of VS 2008, which does not even appear in the table.
Given this situation of C++ 2011 support on Windows (support of ISO 9899:1999 aka "C99" is even worse), I deem relying on any C++ language features newer than ISO 14882:2003 on this platform unwise, or risky at best. If the large majority of the compilers in use to build Windows software do all support a given feature of C++ 2011 - okay. But examples for this (in the Visual C++ table) seem to be features like "static_assert", "nullptr", "right angle brackets" - all nice, all trivial, and all dispensable IMHO.
To bring this discussion about compiler versions to an end, please do accept that I
do not stick to VC++ 7.1 out of caprice, but have my - though personal - reasons,
already use VC++ 9.0 nearly every day, at work,
yet have to see a VC++ 9.0 feature that would justify doling out several hundred EUR to Microsoft for it (writing ">>", or the assignment in line 53, most certainly do not! - And as I said, I have rather spent the money on Gimpel PC-lint ...),
will most certainly not write code that requires this version,
have nothing against programming language evolution in general and "modern" C++ in particular.
Last edit: Martin Hofmann 2014-04-12
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
due to your constant sneering how "pointless" the use of my "antique" IDE is :o) - and out of curiosity to be honest - I have finally fetched and installed a shiny new Visual C++ 2010 Express IDE.
And after configuring some external tools (the ViEmu plugin is not supported in Express editions, but I need a decent editor), and upgrading the KeePass and KeeChipCard project files - I'm ready to go!
But don't blame me should some C++ constructs slip into the repository that your outdated VC++ 9.0 compiler doesn't support ...! :-p
It is a true spectacle how the new compiler bitches and bites about "behavior change: ...", "obsolete declaration style: please use: ..." and so on in the build log ... ;-)
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
Ha! I knew the "Express" editions come without MFC, but I thought I could use the MFC I already have. - Think twice, dummy! Most of the "this is no longer valid!" error messages stem from the MFC headers! And going through these and try to tweak them so that VC++ 10.0 can finally swallow the header files is something I really don't have the nerve to do.
Next option: Try my luck with VS 2008 Express. This has no MFC either, but maybe VC++ 9.0 is less picky. - And I would be completely in step with the KeePass code base!
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
Gee, even the VC++ 9.0 compiler spits out my "antique" MFC headers!
Coincidentally however, it turns out that this doesn't bother me at all. And after upgrading to a new state-of-the-art boost 1.55 I'm on board with my (not-so new and not-so shiny, but I like the GUI much better this way) Visual C++ 2008 Express IDE:
1>Compiling...
1>Microsoft (R) 32-bit C/C++ Optimizing Compiler Version 15.00.30729.01 for 80x86
1>Copyright (C) Microsoft Corporation. All rights reserved.
[...]
========== Build: 3 succeeded, 0 failed, 0 up-to-date, 0 skipped ==========
And that's what I will be using for the rest of the project.
So thank you Bill for prodding me into this update - and I really mean it, honestly!
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
Btw, I have the feeling that the rejection of the old MFC headers by newer compilers is kind of a point for one side in our discussion about language and compiler versions - but I'm really not sure for which one ;-)
That said, I will now get back to work (the "VS2008/" directory is already updated in the repository) using VC++ 9.0 - and the two source code modifications which you and Dominik kindly accepted.
Kind regards,
Martin Hofmann
(If you don't mind we can finish this thread now.)
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
For my Project I'm currently at work to get the KeePass 1.27 sources compiled using VC++ 7.1 (aka "Visual Studio .NET 2003"). I know that this is an outdated and unsupported compiler in the KeePass context, but I'm really really really close to finish, and I honestly believe that my current stumbling blocks in the sources are
either nuisances and trivial to fix,
or legitimate problems (or errors) in the source code,
which need attention anyway and independend from my situation. So please read on while I report what errors and warnings I get.
A mere nuisance: Double template brackets "
>>"The files
StrUtil.handStrUtil.cppinKeePassLibCpp/Utilhave nested template instances which end in the token ">>". While I know that a modern and fancy compiler should and will see two tokens, a ">" and another ">", my compiler does not (as I guess will several other compilers out there, too); instead this gives an error and I have to insert the space to separate the two characters by myself.But hey -- this is probably the easiest-to-solve source code problem report ever!
More serious: Illegal use of a copy constructor.
In the file
FullPathName.cpp(directoryWinGUI/Util/CmdLine), I get the following warning:The problem here is that line 53 reads
which does in fact involve a copy constructor of
SmartCharPtr, but this class is a template instance ofauto_ptr, and the copy constructor ofauto_ptris by definition different from all other conventional copy constructors: it modifies it's argument, which is therefore a non-const reference. (This is to implement the special ownership semantics ofauto_ptr.)Now the target variable,
heapFullPathName, is default-constructed immediately above in line 49:So my interim "solution" is to move the definition of this object down into line 53, and make it an explicit initialization:
The compiler is happy with this, but not me: the lifetime of this
SmartCharPtrnow ends on exiting theifstatement, butfullPathNamewill then still point into it's -- now invalid -- allocated string memory. I'm sure there is a better solution to this; but if the code as it is now compiles cleanly, then IMO the compiler is in error. I guess in this case the invocation of the copy ctor gets optimized away, and the compiler "forgets" to diagnose that invoking the ctor had been illegal in the first place. (But I'd have to look it up if the compiler is allowed to act like this or not.)A curious nuisance: The intractable icon
Close to the end of the build process, when
rcprocesses thePwSave.rcresource file (inWinGUI/res/), it throws up the final diagnostic I get:I know what a DIB is, but I have no idea what's wrong with this one, what SDKPAINT is (in my Windows "Platform SDK 2003 R2" -- yes, as outdated as the compiler! ;-) -- there is no such tool). And I even don't know what it all has to do with an icon file (do
.icofiles use the DIB format internally?) ...But on this one I'm probably on my own, because the rest of the world (with it's sleek development environments) seems to have no problem with this icon file -- which is the KeePass Icon itself, btw!
And that's it already. I'm glad I could solve the other problems, like
the use of "SAL Annotations" (eg
_In_),the use of MS's (and ISO's) "safer" library functions (eg
strcpy_s),the use of SDK features that were introduced in Vista (Task Dialogs).
But these all are of course problems in my outdated environment, and not in the source code.
And now only the three hickups keep my venerable compiler from building a current KeePass executable -- too bad! Of course I could patch the source by myself, but I really would like to see these problems addressed in the KeePass source code itself (at least the first two of them) ...
Thank you for your attention, and for your responses!
Kind regards,
Martin Hofmann mh@tin-pot.net
PS: I did forget to mention that I build using the unmodified KeePass source distribution (freshly extracted from
KeePass-1.27-Src.zip). Or rather: I would like to. Because for these remaining three problems I can't create a workaround from "outside", ie without editing the sources. That's the reason why I bother you, the KeePass developer community, in the first place: If I already had applied patches over patches to the source distribution, I wouldn't have dared to come up with these problems after all.Related
KeeChipCard: Wiki: Home
Last edit: Martin Hofmann 2014-04-11
2014-04-11T06:04:59Z: The repository is now up to date and reflects the situation I reported about. Only those (SDK) files of the KeePass 1.27 sources are included which are needed to build the plugin. To build both KeePass and the plugin, you need to "fill in" the rest of the KeePass sources. See the
README.html(orREADME.md) file(s) if you would like to build the project (for the purpose of reproducing my results - in the current stage it's useless otherwise!).Or if you are interested in how I adapt my build process to cope with the "outdatedness" of my compiler and environment, (rsp with platform differences when compiling "Linuxish" source code): see the 'README.html' and 'POSC.html' files in the 'POSC/' directory; the latter text is also available online.
Martin Hofmann mh@tin-pot.net
Last edit: Martin Hofmann 2014-04-11
I am the author of the FullPathName class that you have a problem with. I've been following your proposal to use chipcards as a replacement for keyfiles with considerable interest. That's because, as I see it, the primary security weakness in password managers such as KeePass is the difficulty of keeping the master password and/or keyfile strong (random), secret (uncompromised), and secure (difficult to lose). Your proposal is an attempt to address these issues. I also like that you focus on KeePass 1.x rather than 2.x, because using C# as the basis for a security-critical program makes me uncomfortable. That said, your decision to attempt to build KeePass 1.27 under VC++ 7.1 makes no sense to me.
The FullPathName class was developed under VC++ 8.0, which was the compiler KeePass used at the time. FullPathName.cpp compiles cleanly under that compiler and, as far as I know, under all Visual C++ compilers released since then. It is not clear to me why you want to compile and execute KeePass under a compiler that was obsolete before this code was even written. Such an undertaking seems risky at best, since most of the code has not even been tested with this old compiler.
Your analysis of a supposed “fix” is correct: Moving the definition and initialization of heapFullPathName down into the “else” clause avoids the compiler warning, but at the cost of a crash at run-time, undetected by the compiler. It is important to understand that in C++, a clean compile is no guarantee that the program will not crash. This is not an error in the compiler -- it's a property of the language. Just as in C, one can write programs that will crash at run time without the compiler having any way to anticipate this.
At the time I wrote the FullPathName class, auto_ptr was the only smart pointer in the standard library, so I used it. Since then, better smart pointers have been introduced into C++. Auto_ptr is now considered a failed experiment, and will be deprecated in the next C++ standard. If I were to modify FullPathName now, I'd probably just change the SmartCharPtr typedef from auto_ptr to unique_ptr. Of course, compiling that modification would require a modern C++ compiler – or at least a modern class library.
Hi Bill,
thank you for your detailled answer!
The main reason for my attempt to build using VC++ 7.1 Std is simply that this is the compiler I use personally, at home, in my free time - for work on projects like that (at work - where I'm sitting right now - the IDE is VS 2008 Pro). But I see that maybe it's getting time to churn out some money to MS again ...
(The reason that I stick with KeePass 1.x - for everyday use as well and therefore in development, is mainly to stay compatible with KeePassX on FreeBSD and Mac - ie with a native password manager on these systems.)
By the way, I had successfully used a VC++ 15, the newer one, to built
KeePass.exeandKeePass.pdbfiles and to afterwards load my VC++ 13.1 built plugin into this KeePass executable, and then, still inside VS 2003, the outdated one, debug and trace through the plugin and KeePass inside my outdated/venerable IDE.So that's an alternative I still have.
The other reason is that I do this as a kind of "sport", or concept proof. As I said, I'm very close to being able to build the whole KeePass application, from its unmodified source distribution, using my old compiler (and said "POSC" scaffold). Using this built for "production" purposes, is obviously a whole different question!
But you might as well call this "stubbornness" ;-)
Back to the C++ questions: That "a clean compile is no guarantee" that a program will work is a mild euphemism ... :-) Most of the time I get rather nervous if a source file compiles clean on the very first attempt - that's often a sign that something totally strange is happening (like you did in fact compile a different file than you thought :-). And I hope I didn't make the impression that I would blame the compiler for this little difficulty I had surrounding
heapFullPathName.I know that
auto_ptris about to be deprecated (wasn't it already in C++ 2011?). And yes, "upgrading" tounique_ptris an option, maybe the best option.And I have just looked up that boost did have the
unique_ptrtemplate already in ver 1.42 (which I'm using) and even before:http://www.boost.org/doc/libs/1_42_0/doc/html/interprocess/interprocess_smart_ptr.html#interprocess.interprocess_smart_ptr.unique_ptr
So "even I with my rubbish VC++ 7.1" would be fine with replacing
auto_ptr:-)While I'm at it: how about me sending you my "upgrading" result (ie the modified
FullPathName.cpp) as a change proposal in a couple of days?Kind regards,
Martin Hofmann mh@tin-pot.net
'>>': I've replaced these by '> >' now.
Icon: The PwSafe.ico file contains a high-resolution image in PNG format, which the old resource compiler most likely doesn't know yet. You could replace the PwSafe.ico file by a version that doesn't contain the high-resolution image, like the file Ext\Icons\Finals2\plockb.ico. Obviously, I'm not going to downgrade the icon file just for VC++ 7.1 compatibility :-)
Best regards,
Dominik
Bill, I just saw that I was completely silent on your remarks regarding my project as a whole - sorry about that!
Of course I'm very happy to read about your interest in my project (or proposal, as there isn't yet much of a project ...). Even more so given that you are a KeePass "insider", so to say.
I have seen that there were several attempts before mine, that more or less stalled (the KeeCard project is a good example).
So I will very much appreciate any suggestions, comments, questions and help from the people around KeePass - both users and developers.
You are right in pointing out the aspect of convenience: that's my main reason to try out chip cards (beside the pure geek value in it!). I have a rather long password for my main KeePass database, and typing that again and again at different computers is really tiresome.
As I have written from the start, my first goal is to use chip cards as a replacement for key files (an interchangeable one, that is). It is for now not my goal to provide higher security than these key files with passwords. But I'm open for a second round-project that uses "smart cards" with crypto features, and this could indeed improve security beyond what can be done using passwords and key files alone.
I you haven't seen it yet, you are welcome to browse through my project's wiki and blog, where I collect technical and planning details as I go.
Kind regards,
Martin Hofmann mh@tin-pot.net
Dominik,
it's great to have these pesky "
>>" out of my way, a big "thank you" for your quick action!I don't know much about the different variants of icon files and their internal formats - what you write is quite plausibly the reason for my issue with the icon file.
As I have told Bill, I'm not intending to put the resulting executable into "production" use - it is solely for the purpose of hosting the plugin during development and debugging. So the icon is really not that important ...
(But I wouldn't dare to request back-porting or dumbing-down (re-)source files for my old compiler anyway!)
Kind regards,
Martin Hofmann mh@tin-pot.net
Bill,
I wasn't aware (any more) how contentious the
auto_ptrclass template was from its very beginning, and what a complicated history with multiple differing specifications it had. As I understand it, the difficulties were mostly about pushing aroundauto_ptrobjects, specifically as function arguments and return values. And I think the assignment in line 53 is an example of this: the RHS is aauto_ptrr-value object, while the assignment operator expects a non-const reference toauto_ptr.But this problem (or sould I say "my" problem?) with
auto_ptrat this point can be solved - in a ridiculously easy manner (I'm embarrassed that I didn't realize this sooner), which is at the same time conforming to all ISO 14882 editions I can get a hand on. And has, of course, the same semantics in all of them :Change the assignment in line 53 to this:
This makes my compilers - VC++ 7.1 and 9.0 - happy, and I would be very surprised if any other compiler would complain about it.
These claims can be justified in the following way:
(1) In line 49, the pointer object gets default-constructed. Any half-usable implementation of a half-decent specification for
auto_ptrsets the "dumb" pointer inside to NULL - I think there is no doubt about this.(2) Depending on the decision in line 50, there are two cases: either the array
stackFullPathName- in the auto storage class - is big enough to hold the pathname, or it is not.(2.1) If the automatic array is sufficient, the
auto_ptrobject will not be accessed at all any more. When its scope is up, it will leave this world without ever having owned some free store (poor thing! ;-). The destructor will do nothing in this case, and again this is so in allauto_ptrvariants I can read or think of.(2.2) If the automatic array is too small, the free store is used (or the "heap", if you prefer this term).
(2.2.1) In the modified line 53, the
newexpression yields a pointer to the allocated storage, and this is taken over by thereset()member function - and the allocated memory is owned by theauto_ptrobject hereafter: Again, in all standards and variants.(2.2.2) The next operation on the pointer object - if any - is the invocation of the
get()member function in line 59. (If the first alternative of theifin line 55 is taken, the game is over anyway.) Theget()returns a pointer to the owned storage of the pointer object, but does not change it's state. (Again, in all ...)(2.2.3) This pointer into free store remains valid until either the pointer object changes state to own another piece of storage, or the memory is released explicitly or implicitly. As there are no more accesses to the
auto_ptrobject, this pointer actually remains valid until the end of the pointer object's lifetime - which is the end of the function scope. And as far I can tell or even imagine, this is again the case in all ...Thus you can safely go on using the
auto_ptrtemplate in this case (which was invented for simple memory management purposes like this), and only need to consider the switch tounique_ptror so when the compilers starts nagging you about the deprecatedauto_ptrtemplate :o) (It is already relegated to appendix D "Compatibility Features" in ISO/IEC 14882:2011, at least in the N3242 draft I use.)Kind regards,
Martin Hofmann mh@tin-pot.net
Last edit: Martin Hofmann 2014-04-11
OK, it's fine with me if Dominik is willing to change the assignment of heapFullPathName to an invocation of reset() instead, as you suggest. This gives the same intent as the original code. Thanks to Dominik for bringing me into this discussion.
I have to say, I still think building KeePass with an antique compiler is pointless at best, and just asking for trouble at worst. You're going to have to adopt a modern compiler if you want your code to be taken seriously by others.
Regarding the goals of your project as a whole, I'm actually much more interested in the possibility of a "second-round project" using smart cards than I am in your initial project. I've been using something like that for years with KeePass: The Sony Puppy FIU-810. It's not exactly a smart card; it's sort of a USB flash drive with a microcontroller and a fingerprint reader. The idea is that the keyfile is stored in the Puppy, but is not accessible to the PC until the storage is unlocked by my fingerprint. The PC has no ability to hack the system because the unlocking decision and the reference fingerprints are self-contained in the Puppy. Well, it's definitely not a perfect system, but at least it has the advantage that I can leave the Puppy lying around all the time without compromising my keyfile -- unlike with an ordinary USB flash drive.
The problem is that the Puppy is a very old technology and is limited in many ways. (like VC++ 7.1 :-) I'm looking for a better system, possibly involving biometrics. NYMI? Anyway, I'd be interested to be involved in a project that might lead to smart-device protection for keyfiles.
I've changed line 53 to 'heapFullPathName.reset(new TCHAR[length]);', like discussed.
Best regards,
Dominik
Thank you guys for your swift answers and support!
I go on now and work with the two source code fixes. And I use the "old" icon, but that's indeed my own problem.
@Bill: I don't know how much longer I'm willing to cope with VC++ 7.1 (at home), so an upgrade in the near future is quite possible (or I try out gcc / MinGW first!). Lately I have instead spent the money for a Gimpel PC-Lint license ... So far I'm content writing and compiling code that conforms to ISO/IEC 14882:2003, the second edition of the standard. I believe my compiler (maybe with the help of some libraries like
boost) is usable for this.And yes, I do hope that my code is "taken seriously" by others :-)
The thing I miss right now of the new (for me! ;-) C++ 2011 features is "move semantics", something that I think is really useful.
On the other hand, how many of the C++ compilers out there do conform to the 2011 C++ standard? Visual Studio 2008 for sure does not.
I chose to use "dumb" cards (memory cards, or "storage cards" to contrast against Compact Flash cards etc) for the first incarnation of the project mostly for practical reasons: They are ubiquitous and cheap, and I hope easer to access (I might be wrong on this).
You are absolutely right that this gives away some security advantages that "smart" cards can have. But I expect that the modifications needed to use these are confined to a small part of the plugin, namely in the card reader access (protocol) and likely in the derivation of the key to be provided to KeePass.
The use of USB tokens, biometric devices etc is - while interesting - clearly out of scope for my project, which deals explicitly with chip cards.
So for the time being I'll stick to my plans, but maybe I use ISO "smart cards" (asynchronous, T=0 / T=1 protocol, ISO MF/DF/EF file system) earlier on, in addition to "storage cards" (synchonous, I²C / Two Wire protocol, flat array of bytes).
Only after the basic infrastructure is in place will it make sense to grapple with smart card authentication using the cryptographic features of such cards.
Kind regards,
Martin Hofmann mh@tin-pot.net
PS: This thread now veers away form discussions about source code to discussions about project goals and features. I'd like to propose to have discussions about that in the project's forum over there.
Conformance to the C++11 standard is not an all-or-nothing thing. Every compiler since VC++8.0 has added useful language features that now appear in C++11, even though no compiler provides full C++11 support.
Yes, that's certainly true. In general practice however, I prefer to write (and borrow!) C and C++ code conforming to a specific language specification (and if required to a platform specification like POSIX or LSB or the Win32 API) -- not to a specific set of features that my compiler happens to support and that may or may not be missing in the next person's favourite compiler.
Compare for example the implementation status of C++ 2011 features in
GCC -- pretty good, I'd say!
clang -- very close to complete.
Intel C++ -- nope, C++ 1998 (!) only.
MS Visual C++ -- a hodgepodge of "Yes" and "No".
I concede that in our case things look different: KeePass is bound to the Windows platform, it is even bound to the Visual C++ family of compilers - version 9.0 or later (what I respect!).
But also note the number of "No" entries for Visual Studio 2010 -- the successor of VS 2008, which does not even appear in the table.
Given this situation of C++ 2011 support on Windows (support of ISO 9899:1999 aka "C99" is even worse), I deem relying on any C++ language features newer than ISO 14882:2003 on this platform unwise, or risky at best. If the large majority of the compilers in use to build Windows software do all support a given feature of C++ 2011 - okay. But examples for this (in the Visual C++ table) seem to be features like "static_assert", "nullptr", "right angle brackets" - all nice, all trivial, and all dispensable IMHO.
To bring this discussion about compiler versions to an end, please do accept that I
do not stick to VC++ 7.1 out of caprice, but have my - though personal - reasons,
already use VC++ 9.0 nearly every day, at work,
yet have to see a VC++ 9.0 feature that would justify doling out several hundred EUR to Microsoft for it (writing "
>>", or the assignment in line 53, most certainly do not! - And as I said, I have rather spent the money on Gimpel PC-lint ...),will most certainly not write code that requires this version,
have nothing against programming language evolution in general and "modern" C++ in particular.
Last edit: Martin Hofmann 2014-04-12
Bill,
due to your constant sneering how "pointless" the use of my "antique" IDE is :o) - and out of curiosity to be honest - I have finally fetched and installed a shiny new Visual C++ 2010 Express IDE.
And after configuring some external tools (the ViEmu plugin is not supported in Express editions, but I need a decent editor), and upgrading the KeePass and KeeChipCard project files - I'm ready to go!
But don't blame me should some C++ constructs slip into the repository that your outdated VC++ 9.0 compiler doesn't support ...! :-p
Kind regards
Martin Hofmann mh@tin-pot.net
PS: I hope that the tongue-in-cheekness of my words is obvious!
Last edit: Martin Hofmann 2014-04-12
It is a true spectacle how the new compiler bitches and bites about "behavior change: ...", "obsolete declaration style: please use: ..." and so on in the build log ... ;-)
Ha! I knew the "Express" editions come without MFC, but I thought I could use the MFC I already have. - Think twice, dummy! Most of the "this is no longer valid!" error messages stem from the MFC headers! And going through these and try to tweak them so that VC++ 10.0 can finally swallow the header files is something I really don't have the nerve to do.
Next option: Try my luck with VS 2008 Express. This has no MFC either, but maybe VC++ 9.0 is less picky. - And I would be completely in step with the KeePass code base!
Gee, even the VC++ 9.0 compiler spits out my "antique" MFC headers!
Coincidentally however, it turns out that this doesn't bother me at all. And after upgrading to a new state-of-the-art boost 1.55 I'm on board with my (not-so new and not-so shiny, but I like the GUI much better this way) Visual C++ 2008 Express IDE:
And that's what I will be using for the rest of the project.
So thank you Bill for prodding me into this update - and I really mean it, honestly!
You're welcome.
:-)
Thanks!
Btw, I have the feeling that the rejection of the old MFC headers by newer compilers is kind of a point for one side in our discussion about language and compiler versions - but I'm really not sure for which one ;-)
That said, I will now get back to work (the "VS2008/" directory is already updated in the repository) using VC++ 9.0 - and the two source code modifications which you and Dominik kindly accepted.
Kind regards,
Martin Hofmann
(If you don't mind we can finish this thread now.)