Opened 2 years ago

Closed 22 months ago

#2355 closed defect (done)

Fix problem with Hive slave sandboxing

Reported by: ascheibe Owned by: ascheibe
Priority: medium Milestone: HeuristicLab 3.3.13
Component: Hive.Slave Version: 3.3.11
Keywords: Cc:

Description (last modified by ascheibe)

mkommend discovered that constant optimization + the sine symbol + the Hive slave sandboxing produces runs with wrong results. This should be investigated. Either

  • fix the problem
  • remove sandboxing functionality because
    • it is probably not secure anyway
    • the slave is run as network service which already has minimum rights (no admin rights)

Attachments (2)

OSGA CoOp 2 + Trig Hive.hl (244.1 KB) - added by ascheibe 2 years ago.
OSGA CoOp 2 + Trig.hl (189.3 KB) - added by ascheibe 2 years ago.

Download all attachments as: .zip

Change History (19)

comment:1 Changed 2 years ago by ascheibe

  • Description modified (diff)
  • Status changed from new to accepted

Changed 2 years ago by ascheibe

Changed 2 years ago by ascheibe

comment:2 Changed 2 years ago by ascheibe

Added files for testing the bug.

Last edited 2 years ago by ascheibe (previous) (diff)

comment:3 Changed 2 years ago by ascheibe

  • Milestone changed from HeuristicLab 3.3.12 to HeuristicLab 3.3.13

comment:4 Changed 23 months ago by ascheibe

  • Owner changed from ascheibe to jkarder
  • Status changed from accepted to assigned

comment:5 Changed 22 months ago by jkarder

  • Status changed from assigned to accepted

comment:6 Changed 22 months ago by jkarder

r12926:

  • changed sandboxing to always use an unrestricted permission set
  • removed IsAllowedPrivileged role and according IsPrivileged code

comment:7 Changed 22 months ago by jkarder

  • Owner changed from jkarder to ascheibe
  • Status changed from accepted to reviewing

comment:8 Changed 22 months ago by gkronber

Just a quick question: does this mean that "sandboxing" is effectively deactivated?

comment:9 Changed 22 months ago by ascheibe

Yes we removed sandboxing. There are a few reasons for this:

  • It never really was a sandbox because we had to activate certain permissions (like e.g. SecurityPermissionFlag.UnmanagedCode) so that HL could even run. E.g. you could write a native dll that formats the hard drive or similar stuff.
  • It sometimes led to Hive slave crashes when people forgot to check the "IsPrivileged" checkbox in the job manager. You could essentially wipe out all the slaves with a single job which would then require me going in the labs and manually restart ~100 computers. That does not make me happy.
  • Getting wrong results as described by mkommend which is probably even worse then dying slaves. jkarder tried to find out why this is happening but wasn't able to track this issue despite quite a bit of testing.
  • The slave service is executed as user NetworkService. This user has just normal privileges as every normal Windows user would have. Therefore it is not possible to harm the computer.

Because of this we decided to remove this feature as it causes a lot of problems and does not in any way provide security.

comment:10 Changed 22 months ago by gkronber

Thanks.

This was not meant as a critique, of course it is important that all of the research group members are happy! ;)

I'm only asking because I have the task to clean up the plugin infrastructure and if we don't need the separate AppDomain anymore then this can also be removed now. Is the separate AppDomain still necessary/useful in your point of view?

comment:11 Changed 22 months ago by ascheibe

Thanks :) AppDomains are still necessary. Each task in Hive has its own set of assemblies that the slave needs to load before executing a task and unload when the task is finished. Because you cannot unload single assemblies but only AppDomains with all loaded assemblies we still need them.

comment:12 Changed 22 months ago by ascheibe

r12927 fixed docu comment

comment:13 Changed 22 months ago by ascheibe

r12930 adapted MetaOpt branch to changes from r12926

comment:14 Changed 22 months ago by ascheibe

r12959 added backwards compatibility to task in the service

comment:15 Changed 22 months ago by ascheibe

  • Status changed from reviewing to readytorelease

Reviewed and tested, looks good.

comment:16 Changed 22 months ago by ascheibe

Merge after #2388

comment:17 Changed 22 months ago by ascheibe

  • Resolution set to done
  • Status changed from readytorelease to closed

r12963 merged r12926, r12927 and r12959 into stable

Note: See TracTickets for help on using tickets.