iotagent-node-lib icon indicating copy to clipboard operation
iotagent-node-lib copied to clipboard

[WIP] northBound server in HTTPS and Verify ContextBroker Certificate

Open m4n3dw0lf opened this issue 6 years ago • 12 comments

Described here: https://github.com/m4n3dw0lf/SecureFiware Not shure if need some adjustments or something to merge in the master branch, any feedback will be appreciated.

m4n3dw0lf avatar Apr 03 '18 03:04 m4n3dw0lf

Instead of defining a protocol filed, I'd suggest to use the approach already implemented in PR https://github.com/telefonicaid/iotagent-node-lib/pull/584 based in URL/HOST/POST variables, eg:

    if (process.env.IOTA_CB_URL) {
        config.contextBroker.url = process.env.IOTA_CB_URL;
    } else if (process.env.IOTA_CB_HOST) {
        config.contextBroker.url = "http://" + process.env.IOTA_CB_HOST;
        if (process.env.IOTA_CB_PORT) {
            config.contextBroker.url += ":" + process.env.IOTA_CB_PORT;
        }

What do you think @chicco785 ?

fgalan avatar Apr 03 '18 16:04 fgalan

i agree that for the https support in url @m4n3dw0lf can leverage on #584

still, he wants to be able to specify certificates to be used, and for that i didn't add support

chicco785 avatar Apr 03 '18 16:04 chicco785

basically i recommend him to rebase code using my PR and add the TLS verification support on top.

chicco785 avatar Apr 03 '18 16:04 chicco785

@chicco785 your PR (#584) was already merge into master, so I understand that rebase will be done with master branch, isn't it?

fgalan avatar Apr 03 '18 17:04 fgalan

sure :) i didn't realise it was already merged

On Tue, Apr 3, 2018 at 7:22 PM, Fermín Galán Márquez < [email protected]> wrote:

@chicco785 https://github.com/chicco785 your PR (#584 https://github.com/telefonicaid/iotagent-node-lib/pull/584) was already merge into master, so I understand that rebase will be done with master branch, isn't it?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/telefonicaid/iotagent-node-lib/pull/601#issuecomment-378329378, or mute the thread https://github.com/notifications/unsubscribe-auth/AAvcAkUVuILOHjYL11KhvuQk2-cz3z4iks5tk6_BgaJpZM4TEY2a .

chicco785 avatar Apr 03 '18 18:04 chicco785

@fgalan and @chicco785

I will checkout with #584 as soon as possible and then apply the verify certificate parameter, also will fix those checks that failed in the CI, about the HTTPS support for the northBound.js express server, any comments?

m4n3dw0lf avatar Apr 03 '18 21:04 m4n3dw0lf

@m4n3dw0lf i am not a TLS expert, my only comments are:

  • what happens when the context broker specified for a group of devices is not the same one in the main configuration?
  • it would be good to add mocha tests for this feature

chicco785 avatar Apr 04 '18 07:04 chicco785

@m4n3dw0lf not sure the scope (and duration) of your project. it may be interesting to use a certificate repository to retrieve them, so to decouple the management of certificates from their usage. for another project, i am looking into https://www.vaultproject.io

chicco785 avatar Apr 04 '18 13:04 chicco785

We are considering merging PR https://github.com/telefonicaid/iotagent-node-lib/issues/831 and this PR may be impacted (see more details here). However, any potential conflict is very easy to solve (I understand @jason-fox may provide details, if needed).

Is this PR still active? @m4n3dw0lf @chicco785 do you agree in merging PR #831 before your PR?

fgalan avatar Oct 07 '20 14:10 fgalan

@fgalan i was just commenting on it :) i have no clue about what's the state of this pr

chicco785 avatar Oct 07 '20 15:10 chicco785

@fgalan i was just commenting on it :) i have no clue about what's the state of this pr

Sorry, I thought you were involved somehow ;)

Thus, @m4n3dw0lf could you provide some feedback, please?

fgalan avatar Oct 07 '20 21:10 fgalan

We finally merged PR https://github.com/telefonicaid/iotagent-node-lib/pull/831 in order not to block in absence of answer from @m4n3dw0lf

The PR would need some adaptation to solve the conflicts. Basically:

  1. Solve the conflict accepting your changes
  2. Change style with npm run prettier

(Please @jason-fox amend mi if I'm wrong)

fgalan avatar Nov 19 '20 12:11 fgalan