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

Make chown path in security-secretstore-setup startup.sh parameterizable. #4570

Closed
rk94655 opened this issue May 17, 2023 · 8 comments · Fixed by #4592
Closed

Make chown path in security-secretstore-setup startup.sh parameterizable. #4570

rk94655 opened this issue May 17, 2023 · 8 comments · Fixed by #4592
Assignees
Labels
enhancement New feature or request
Milestone

Comments

@rk94655
Copy link

rk94655 commented May 17, 2023

🐞 Bug Report

Affected Services [REQUIRED]

The issue is located in: entrypoint.sh file of security-secretstore-setup

Is this a regression? no

no, the issue is from Jakarta version itself No

Description and Minimal Reproduction [REQUIRED]

entrypoint.sh file of security-secretstore-setup change the ownership of token directory. Attaching here the code snippet from entrypoint.sh file in <> section
<* /tmp/edgex/secrets need to be shared with all other services that need secrets and
*thus change the ownership to EDGEX_USER:EDGEX_GROUP
echo "$(date) Changing ownership of secrets to ${EDGEX_USER}:${EDGEX_GROUP}"
chown -Rh ${EDGEX_USER}:${EDGEX_GROUP} /tmp/edgex/secrets>

I have changed the location of tokens from both host as well as inside containers of all services. But, the issue is secretstore-setup is not changing the ownership of tokens path to "EDGEX_USER" which is 2001. On debugging, it came out that, entrypoint.sh file has hard-coded location of tokens directory. As a result other services like core-metadata is not able to read token from new path due to permission issues

Logs of core-metadata
level=INFO ts=2023-05-17T07:44:32.967662669Z app=core-metadata source=secret.go:59 msg="Reading secret store configuration and authentication token"
level=WARN ts=2023-05-17T07:44:32.967816071Z app=core-metadata source=secret.go:96 msg="Retryable failure while creating SecretClient: open /usr/local/share/secrets/core-metadata/secrets-token.json: permission denied"

🔥 Exception or Error





🌍 Your Environment

Deployment Environment: Oracle Linux 9

EdgeX Version [REQUIRED]: Jakarta Version

Anything else relevant?
Reference https://github.com/edgexfoundry/edgex-go/blob/main/cmd/security-secretstore-setup/entrypoint.sh

@rk94655 rk94655 added the bug Something isn't working label May 17, 2023
@bnevis-i
Copy link
Collaborator

bnevis-i commented May 17, 2023

Although it is true that references to /tmp/edgex/secrets are parameterizable almost everywhere else except here, I also want to point out that entrypoint.sh is intended to be run in a Docker context. Consequently, /tmp/edgex/secrets is entirely virtualized inside of the container and can be mapped to any host path or docker volume of the user's choice -- the location of these files inside the container and the actual location outside of the container are completely independent.

In other words, is changing this line
https://github.com/edgexfoundry/edgex-compose/blob/jakarta/compose-builder/add-security.yml#L94
in your copy of edgex-compose sufficient to resolve this issue?

@bnevis-i
Copy link
Collaborator

Additionally, I would like to comment on the choice of the relocated path: /usr/local/share/secrets/

The reason /tmp/edgex/secrets was chosen is because /tmp is cleared across reboots in many Linux distros (/run was preferrable but it wasn't supported in Docker for Mac by default). If the intent is to make this data persistent across reboots, why not map /usr/local/share/secrets/ or the original /tmp/edgex/secrets to a plain docker volume?

@bnevis-i
Copy link
Collaborator

@rk94655 did the workaround of changing the container mount path take care of the issue?

@rk94655
Copy link
Author

rk94655 commented May 26, 2023

Hi @bnevis-i
Yes, we thought of changing bind mounts to volumes, so that we don't have dependencies on directory path.
For now, I have changed the token path in entrypoint.sh of secretstore-setup manually and re-built the image, which solved my problem.

The reason for keeping tokens out of /tmp folder is security concerns of the organization. Hence, the tokens are stored in /usr/local/share/secrets inside the container and it is binded with path ~/.local/share/secrets on the host.

In future, we will try the volume part, but it would be better if we make it token path configurable in entrypoint.sh also.

@rk94655 rk94655 closed this as completed May 26, 2023
@bnevis-i
Copy link
Collaborator

@rk94655 I will reopen this then and put the request in the backlog.

@bnevis-i bnevis-i reopened this May 26, 2023
@bnevis-i bnevis-i added enhancement New feature or request and removed bug Something isn't working labels May 26, 2023
@bnevis-i bnevis-i changed the title Security-SecretStore-Setup service :: Unable to change permission of token directory to EDGEX_USER Make chown path in security-secretstore-setup startup.sh parameterizable. May 26, 2023
@bnevis-i bnevis-i added this to the Napa milestone Jun 7, 2023
@bnevis-i
Copy link
Collaborator

bnevis-i commented Jun 21, 2023

@rk94655 I would like your feedback on the attached pull request.

@rk94655
Copy link
Author

rk94655 commented Jun 21, 2023

Yes this is right now, that we are taking Root of token directory from environment variables. In this way, token path for edgex services inside containers is also configurable. Thanks for the PR @bnevis-i. But, this value can be read from TokenFileProvider.OutputDir of file cmd/security-secretstore-setup/res-file-token-provider/configuration.toml. So, we should remove defining a new variable

@bnevis-i
Copy link
Collaborator

But, this value can be read from TokenFileProvider.OutputDir of file cmd/security-secretstore-setup/res-file-token-provider/configuration.toml. So, we should remove defining a new variable

To do this we'd have to parse https://github.com/edgexfoundry/edgex-go/blob/main/cmd/security-secretstore-setup/res-file-token-provider/configuration.yaml#L12 from shell script (possible, now that it is YAML) and also be able to check TOKENFILEPROVIDER_OUTPUTDIR for an override.

@lenny-intel Comments?

bnevis-i added a commit that referenced this issue Jul 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants