FWD » Core Development » Runtime Infrastructure

If an OS user is added without a password, running the spawner will cause a segmentation fault. This might appear in the server.log like this:

[05/17/2019 08:21:20 UTC] (ClientSpawner.spawn():SEVERE) {qtp33435545402-22} Failed spawn with exit code: 139

Because the spawner executable itself is abending, you will not see output in the syslog (spawner normally logs errors to syslog). The FWD client does not launch so there is no client log. And the exit code 139 is not a possible spawner error code. In this case, there must be a null pointer for the password and when we dereference it the spawner process traps. The OS returns the errno 139 (which is 128 + 11 or SEGV which is a segmentation violation/protection fault/access violation). The ClientSpawner reads this back from the OS and creates the server log entry.

Here is an example of how to create a user without a password:

groupadd <username>
useradd -g <username> <username>

Use this user from the virtual desktop mode web client and it will cause this failure.

I suspect the problem is in src/native/spawn.c in function check_user() when we call crypt() with a 2nd parameter that is NULL.

I don't think PAM authentication is affected but we should check it too.

More findings so far debugging spawn.c, Ubuntu+Chrome:
1. For the user without password the Java side sends "" empty string as password via OutputStreamWriter with stdout pipe.
2. The spawn.c reads the password by getline() function. The call returns -1 for total bytes received and sets 120 into len variable:

   /* read a line from the console */
   int read = getline(&pswd, len, stdin);

...
         // considering EOF encountered 
         if (pswd == NULL)
         {
            pswd = malloc(1);
         }
         else
         {
            pswd = realloc(pswd, 1);
         }
         pswd[0] = '\0';
...

