pseymour
Corrupted users.xml causes service failure
If "C:\Program Data\Make Me Admin\Users.xml" exists, but has invalid data in it then the MakeMeAdmin service starts to fail. I've only seen this once after a computer blue-screened, so I'm not sure how important it really is to address or in-fact what the best behaviour really is, but I thought I should at least document the behaviour here so that it can be addressed.
In this case you can get an error logged such as the following:
Failed to process session change. System.Security.Cryptography.CryptographicException: The data is invalid. at System.Security.Cryptography.ProtectedData.Unprotect(Byte[] encryptedData, Byte[] optionalEntropy, DataProtectionScope scope) at SinclairCC.MakeMeAdmin.EncryptedSettings.Load(String filePath) at SinclairCC.MakeMeAdmin.EncryptedSettings..ctor(String filePath) at SinclairCC.MakeMeAdmin.MakeMeAdminService.OnSessionChange(SessionChangeDescription changeDescription) at System.ServiceProcess.ServiceBase.DeferredSessionChange(Int32 eventType, Int32 sessionId)
This raises a couple of questions. Firstly, if the service is unable to deserialise and decrypt the contents of the file, what is the correct behaviour? Should it create a new empty settings file instead of failing? Should it be keeping a copy in memory and using that instead if it isn't the initial service startup?
Also, in this instance the calls from the UI into the service were failing. Should exceptions generated on the service side of these calls be logged? (similar to how the base service code logs the session change error). Currently, exceptions within these are not handled or logged in any way.
Finally, the folder "C:\ProgramData\Make Me Admin" doesn't get created at install time - it is only created when an action such as adding or removing a user gets triggered that results in saving the file. The permissions on ProgramData are such that a standard user can create a folder under C:\ProgramData without any special permissions, which means that a standard user is able to disrupt the service for someone else in the future if MakeMeAdmin has not yet been used on that computer.
You mentioned in the other issue that perhaps a using is in order. I suppose it doesn't hurt, but when the file is written, Close() is called, which in turns calls Dispose(). I'm not sure how much of an effect using would have.
The corruption seems to happen pretty rarely, so I haven't put much effort into it. In most of the cases I know of, Windows had crashed. That doesn't help matters. :)
I think maybe it would be appropriate, when saving the file, to check that the bytes were written properly. Maybe by hashing the file and the encrypted byte array, which is still in memory at the time.