#81 security issue - password on NT service command line

[John Mazzitelli]

On Windows platform, using the -i option.

To avoid having the account name and password in the
actual wrapper.conf configuration file, users would
want to pass those two configuration settings on the
command line and not specify them at all in the config
file:

wrapper.exe -i wrapper.conf
wrapper.ntservice.account=DOMAIN\username
wrapper.ntservice.password=mypassword

That way, I won't be forced to have my password in
plain text on my filesystem.

However, when installing the NT service, the
wrapper_win.c code seems to take the command line
arguments verbatim when setting up the Service's "Path
to executable" command that is executed when the
service starts. This is actually a good thing, with
the exception that it puts those two args in the
command too. So when I look at my Service GUI and view
my new service's properties, I can see in plain text my
password because it is in the "Path to executable"
field on the GUI.

Request an enhancement such that when the NT service is
installed, it still copies the command line verbatim,
but it should be smart enough to NOT put those two
arguments in there. It should look at all the
arguments and remove the wrapper.ntservice.account=X
and wrapper.ntservice.password=Y arguments if they
exist. This should be harmless because the wrapper
should not need to use those when given -s command line
(which is how the service starts). So, we know it
doesn't need those args. The account/password is only
needed for installing (and maybe uninstalling)???

Otherwise, you either have a security problem with
showing the password OR you have to play games with
environment variables to pass the account name and
password in.

In my case, its doable but adding more work than I had
hoped. I can put the ntservice.password/account
configuration settings in my conf file, but I am using
a global config file for a bunch of services. Some run
as LocalAccount, others as a specific user. The
problem is if wrapper.exe sees an empty or blank
account/password, it throws an error. For example, if
my config has this:

wrapper.ntservice.account=%ACCOUNT%
wrapper.ntservice.password=%PASSWORD%

and I do not define ACCOUNT and/or PASSWORD, i get an
error and it won't install. I will be forced to create
a separate config file that I will include from my
global config file and only put those settings in the
included file if I am to install the service as a
specific user. Much more work than I wanted to do.

[John Mazzitelli]

Logged In: YES
user_id=1179820

That workaround I mentioned before will not work. Because
the environment doesn't pass from the -t invocation to the
-s process (remember, it picks up the environment at boot
time since its the Windows Services framework that actually
starts the service process) I can't simply set environment
variables ACCOUNT and PASSWORD and expect it to work. I was
hoping to do something like this as well, but the same
limitation applies:

wrapper.exe -i wrapper.conf
wrapper.ntservice.password=%PASSWORD%

For security purposes, the only way around it is for the
wrapper.exe to not put the account and password config
settings in the command line when installing the service.

[John Mazzitelli]

Logged In: YES
user_id=1179820

Recommendation for the fix:

In wrapper_win.c, in function wrapperInstall, the case
statement adds the command line args verbatim in the default
statement.

Before doing that if-else where the strcat happens, it
should check that argv[i] does not start with
"wrapper.ntservice.account" or "wrapper.ntservice.password".
If it does, then just continue on without doing any strcat.

default:
/* All other argv[n] should be preserved as is. */
/* If the argument contains spaces, it needs to
be quoted */

if (!stringStartsWith(argv[i],
"wrapper.ntservice.account=") && !stringStartsWith(argv[i],
"wrapper.ntservice.password="))
{
if (strchr(argv[i], ' ') == NULL) {
strcat(binaryPath, argv[i]);
} else {
strcat(binaryPath, "\"");
strcat(binaryPath, argv[i]);
strcat(binaryPath, "\"");
}
}
break;

I forget C - don't know how to fully implement
stringStartsWith - but I'm sure its easy.

[Leif Mortenson]

Logged In: YES
user_id=228081

Thanks for pointing that out. It has been fixed pretty much
as you suggested so neither of those properties will be
stored in the registry. This will be in the 3.2.0 release
and is already in CVS.

Cheers,
Leif