Azure / azure-iot-sdk-node

A Node.js SDK for connecting devices to Microsoft Azure IoT services
https://docs.microsoft.com/en-us/azure/iot-hub/
Other
261 stars 227 forks source link

azure-iot-security-tpm doesn't allow for more than one TpmSecurityClient when using TPM simulator #480

Closed ghost closed 5 years ago

ghost commented 5 years ago

Context

Description of the issue:

When two TpmSecurityClients are instantiated in the same application, the TPM simulator will no longer function - instead, only a single instance of TpmSecurityClient used in this case with the simulator seems to be required.

If this is the case, it's rather confusing. When trying to build an application with multiple files, it's common to require the package and instantiate the security client on a per-class/per-module basis - this is where I originally ran into this issue, where two classes both created their own instances of TpmSecurityClient to do work, which fails.

If you can only have one simulated TpmSecurityClient, then it might make more sense to handle this with a Singleton under the hood - at least for me, seeing the var foo = new bar(); pattern used in the quickstarts shows that I can create quite a few of these things and they shouldn't conflict with one another and I can have locally scoped TpmSecurityClients instead.

Code sample exhibiting the issue:

A failing testcase, note the locally scoped new ... TpmSecurityClient at L20, L46: https://gist.github.com/casabon/8ebcf905a07a53f28b3320d8275d24af

A passing testcase, note const ... new TpmSecurityClient at L9: https://gist.github.com/casabon/73d6f25fdb4168f27f42d031eceb4c29

anthonyvercolano commented 5 years ago

I'm going to have to page all the stuff in having to do with tpm. I have a fairly in-efficient memory manager.

Let me get back to you tomorrow.

anthonyvercolano commented 5 years ago

Looking at the link you sent. The first parameter of the TpmSecurityClient constructor is always undefined. Where am I missing the invocation with it set otherwise?

In addition, great care should be taken. If you are trying to call both of your functions, you could easily end up both trying to invoke commands on the tpm at the same time. This could be bad.

ghost commented 5 years ago

Ah, I seem to have mixed up two of the methods when implementing this. The core issue might be slightly different but I believe there is still an issue; I'm going to try to make a reduced testcase and get back to you.

ghost commented 5 years ago

Edited the primary bug with better test cases and a description. Sorry for the lack of clarity.

anthonyvercolano commented 5 years ago

It is not a permitted use of the tssJs.tpm(bool) to call it twice. The first is opening a socket and the second will just wait until the first one goes away.

ghost commented 5 years ago

Closing as by design.

az-iot-builder-01 commented 5 years ago

@casabon, thank you for your contribution to our open-sourced project! Please help us improve by filling out this 2-minute customer satisfaction survey

pierreca commented 5 years ago

paging @amarochk for issues with tss.js and the TPM simulator.