...
   if (password != NULL && strlen(password) > 0)
   {
      /* Get a pointer to a structure containing the broken-out fields
      of the record in the shadow password database that matches the username name. */
      sp = getspnam(pw->pw_name);
      endspent();
...

...
   if (password != NULL)
   {
      /* Get a pointer to a structure containing the broken-out fields
      of the record in the shadow password database that matches the username name. */
      sp = getspnam(pw->pw_name);
      endspent();
...

Greg,

One question. Does the Ubuntu 18.04 support passwordless user? I read the guest user support was removed when migrating from 16.04. This can be serious restriction for using web client even we clear all FWD related issues. Is there a way to test if the user added by instructions from #4122-1 is working fine in OS itself(without FWD usage)?

Does the Ubuntu 18.04 support passwordless user?

I think so:

https://askubuntu.com/questions/1102195/how-to-login-to-ubuntu-18-04-without-a-password

Is there a way to test if the user added by instructions from #4122-1 is working fine in OS itself(without FWD usage)?

We don't have to support the #4122-1 case, if it is really a broken login on Ubuntu. We do have to ensure that we are safe (no abend).

If it is possible to use a no password account on Ubuntu, then we should also support that in the spawner. Whether it is the #4122-1 scenario or not, does not matter.

Greg Shah wrote:

Does the Ubuntu 18.04 support passwordless user?

I think so:

https://askubuntu.com/questions/1102195/how-to-login-to-ubuntu-18-04-without-a-password

Is there a way to test if the user added by instructions from #4122-1 is working fine in OS itself(without FWD usage)?

We don't have to support the #4122-1 case, if it is really a broken login on Ubuntu. We do have to ensure that we are safe (no abend).

If it is possible to use a no password account on Ubuntu, then we should also support that in the spawner. Whether it is the #4122-1 scenario or not, does not matter.

OK. Let me clarify several points regarding #4122-1 scenario.

1. Assume two cases. In one there are two users, user1 that regular user with password, home dir and etc... And another user2 without password and created as in #4122-1. The user1 starts web browser and attempts to run client using user2 in login. In another case the user2 is used for both run browser and then login to FWD. Which case describes the failure in #4122-1?

2. What was the OS version at the time of failure? 16.04 or 18.04? Can you see the failure in a current 18.04 system if initially the testing system was 16.04?

1. Assume two cases. In one there are two users, user1 that regular user with password, home dir and etc... And another user2 without password and created as in #4122-1. The user1 starts web browser and attempts to run client using user2 in login. In another case the user2 is used for both run browser and then login to FWD. Which case describes the failure in #4122-1?

I don't know for sure, but it is probably "user1 starts web browser and attempts to run client using user2 in login".

What was the OS version at the time of failure? 16.04 or 18.04? Can you see the failure in a current 18.04 system if initially the testing system was 16.04?

I think it was always 18.04.

From Eugenie:

The current status is we have the spawn started with error code is 0. However nothing is happening and the client is not showing. Need to understand the reasons and separate possible FWD issues from OS constrains we may have when running the process on behalf of the passwordless user. The message on login screen is 'Cannot start the embedded server.'. This also be a kind of the config issue due to requirement to adjust directory.xml to the another passwordless web client startup condition. Continue working. One thing for sure - we do not have segmentation violation/protection fault/access violation issue now. Need to clarify what is the branch I can commit the issue.

I wonder what logging or error checking can be added to the client to make this (and future) diagnosis much easier.

Changes can be placed in 4335a.

The task branch 4335a has been updated for review to revision 11363.

This is the set of changed to enable no-issue passwordless user. The idea is to check if the user is valid and has no password, not just the user with missing password or missing user. The result - we have no SEGV issue. However the client is not starting with 'Cannot start the embedded server.' message in browser.

What I think is this can be configuration issue that prevents to run passwordless user command or another GUI session with different users.

Warning, this is only review version, has many debug outputs, if you want to see what is happening on your system. If the changes are OK I'll commit the version that is cleaned from extra debug info calls, because one of the call has password in plain text, so this version is security unsafe. But the calls are very informative to understand what is happening.

About future plans. Taking into account the timing pressure from plan-board implementation task should I stop working on this issue and move to the 3822a? Or we need to investigate make the Web client working for passwordless user? BTW, as additional info the Swing client also not started using passwordless user account. So we can have the OS restrictions here, anyway it will take additional time to investigate the reasons. May be later. What do you think?

I'm fine with the code changes.

Please clean up the code. I want to leave the logging there, but changed:

1. Remove all the EVL extra comments.
2. Do not ever display the password. Change the logging to say "password is null" or "password is empty" or "password is non-empty".
3. If possible, it would be very useful to allow the extra logging to be turned on using extra configuration in the directory. The idea is to be able to pass a debug level on the spawner command line.

Greg Shah wrote:

I'm fine with the code changes.

Please clean up the code. I want to leave the logging there, but changed:

1. Remove all the EVL extra comments.

OK. I use this mark to not to forget what I'm currently changing.

2. Do not ever display the password. Change the logging to say "password is null" or "password is empty" or "password is non-empty".

OK.

3. If possible, it would be very useful to allow the extra logging to be turned on using extra configuration in the directory. The idea is to be able to pass a debug level on the spawner command line.

How about spawn.ini? Can we use this file instead of directory.xml? The advantage - we can read it directly from native spawn code. Or you think it is a bad idea?

The task branch 4335a has been updated for review to revision 11364.

Almost all notes are cleared in this update. The thing I'm working on now is to do the extra logging level configurable. Expecting to finish later today.

The spawn.ini is only used on Windows. I would like a solution that works on all platforms. We already pass parameters on the command line when we execute the spawner. Why not just pass a debug level parameter?

Greg Shah wrote:

The spawn.ini is only used on Windows. I would like a solution that works on all platforms. We already pass parameters on the command line when we execute the spawner. Why not just pass a debug level parameter?

OK. Currently I'm implementing the way you noted.

Guys,

I have faced with strange issue here. The code in ClientBuilder.buildCommand():

...
      if (Utils.getDirectoryNodeBoolean(
               null, "clientConfig/spawnerExtraLogs", false, Utils.DirScope.SERVER, null))
      {
         command.add("extra_logs");
      }
...

can not find the directory.xml node in:

...
        <node class="container" name="clientConfig">
...
          <node class="boolean" name="spawnerExtraLogs">
            <node-attribute name="value" value="TRUE"/>
          </node>
...

The reason the DirectoryService object refuses to get the read only lock. Is it expected? Are there the server side code where the directory scan service is not available?

The reason the DirectoryService object refuses to get the read only lock. Is it expected? Are there the server side code where the directory scan service is not available?

Yes, I don't think it has the security context to access the directory. I think the extra data must be looked up and stored in ClientBuilderOptions instead.

Please just use an integer "spawner-debug-level" instead of "extra logging". A debug level is more flexible and can be enhanced later by just making changes in the spawner.

The task branch 4335a has been updated for review to revision 11369.

This adds support for configurable debug level for native spawn code. The optional parameter in directory is:

...
        <node class="container" name="clientConfig">
...
          <node class="integer" name="spawner-debug-level">
            <node-attribute name="value" value="NEW_LEVEL"/>
          </node>
...

The default value is 0 meaning no extra logging to output. For now we support only one extra level(any number >0) meaning to log all important info. If directory has no parameter - the level 0 is assumed.

The TODO list for this task for future work. The remaining issues are related to the fact we can not start the client for passwordless user in Ubuntu 18.04. Possible ways to solve:
1. Walk through the spawn() function implementation to check all new session settings for compatibility with conditions required to successfully start new process with another user.
2. Check the OS level compatibility for passwordless user session. As far as I know by default the guest user session support has been disabled since 18.04. There can be some OS restrictions preventing to start Web client for passwordless user.

Code Review Task Branch 4335a Revision 11369

Overall, my primary concern is that from the Java perspective we appear to be adding debug_level to the JAVA command line, but it is really being processed by the spawner. And it is still passed to java so I worry that it can confuse the ClientDriver in the future because there is extra text at the end. This may not occur today, but if so it is only by chance. Instead I prefer that we pass this outside of the command itself (preferably after the mode but before the command). I think this is a safer long term approach.

Other minor comments:

1. I think the use of strchr is not needed. The sscanf can just reference &dbg_level[12] directly.

2. On most systems, the C constant for EOF will be -1. This means that the condition if (rc > 0 && rc != EOF) is a bit redundant.

Greg Shah wrote:

Code Review Task Branch 4335a Revision 11369

Overall, my primary concern is that from the Java perspective we appear to be adding debug_level to the JAVA command line, but it is really being processed by the spawner. And it is still passed to java so I worry that it can confuse the ClientDriver in the future because there is extra text at the end. This may not occur today, but if so it is only by chance. Instead I prefer that we pass this outside of the command itself (preferably after the mode but before the command). I think this is a safer long term approach.

You mean the command like: spawn 1 debug_level=1 java all_the_rest_of_java_args?

Other minor comments:

1. I think the use of strchr is not needed. The sscanf can just reference &dbg_level[12] directly.

2. On most systems, the C constant for EOF will be -1. This means that the condition if (rc > 0 && rc != EOF) is a bit redundant.

OK. I'll fix in a next update.

You mean the command like: spawn 1 debug_level=1 java all_the_rest_of_java_args?

Yes.

Greg Shah wrote:

You mean the command like: spawn 1 debug_level=1 java all_the_rest_of_java_args?

Yes.

OK. Agree, this form is better.

The task branch 4335a has been updated for review to revision 11391.

This is the notes resolution changes. The debug mode option is now just before the working directory option.

Code Review Task Branch 4335a Revision 11391

The changes are a nice improvement. I think they will work in smode 1. What about in smode 0? If someone tries to enable debug logging in that case, I think the hard coded argv[2-5] references will no longer be correct.

Greg Shah wrote:

Code Review Task Branch 4335a Revision 11391

The changes are a nice improvement. I think they will work in smode 1. What about in smode 0? If someone tries to enable debug logging in that case, I think the hard coded argv[2-5] references will no longer be correct.

I also worried about this incompatibility. But I have found in this mode all (2-5) options will be before debug option. So we do not need to make index shift in this case. So it is safe I think.

Great! Please add comments to the code (in the smode == 0 case) to explain this.

Greg Shah wrote:

Great! Please add comments to the code (in the smode == 0 case) to explain this.

Done.

The task branch 4335a has been updated for review to revision 11392.

Task branch 4335a was merged to trunk as revision 11345.