Allow Configurator on secure desktop only (release mode) #95

DonatJR · 2018-08-04T19:02:36Z

fixes #55

regarding WIP: @SailReal the CageConfigurator.sconfig file must be copied to the public documents folder on install and the registry needs to have a corresponding entry

!! Only merge this after #91 !!

SailReal · 2018-08-04T20:10:42Z

SharkCage/special_configs/CageConfigurator.sconfig

@@ -0,0 +1,12 @@
+{
+  "application_name": "CageConfigurator",
+  "application_path": "C:\\sharkcage\\CageConfigurator.exe",


I'm currently enhancing our installer. The application_path must be written by the installer or? It depends on the target folder specified by the user while installing.

Yeah, we can change the path to a placeholder and then replace it when installing

…g to user system #95

SailReal · 2018-08-05T12:40:59Z

SharkCage/CageServiceInstaller/CageServiceInstaller.cs

+using System.IO;
+using System.Security.AccessControl;
+using System.Security.Cryptography;
+using System.Security.Principal;

 namespace CageServiceInstaller


I still have to rename the project...

SailReal · 2018-08-05T15:42:33Z

regarding WIP: @SailReal the CageConfigurator.sconfig file must be copied to the public documents folder on install and the registry needs to have a corresponding entry

Should be fixed now.

@DonatJR will review your additions in this pr now. Maybe you can check my?

SailReal

Awesome work, thx a lot 👍!!!

If I build the solution with release target, the cage manager will not be started by the chooser when selecting an config and pressing the start button. Only a error with message "Error communicating with service: Starting CageManager failed." is displayed.
If I start the CageManager manually before pressing the start button in the CageChooser, the desktop is switched and everything seems to be fine. After switching back, the same error message is displayed.

SailReal · 2018-08-05T17:24:41Z

SharkCage/CageChooser/CageChooserForm.cs

@@ -49,11 +49,11 @@ private void LoadConfigList()
            {
                foreach (var value_name in key.GetValueNames())
                {
+


Pls remove this empty newline.

SailReal · 2018-08-05T17:29:27Z

SharkCage/CageConfigurator/CageConfigurator.cs

+            }
+        }
+
+        // FIXME might be easier to ask the service for this info in the future? :/


Maybe we should create an issue for that? Otherwise we can remove the FIXME.

SailReal · 2018-08-05T17:47:07Z

SharkCage/CageConfigurator/CageConfiguratorForm.cs

@@ -91,6 +94,7 @@ private void CheckPath(string path, string[] file_types, Action<string> onSucces
                return path.EndsWith(type);
            });

+            // fixme: check if path contains cage configurator and disallow it?


Maybe we should create an issue for that as well?

this can just be deleted as we now explicitely want to be able to create a config for the Configurator

DonatJR · 2018-08-05T20:36:45Z

@SailReal Regarding your issue with the release mode: Did you copy all necessary files into the 'install folder'? If yes, did you recreate the config with the new release binary of the Configurator?

I think I know why (from the Service):

#ifdef _DEBUG
    return StartCageManager(session_id, filename, user_token);
#else
    if (SharedFunctions::ValidateCertificate(filename))
    {
        return StartCageManager(session_id, filename, user_token);
    }
    else
    {
        std::wostringstream os;
        os << "Failed to validate the integrity of CageManager! Not starting." << std::endl;
        ::OutputDebugString(os.str().c_str());
        return 0;
    }
#endif

In release mode the Manager must be signed ;)

DonatJR

@SailReal
can't request changes on my own PR :) additional things:

can you remove the Configurator desktop shortcut? it cannot be launched with the shortcut anyway

apart from the comments your changes look good, thx!

DonatJR · 2018-08-05T21:00:44Z

SharkCage/CageServiceInstaller/SharkCageInstallerHelper.cs

+            UninstallService();
+        }
+
+        private void AssimilateConfigToCurrentSystem(string dir_path)


