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

feat(security): Implementation for adding Consul ACL policies, roles #3273

Merged

Conversation

jim-wang-intel
Copy link
Contributor

@jim-wang-intel jim-wang-intel commented Mar 18, 2021

New addition for implementing for Consul's ACL policies creation and roles for Consul tokens generated later on via go-mod-secret

  • Add logic to check whether the ACL policy is already per-existing before creation of new policy
  • Add implementation to create a new ACL policy
  • Add implementation to create a role for EdgeX's services via Vault's /consul/roles/* APIs: this sets the stage for creating role-based Consul tokens used by EdgeX services
  • Add logic for creating token roles based on EdgeX service keys from configuration file
  • Update token-file-provider on Edgex's default policy to add the permission for calling /consul/creds/"service-key" endpoint

Closes: #3254, #3160

Signed-off-by: Jim Wang [email protected]

PR Checklist

Please check if your PR fulfills the following requirements:

  • [x ] Tests for the changes have been added (for bug fixes / features)
  • Docs have been added / updated (for bug fixes / features)

If your build fails due to your commit message not passing the build checks, please review the guidelines here: https://github.com/edgexfoundry/edgex-go/blob/master/.github/Contributing.md.

What is the current behavior?

this is a new feature, no ACL policies and roles existing yet.

Issue Number: #3254

What is the new behavior?

Add Consul's ACL policies for EdgeX services and management token.
Add roles for Vault so that we can utilize the /consul/creds/rolename api to create Consul tokens for services later.
Add consul management token
Augment the Vault's ACL policy of /consul/creds/servicekey on file-token-provider

Does this PR introduce a breaking change?

  • Yes
  • [x ] No

New Imports

  • Yes
  • [x ] No

Specific Instructions

Are there any specific instructions or things that should be known prior to reviewing?
With this PR and run the docker compose, docker logs edgex-core-consul should give successful logging messages like the following:


level=INFO ts=2021-03-18T21:13:05.744452978Z app=edgex-security-bootstrapper source=command.go:486 msg="successfully retrieved secretstore management token from /tmp/edgex/secrets/edgex-consul/admin/token.json"
level=INFO ts=2021-03-18T21:13:05.744465707Z app=edgex-security-bootstrapper source=command.go:163 msg="successfully get secretstore token and configuring the registry access for secretestore"
level=INFO ts=2021-03-18T21:13:05.746086231Z app=edgex-security-bootstrapper source=command.go:538 msg="successfully configure Consul access for secretstore"
    2021-03-18T21:13:05.747Z [ERROR] agent.http: Request error: method=GET url=/v1/acl/policy/name/edgex-service-policy from=192.168.48.7:58514 error="ACL not found"
level=INFO ts=2021-03-18T21:13:05.758003985Z app=edgex-security-bootstrapper source=aclpolicies.go:150 msg="successfully created a new agent policy with name edgex-service-policy"
level=INFO ts=2021-03-18T21:13:05.759230928Z app=edgex-security-bootstrapper source=aclroles.go:127 msg="successfully create a role [edgex-security-proxy-setup] for secretstore"
level=INFO ts=2021-03-18T21:13:05.760299764Z app=edgex-security-bootstrapper source=aclroles.go:127 msg="successfully create a role [edgex-support-notifications] for secretstore"
level=INFO ts=2021-03-18T21:13:05.76133305Z app=edgex-security-bootstrapper source=aclroles.go:127 msg="successfully create a role [edgex-support-scheduler] for secretstore"
level=INFO ts=2021-03-18T21:13:05.762399343Z app=edgex-security-bootstrapper source=aclroles.go:127 msg="successfully create a role [edgex-application-service] for secretstore"
level=INFO ts=2021-03-18T21:13:05.763350181Z app=edgex-security-bootstrapper source=aclroles.go:127 msg="successfully create a role [edgex-core-command] for secretstore"
level=INFO ts=2021-03-18T21:13:05.764296028Z app=edgex-security-bootstrapper source=aclroles.go:127 msg="successfully create a role [edgex-core-data] for secretstore"
level=INFO ts=2021-03-18T21:13:05.765305796Z app=edgex-security-bootstrapper source=aclroles.go:127 msg="successfully create a role [edgex-core-metadata] for secretstore"
level=INFO ts=2021-03-18T21:13:05.768771663Z app=edgex-security-bootstrapper source=acltokens.go:166 msg="internal Consul ACL system is ready"
level=INFO ts=2021-03-18T21:13:05.779467636Z app=edgex-security-bootstrapper source=acltokens.go:423 msg="successfully created a new registry token"
level=INFO ts=2021-03-18T21:13:05.779711338Z app=edgex-security-bootstrapper source=acltokens.go:316 msg="successfully created a new agent token"
    2021-03-18T21:13:05.788Z [INFO]  agent: Updated agent's ACL token: token=agent
level=INFO ts=2021-03-18T21:13:05.789274373Z app=edgex-security-bootstrapper source=acltokens.go:368 msg="agent token is set with type [agent]"
level=INFO ts=2021-03-18T21:13:05.789381234Z app=edgex-security-bootstrapper source=command.go:345 msg="successfully set up agent token into agent"
    2021-03-18T21:13:05.790Z [ERROR] agent.http: Request error: method=GET url=/v1/acl/policy/name/edgex-management-policy from=192.168.48.7:58514 error="ACL not found"
level=INFO ts=2021-03-18T21:13:05.804597835Z app=edgex-security-bootstrapper source=aclpolicies.go:150 msg="successfully created a new agent policy with name edgex-management-policy"
level=INFO ts=2021-03-18T21:13:05.81879284Z app=edgex-security-bootstrapper source=acltokens.go:423 msg="successfully created a new registry token"
level=INFO ts=2021-03-18T21:13:05.819110185Z app=edgex-security-bootstrapper source=command.go:306 msg="token is written to /tmp/edgex/secrets/consul-acl-token/mgmt_token.json"
level=INFO ts=2021-03-18T21:13:05.819189022Z app=edgex-security-bootstrapper source=command.go:272 msg="successfully created and saved registry management token with policy name edgex-management-policy"
level=INFO ts=2021-03-18T21:13:05.819399123Z app=edgex-security-bootstrapper source=command.go:197 msg="setupRegistryACL successfully done"

Other information

Copy link
Collaborator

@bnevis-i bnevis-i left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review complete. Please double check HTTP response code processing in getPolicyByName

@jim-wang-intel
Copy link
Contributor Author

Review complete. Please double check HTTP response code processing in getPolicyByName

@bnevis-i i've double checked and it returns 403 on my local box. I've also removed the code to save management token as you indicate we use the bootstrap token for now.

bnevis-i
bnevis-i previously approved these changes Mar 19, 2021
lenny-goodell
lenny-goodell previously approved these changes Mar 22, 2021
Copy link
Member

@lenny-goodell lenny-goodell left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

New addition for implementing for Consul's ACL policies creation and roles for Consul tokens generated later on via go-mod-secret
 - Add logic to check whether the ACL policy is already per-existing before creation of new policy
 - Add implementation to create a new ACL policy
 - Add implementation to create a role for EdgeX's services via Vault's /consul/roles/* APIs: this sets the stage for creating role-based Consul tokens used by EdgeX services
 - Add logic for creating token roles based on EdgeX service keys from configuration file
 - Update token-file-provider on edgex's default policy to add the permission for calling /consul/creds/"service-key" endpoint

Closes: edgexfoundry#3254, edgexfoundry#3160

Signed-off-by: Jim Wang <[email protected]>
@jim-wang-intel jim-wang-intel dismissed stale reviews from lenny-goodell and bnevis-i via ade5e89 March 22, 2021 18:39
@jim-wang-intel jim-wang-intel force-pushed the create-policy-and-role branch from 3290b09 to ade5e89 Compare March 22, 2021 18:39
@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 1 Code Smell

No Coverage information No Coverage information
0.0% 0.0% Duplication

@jim-wang-intel
Copy link
Contributor Author

commits squashed and rebased before the merging

@jim-wang-intel jim-wang-intel changed the title feat(security): Implementation for adding Consul ACL policies, roles and management token feat(security): Implementation for adding Consul ACL policies, roles Mar 22, 2021
@codecov-io
Copy link

Codecov Report

Merging #3273 (ade5e89) into master (eeaee6b) will increase coverage by 0.26%.
The diff coverage is 69.41%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #3273      +/-   ##
==========================================
+ Coverage   42.14%   42.41%   +0.26%     
==========================================
  Files         181      183       +2     
  Lines       15598    15742     +144     
==========================================
+ Hits         6574     6677     +103     
- Misses       8614     8634      +20     
- Partials      410      431      +21     
Impacted Files Coverage Δ
.../security/bootstrapper/command/setupacl/command.go 66.82% <57.89%> (-2.09%) ⬇️
...ecurity/bootstrapper/command/setupacl/acltokens.go 72.27% <66.66%> (-0.46%) ⬇️
internal/security/fileprovider/provider.go 63.04% <66.66%> (+3.04%) ⬆️
...urity/bootstrapper/command/setupacl/aclpolicies.go 69.44% <69.44%> (ø)
...security/bootstrapper/command/setupacl/aclroles.go 77.77% <77.77%> (ø)
internal/security/fileprovider/defaults.go 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update eeaee6b...ade5e89. Read the comment docs.

@jim-wang-intel jim-wang-intel merged commit 8b8c045 into edgexfoundry:master Mar 22, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3-high priority denoting release-blocking issues enhancement New feature or request ireland security-services
Projects
None yet
4 participants