Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Plugin cannot properly drop privilege twice, causing fatal error #77

Open
kfeinauer opened this issue Apr 16, 2024 · 3 comments
Open

Plugin cannot properly drop privilege twice, causing fatal error #77

kfeinauer opened this issue Apr 16, 2024 · 3 comments
Assignees
Labels
bug Something isn't working launcher Related to the launcher

Comments

@kfeinauer
Copy link
Contributor

Currently, the SageMaker plugin SDK cannot properly start without a code patch to the SDK. The following is the error they get:

2022-06-28T16:29:02.737432Z [rstudio-sagemaker-launcher] INFO Received signal: 2
2022-06-28T16:29:02.737790Z [rstudio-sagemaker-launcher] INFO Stopping plugin...
2022-06-28T16:29:56.703204Z [rstudio-sagemaker-launcher] ERROR SystemError error 1 (Operation not permitted) [subcategory: system]; OCCURRED AT temporarilyDropPrivileges /local/home/jedambr/RStudioUpdate2022/src/Rstudio-launcher-plugin-sdk/third-party-src/sdk/src/system/PosixSystem.cpp:262
2022-06-28T16:29:56.703568Z [rstudio-sagemaker-launcher] ERROR Could not lower privilege to server user: sagemaker-user.

Possible causes:

  1. Early during startup, we check the scratch path and drop privileges from root -> server user. This also was how things worked before the new SDK release:
    // Drop privileges to the server user.
    error = system::posix::temporarilyDropPrivileges(in_serverUser, {});
    CHECK_ERROR(error, "Could not lower privilege to server user: " + in_serverUser.getUsername() + ".")
  2. In the new version, in addition, we need to setup the file log destination with root privileges to ensure we can initially create the file under /var/log. We restore root
    // If we are, restore root privileges.
    if (!options.useUnprivilegedMode() && system::posix::realUserIsRoot())
    {
    error = system::posix::restoreRoot();
    CHECK_ERROR(error, "Could not restore root privilege.")
    }
  3. After we're done with setting up the log file, we again lower root privilege in the new version:
    // Drop privileges to the server user.
    if (system::posix::realUserIsRoot())
    {
    const Optional<system::GidType> in_group;
    error = system::posix::temporarilyDropPrivileges(serverUser, in_group);
    CHECK_ERROR(error, "Could not lower privilege to server user: " + serverUser.getUsername() + ".")
    }

From the SageMaker logs, it looks like maybe the call to restoreRoot is not allowing them to again call temporarilyDropPrivileges again. In the previous version, there was one restoreRoot() -> temporarilyDropPrivileges() call. In the new version, there are two.

SageMaker is working around the issue by commenting out the following code in AbstractMain.cpp:

// Drop privileges to the server user.
if (system::posix::realUserIsRoot())
{
const Optional<system::GidType> in_group;
error = system::posix::temporarilyDropPrivileges(serverUser, in_group);
CHECK_ERROR(error, "Could not lower privilege to server user: " + serverUser.getUsername() + ".")
}

@kfeinauer kfeinauer added bug Something isn't working needs refinement Marked for review at a future meeting labels Apr 16, 2024
@nahara7 nahara7 self-assigned this Jul 3, 2024
@kfeinauer kfeinauer removed the needs refinement Marked for review at a future meeting label Jul 10, 2024
@nahara7
Copy link
Contributor

nahara7 commented Jul 12, 2024

So far I haven't reproduced this locally. I've tried a few configurations of user ownership to see if it would arise, but everything is working well. I did notice that the binary lives in a different user directory than the server user

ERROR SystemError error 1 (Operation not permitted) [subcategory: system]; OCCURRED AT temporarilyDropPrivileges /local/home/jedambr/RStudioUpdate2022/src/Rstudio-launcher-plugin-sdk/third-party-src/sdk/src/system/PosixSystem.cpp:262

Where sagemaker is the server user.

If that directory has group permissions for the server-user all looks well.

I can still go ahead and remove the second restoreRoot call. But to really see why this happening, we should see the user configurations and permissions of the sagemaker environment.

@nahara7
Copy link
Contributor

nahara7 commented Jul 15, 2024

Moving to backlog since this has not been reproduced across the team

@kfeinauer
Copy link
Contributor Author

AWS is silent on this and we cannot reproduce so we'll re-address this when it comes back up.

@bschwedler bschwedler added the launcher Related to the launcher label Nov 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working launcher Related to the launcher
Projects
None yet
Development

No branches or pull requests

3 participants