I don't quite get the name of this function, why 'assimilate'?

I meant something like customize...?

DonatJR · 2018-08-05T21:01:03Z

SharkCage/CageServiceInstaller/SharkCageInstallerHelper.cs

+        {
+            var config_path = Path.Combine(
+                Environment.GetFolderPath(Environment.SpecialFolder.CommonDocuments),
+                "SharkCage\\CageConfigurator.sconfig");


You can use @"one\slash\only" if you don't want to use characters which are needed to be escaped (with @ everything is interpreted literally

DonatJR · 2018-08-05T21:01:22Z

SharkCage/CageServiceInstaller/SharkCageInstallerHelper.cs

+            const string APPLICATION_HASH_PROPERTY = "binary_hash";
+            var path_to_config_exe = Path.Combine(dir_path, "CageConfigurator.exe");
+
+            var json = JObject.Parse(File.ReadAllText(config_path));


the json parsing can fail (if the file is missing or has a different structure), do we need a try catch here or does the installer handle it for us?

The installer is handling the exception and also will rollback all changes on the system, will display an error message with the content of the exception so nothing to do for us I think...

DonatJR · 2018-08-05T21:01:42Z

SharkCage/CageServiceInstaller/SharkCageInstallerHelper.cs

+            var json = JObject.Parse(File.ReadAllText(config_path));
+            var application_path = json.GetValue(APPLICATION_PATH_PROPERTY).ToString();
+            json[APPLICATION_PATH_PROPERTY] = path_to_config_exe;
+            json[APPLICATION_HASH_PROPERTY] = GetSha512Hash(path_to_config_exe);


I'm not sure patching the hash on the target system is a good idea. We can generate the hash once before creating the installer and just patch the correct path

The hash looks different on our machines with the same target (debug) so I thought it will be the same result with the release target. I don't know what but something on the system must have an influence on the calculation of the hash over the binary. So I think the only way is it to calculate the hash on the target system but I think that should not decrease the security at all especially because up to the point of installation, the config is writable.

Maybe we should create an issue for that to think about how we can install the cage in a secure way on a already infected system.

but the hash only changes on creation. once it is created and signed the hash can be written to the config and the binary hash must be the same on the target machine for us to be sure there was no tampering during distribution

Will create an issue for that.

SailReal · 2018-08-05T21:07:08Z

Did you copy all necessary files into the 'install folder'? If yes, did you recreate the config with the new release binary of the Configurator?

@DonatJR I used the installer to copy all files and also created new configs. Is it working on your device?

SailReal · 2018-08-05T21:37:34Z

I think I know why (from the Service):

oh nooooo you're right, the CageManager wasn't signed 😅 now it's working like a charm 😍

bencikpeter · 2018-08-06T14:29:44Z

Shouldn´t there be a default entry in Chooser to launch Configurator? I am a bit confused about how should I start Configurator in Release? I need an entry to launch Configurator, and to create entry I need Configurator? 🤔

DonatJR · 2018-08-06T14:36:39Z

@bencikpeter
The installer creates an entry in the registry. On your dev system you can simple create your on in the registry itself at Computer\HKEY_LOCAL_MACHINE\SOFTWARE(\WOW6432Node)\SharkCage\Configs (REG_SZ, with name and the path to the config file as value)

Or you can use the debug build of the Configurator to create configs and use them with the release build of all the other applications

DonatJR added 8 commits June 26, 2018 21:05

add special config for CageConfigurator to project

5cec5e6

preliminary work to detect on which desktop the configurator was started

1e1b7e7

chooser now starts configurator directly in cage

70555ae

Merge branch 'develop' into 55-config-in-cage

0eca7d7

add ability to check ACL of current desktop to configurator

0ec51e1

merge develop

c54ccdf

makes CageConfigurator only start on secure desktop (in release mode)

b959fc7

remove unnecessary post build copy

081a2a3

DonatJR requested review from SailReal and baWedekind August 4, 2018 19:02

DonatJR mentioned this pull request Aug 4, 2018

66 Starting caged applications under correct privileges #91

Merged

SailReal reviewed Aug 4, 2018

View reviewed changes

SailReal added 3 commits August 5, 2018 13:25

#55 copy config to user folder and path to registry. assimilate confi…

28f9400

…g to user system #95

#55 fix config registry entry written during the installation #95

785dc6d

#55 simplified CageServiceInstaller #95

efb0696

SailReal reviewed Aug 5, 2018

View reviewed changes

#55 rename CageServiceInstaller to SharkCageInstallerHelper #95

89c3b49

SailReal suggested changes Aug 5, 2018

View reviewed changes

suggestions from review (#95)

c45379a

DonatJR commented Aug 5, 2018

View reviewed changes

fix potential close of invalid handle

e4598f7

SailReal added 3 commits August 5, 2018 23:39

#55 remove CageConfig from the user desktop etc #95

448e141

#55 use @ to interpreted path literally #95

99425e7

#55 enhance function naming in SharkCageInstallerHelper #95

109aaa2

SailReal approved these changes Aug 5, 2018

View reviewed changes

DonatJR merged commit d457a7b into develop Aug 5, 2018

DonatJR changed the title ~~WIP: Allow Configurator on secure desktop only (release mode)~~ Allow Configurator on secure desktop only (release mode) Aug 5, 2018

DonatJR deleted the 55-config-in-cage branch August 5, 2018 21:56

DonatJR mentioned this pull request Aug 5, 2018

Start Configurator in cage #55

Closed

SailReal mentioned this pull request Aug 5, 2018

Hash of the CageConfigurator in the config #97

Open

SailReal mentioned this pull request Aug 9, 2018

87 cleanup readme and remove unused .md files #99

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allow Configurator on secure desktop only (release mode) #95

Allow Configurator on secure desktop only (release mode) #95

DonatJR commented Aug 4, 2018 •

edited

Loading

SailReal Aug 4, 2018 •

edited

Loading

DonatJR Aug 4, 2018 •

edited

Loading

SailReal Aug 5, 2018

SailReal commented Aug 5, 2018

SailReal left a comment

SailReal Aug 5, 2018

SailReal Aug 5, 2018

DonatJR Aug 5, 2018

SailReal Aug 5, 2018

DonatJR Aug 5, 2018

DonatJR commented Aug 5, 2018 •

edited

Loading

DonatJR left a comment •

edited

Loading

DonatJR Aug 5, 2018

SailReal Aug 5, 2018

DonatJR Aug 5, 2018

DonatJR Aug 5, 2018

SailReal Aug 5, 2018

DonatJR Aug 5, 2018

SailReal Aug 5, 2018

DonatJR Aug 5, 2018

SailReal Aug 5, 2018

SailReal commented Aug 5, 2018

SailReal commented Aug 5, 2018

bencikpeter commented Aug 6, 2018

DonatJR commented Aug 6, 2018

Allow Configurator on secure desktop only (release mode) #95

Allow Configurator on secure desktop only (release mode) #95

Conversation

DonatJR commented Aug 4, 2018 • edited Loading

SailReal Aug 4, 2018 • edited Loading

Choose a reason for hiding this comment

DonatJR Aug 4, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SailReal commented Aug 5, 2018

SailReal left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

DonatJR commented Aug 5, 2018 • edited Loading

DonatJR left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SailReal commented Aug 5, 2018

SailReal commented Aug 5, 2018

bencikpeter commented Aug 6, 2018

DonatJR commented Aug 6, 2018

DonatJR commented Aug 4, 2018 •

edited

Loading

SailReal Aug 4, 2018 •

edited

Loading

DonatJR Aug 4, 2018 •

edited

Loading

DonatJR commented Aug 5, 2018 •

edited

Loading

DonatJR left a comment •

edited

Loading