feat: Secure MQTT Bus Credentials #4142

joshua-silverio · 2022-09-01T18:26:02Z

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/main/.github/Contributing.md

PR Checklist

Please check if your PR fulfills the following requirements:

I am not introducing a breaking change (if you are, flag in conventional commit message with BREAKING CHANGE: describing the break)
I am not introducing a new dependency (add notes below if you are)
I have added unit tests for the new feature or bug fix (if not, why?)
I have fully tested (add details below) this the new feature or bug fix (if not, why?)
I have opened a PR for the related docs change (if not, why?)

Testing Instructions

Use compose-builder on https://github.com/joshua-silverio/edgex-compose/tree/secure-mqtt-bus to
"make gen dev mqtt-bus
docker-compose up to view logs of mqtt broker or device/app service
(should see mqtt broker connection w/ authmode:usernamepassword secretname: mqtt-bus"

New Dependency Instructions (If applicable)

internal/security/bootstrapper/messagebus/handlers/handlers.go

internal/security/secretstore/init.go

CLOSES: edgexfoundry#3934 Signed-off-by: Jim Wang (Intel) <yutsung.jim.wang@intel.com> Signed-off-by: intel <joshua.silverio@intel.com>

bnevis-i · 2022-09-01T19:15:36Z

Unit test failed (run make test)


[2022-09-01T18:29:25.963Z] ?   	github.com/edgexfoundry/edgex-go/internal/security/bootstrapper/messagebus/container	[no test files]
[2022-09-01T18:29:25.963Z] --- FAIL: TestHandler_SetupPasswordFile (0.00s)
[2022-09-01T18:29:25.963Z]     --- FAIL: TestHandler_SetupPasswordFile/SetupPasswordFile_ok (0.00s)
[2022-09-01T18:29:25.963Z]         handlers_test.go:277: Handler.GetCredentials() = false, want true
[2022-09-01T18:29:25.963Z] FAIL
[2022-09-01T18:29:25.963Z] coverage: 88.2% of statements

joshua-silverio · 2022-09-01T19:18:51Z

Unit test failed (run make test)


[2022-09-01T18:29:25.963Z] ?   	github.com/edgexfoundry/edgex-go/internal/security/bootstrapper/messagebus/container	[no test files]
[2022-09-01T18:29:25.963Z] --- FAIL: TestHandler_SetupPasswordFile (0.00s)
[2022-09-01T18:29:25.963Z]     --- FAIL: TestHandler_SetupPasswordFile/SetupPasswordFile_ok (0.00s)
[2022-09-01T18:29:25.963Z]         handlers_test.go:277: Handler.GetCredentials() = false, want true
[2022-09-01T18:29:25.963Z] FAIL
[2022-09-01T18:29:25.963Z] coverage: 88.2% of statements

Changed unit test since mosquitto_password binary was available on dev system but not build server...currently being rebuilt

cmd/security-file-token-provider/res/token-config.json

internal/security/bootstrapper/messagebus/handlers/handlers.go

Signed-off-by: intel <joshua.silverio@intel.com>

…edback Signed-off-by: intel <joshua.silverio@intel.com>

cmd/security-bootstrapper/entrypoint-scripts/messagebus_wait_install.sh

cmd/security-bootstrapper/res-bootstrap-messagebus/configuration.toml

internal/security/bootstrapper/messagebus/handlers/handlers.go

internal/security/secretstore/init.go

…mment feedback Signed-off-by: intel <joshua.silverio@intel.com>

Signed-off-by: intel <joshua.silverio@intel.com>

bnevis-i

All my comments were addressed. I have no further feedback.

lenny-goodell

Looking good!

Just need to make it more flexible for future support of NATS or other MQTT brokers. I don't expect the support for future to be perfect, but close so minimal changes are needed.

cmd/security-bootstrapper/res-bootstrap-messagebus/configuration.toml

internal/security/bootstrapper/messagebus_factory.go

internal/security/bootstrapper/mosquitto/README.md

internal/security/bootstrapper/mosquitto/config/config.go

internal/security/bootstrapper/mosquitto/handlers/handlers_test.go

internal/security/secretstore/init.go

…ment feedback Signed-off-by: intel <joshua.silverio@intel.com>

…sing feedback Signed-off-by: intel <joshua.silverio@intel.com>

Signed-off-by: intel <joshua.silverio@intel.com>

…back Signed-off-by: intel <joshua.silverio@intel.com>

Signed-off-by: intel <joshua.silverio@intel.com>

internal/security/secretstore/init.go

@@ -53,11 +53,13 @@
 const (
 	addKnownSecretsEnv   = "ADD_KNOWN_SECRETS"
 	redisSecretName      = "redisdb"
+	messagebusSecretName = "message-bus"


internal/security/secretstore/init.go

+	switch messageBusType {
+	case redisSecureMessageBusType:
+		creds = redisCredentials
+		secretName = redisSecretName


internal/security/secretstore/init.go

lenny-goodell

Just a few minor log messages tweaks.

internal/security/secretstore/init.go

Signed-off-by: intel <joshua.silverio@intel.com>

lenny-goodell

LGTM, Great Job!

internal/security/secretstore/init.go

+		secretName = redisSecretName
+	case mqttSecureMessageBusType:
+		creds = msgBusCredentials
+		secretName = messagebusSecretName


Signed-off-by: intel <joshua.silverio@intel.com>

sonarcloud · 2022-09-07T20:59:18Z

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities
0 Security Hotspots
1 Code Smell

No Coverage information
0.0% Duplication

lenny-goodell

LGTM

joshua-silverio added enhancement New feature or request security-services 2-medium priority denoting issues with cross-cutting project impact labels Sep 1, 2022

joshua-silverio requested review from farshidtz, bnevis-i, jim-wang-intel and lenny-goodell September 1, 2022 18:26

joshua-silverio assigned jim-wang-intel and joshua-silverio Sep 1, 2022

github-advanced-security bot found potential problems Sep 1, 2022

View reviewed changes

internal/security/bootstrapper/messagebus/handlers/handlers.go Fixed Show fixed Hide fixed

internal/security/bootstrapper/messagebus/handlers/handlers.go Fixed Show fixed Hide fixed

internal/security/secretstore/init.go Fixed Show fixed Hide fixed

jim-wang-intel added this to QA/Code Review in Security WG Sep 1, 2022

jim-wang-intel linked an issue Sep 1, 2022 that may be closed by this pull request

Generate security credential for Mosquitto broker #3934

Closed

feat: Add implementation for securing mqtt bus with username password

b3312ac

CLOSES: edgexfoundry#3934 Signed-off-by: Jim Wang (Intel) <yutsung.jim.wang@intel.com> Signed-off-by: intel <joshua.silverio@intel.com>

joshua-silverio force-pushed the mqtt-bus-cred-store branch from badc4ca to b3312ac Compare September 1, 2022 19:09

bnevis-i reviewed Sep 1, 2022

View reviewed changes

cmd/security-file-token-provider/res/token-config.json Outdated Show resolved Hide resolved

bnevis-i reviewed Sep 1, 2022

View reviewed changes

internal/security/bootstrapper/messagebus/handlers/handlers.go Outdated Show resolved Hide resolved

bnevis-i reviewed Sep 1, 2022

View reviewed changes

internal/security/bootstrapper/messagebus/handlers/handlers.go Outdated Show resolved Hide resolved

jim-wang-intel requested a review from cloudxxx8 September 1, 2022 21:42

joshua-silverio added 2 commits September 1, 2022 15:02

fix: Remove Custom Policy addressing comment feedback

0448d23

Signed-off-by: intel <joshua.silverio@intel.com>

fix: Refactor get credentials using pointer obj addressing comment fe…

8c3620d

…edback Signed-off-by: intel <joshua.silverio@intel.com>

lenny-goodell requested changes Sep 1, 2022

View reviewed changes

joshua-silverio added 4 commits September 1, 2022 16:54

fix: Use Local MessageQueueInfo struct for bootstrapper to address co…

2204304

…mment feedback Signed-off-by: intel <joshua.silverio@intel.com>

fix: Make message bus configure more generic addressing comment feedback

4e62cb0

Signed-off-by: intel <joshua.silverio@intel.com>

fix: check if type is mqtt before setting up secrets

b43f507

Signed-off-by: intel <joshua.silverio@intel.com>

docs: Add README for how to add a new broker type

f1438ce

Signed-off-by: intel <joshua.silverio@intel.com>

joshua-silverio requested review from bnevis-i and lenny-goodell September 2, 2022 16:54

fix: Remove msg queue type code

5d60433

Signed-off-by: intel <joshua.silverio@intel.com>

bnevis-i previously approved these changes Sep 2, 2022

View reviewed changes

lenny-goodell requested changes Sep 6, 2022

View reviewed changes

joshua-silverio added 6 commits September 6, 2022 13:19

fix: refactor messagequeueinfo to securemessagebusinfo addressing com…

4488a23

…ment feedback Signed-off-by: intel <joshua.silverio@intel.com>

fix: Refactor ConfigureMessageBus to ConfigureSecureMessageBus addres…

dd6fb8f

…sing feedback Signed-off-by: intel <joshua.silverio@intel.com>

docs: move readme to bootstrapper from mosquitto folder

c250bfd

Signed-off-by: intel <joshua.silverio@intel.com>

fix: Refactor configure.go to use const for Mosquitto broker

98b5421

Signed-off-by: intel <joshua.silverio@intel.com>

fix: refactor mosquitto handlers.go to be more mosquitto specific

df7707b

Signed-off-by: intel <joshua.silverio@intel.com>

fix: Removed args from tt in mosquitto handler_test.go adressing feed…

1563001

…back Signed-off-by: intel <joshua.silverio@intel.com>

joshua-silverio dismissed bnevis-i’s stale review via 1563001 September 6, 2022 21:08

fix: refactor secretstore setup to be more generic addressing feedback

1739131

Signed-off-by: intel <joshua.silverio@intel.com>

github-advanced-security bot found potential problems Sep 7, 2022

View reviewed changes

lenny-goodell requested changes Sep 7, 2022

View reviewed changes

fix: minor log message changes addressing feedback

ad8244c

Signed-off-by: intel <joshua.silverio@intel.com>

joshua-silverio requested a review from lenny-goodell September 7, 2022 20:41

lenny-goodell previously approved these changes Sep 7, 2022

View reviewed changes

github-advanced-security bot found potential problems Sep 7, 2022

View reviewed changes

internal/security/secretstore/init.go

secretName = redisSecretName

case mqttSecureMessageBusType:

creds = msgBusCredentials

secretName = messagebusSecretName

Check failure

Code scanning / CodeQL

Hard-coded credentials

Hard-coded [secret](1).

joshua-silverio requested a review from bnevis-i September 7, 2022 20:47

fix: refactor messagebus install script to be more generic

25887df

Signed-off-by: intel <joshua.silverio@intel.com>

joshua-silverio dismissed lenny-goodell’s stale review via 25887df September 7, 2022 20:58

joshua-silverio requested a review from lenny-goodell September 7, 2022 20:59

lenny-goodell approved these changes Sep 7, 2022

View reviewed changes

jim-wang-intel approved these changes Sep 7, 2022

View reviewed changes

joshua-silverio merged commit 21503d9 into edgexfoundry:main Sep 8, 2022

Security WG automation moved this from QA/Code Review to Done Sep 8, 2022

joshua-silverio deleted the mqtt-bus-cred-store branch September 8, 2022 16:00

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Secure MQTT Bus Credentials #4142

feat: Secure MQTT Bus Credentials #4142

joshua-silverio commented Sep 1, 2022

bnevis-i commented Sep 1, 2022

joshua-silverio commented Sep 1, 2022

bnevis-i left a comment

lenny-goodell left a comment

lenny-goodell left a comment

lenny-goodell left a comment

sonarcloud bot commented Sep 7, 2022

lenny-goodell left a comment

feat: Secure MQTT Bus Credentials #4142

feat: Secure MQTT Bus Credentials #4142

Conversation

joshua-silverio commented Sep 1, 2022

PR Checklist

Testing Instructions

New Dependency Instructions (If applicable)

bnevis-i commented Sep 1, 2022

joshua-silverio commented Sep 1, 2022

bnevis-i left a comment

Choose a reason for hiding this comment

lenny-goodell left a comment

Choose a reason for hiding this comment

lenny-goodell left a comment

Choose a reason for hiding this comment

lenny-goodell left a comment

Choose a reason for hiding this comment

sonarcloud bot commented Sep 7, 2022

lenny-goodell left a comment

Choose a reason for hiding this comment