search

edgexfoundry / device-opc-ua

Apache License 2.0
28 stars 26 forks source link

Authentication Feature need to be Added #46

Open vadivelk2023 opened 3 months ago

vadivelk2023 commented 3 months ago

Username and Password based authentication need to be added.

jiekechoo commented 3 months ago

@vadivelk2023 Can you describe the details? Give us more information, please.

vadivelk2023 commented 3 months ago

@vadivelk2023 Can you describe the details? Give us more information, please.

The OPC UA Protocol supports Authentication mechanisms such as Username/Password, X509 Certificate, JWT token based authentication. For more details please visit https://reference.opcfoundation.org/Core/Part2/v105/docs/5.2.3

Please consider these auth mechanisms for OPC UA device service.

jiekechoo commented 3 months ago

OK, I see. The device service device-opc-ua has supported X509 Certificate, I'll try to add Username/Password, JWT token based authentication. If you can contribute, welcome. 😄

vadivelk2023 commented 3 months ago

Thanks for the Invite :) However, the library used in the device-opc-ua (https://github.com/gopcua/opcua) supports all these authentication mechanisms. The only thing that needs to be done is adding additional keys to get these values in config and adding two more functions to pass the credentials to the underlying Native gopcua library based on the auth mode mentioned in config.

I have another doubt and please consider this as a suggestion as well. The authentication information is mentioned in configuration.yml file. Whenever a new device is added to the device-opc-ua service its auth details need to be added to the configuration.yml file. As of now based on my understanding there is no API to change/update the content of the configuration.yml file (Example is App Service Configurable completely works based on the configuration.yml file - The only way to update the file content is rebuilding the image or mounting the file from local file system - This also requires deleting the metadata volume). My suggestion is why don't we get these details from the device file itself? If it is maintained in the device file it can be updated through Core metadata API.

Like below

Path:cmd/res/devices/Simple-Devices.yaml

deviceList:

jiekechoo commented 3 months ago

The suggestion as you mentioned in the second paragraph, it's a good idea. Maybe we'll refactor it in the next release v3.2 or later.

cloudxxx8 commented 2 months ago

The credentials should not be stored in core-metadata, but in the secret store. The core-metadata should only persist the secret name in the protocol properties, and the device service should implement the logic to access secret store. This feature could be implemented when we have any volunteer.

Ldsystem commented 2 months ago

@vadivelk2023 I have do something about what you've mention, you can find it here Ldsystem/device-opc-ua. I've made some updates as follow:

  1. introduce connection pool to manage ua connections.
  2. accept SecurityPolicy and SecurityMode options from ProtocolProperties, so that user can specify different security policy for each opc server. In this case, opc server's cert is also needed for asymetric encryption, I also take it from ProtocolProperties with a new field RemotePemCert.
  3. Username authentication support.

But I'm totally new to golang and I've made a massive change of the code, so it is difficult for me to bring it back to this repo. you can try it before the updates come to life.

vadivelk2023 commented 2 months ago

@cloudxxx8 I agree with you regarding credential management.

@jiekechoo The device-opc-ua has provision to add certificate and key in configuration.yml. validation also fine. But it is not properly passed or handled in getClient function in subscriptionlistener.go file. My suggestions to improve this is

  1. Adding additional keys under OPCUASERVER in configuration file such a way that OPCUAServer: DeviceName: SimulationServer Auth: None #Options - None / UserNamePassword / Certificate Policy: None Mode: None CertFile: '' KeyFile: '' UserName: '' PassWord: '' Writable: Resources: 'Counter,Random'

  2. Checking the Auth value in getClient. Based on Auth value adding options to opts[]. here is the sample //common opcua.SecurityPolicy(policy), opcua.SecurityModeString(mode), //Auth None opcua.AuthAnonymous(), opcua.SecurityFromEndpoint(ep, ua.UserTokenTypeAnonymous), //Auth Username Password opcua.AuthUsername(UserName,Password), opcua.SecurityFromEndpoint(ep, ua.UserTokenTypeUserName), //Auth Certificate opcua.CertificateFile(certFile), opcua.PrivateKeyFile(keyFile), opcua.SecurityFromEndpoint(ep, ua.UserTokenTypeCertificate),

The certificate paths were directly referring configuration.yml file content not secretstore. This need to be improved.

@Ldsystem I appreciate your effort. But in my experience I have seen connection pool for databases only not for OPCUA Servers. I am not sure it is required or not. please do a performance testing with and without connection pool. If there is some positive results we may consider connection pool as well.

jiekechoo commented 2 months ago

The credentials should not be stored in core-metadata, but in the secret store. The core-metadata should only persist the secret name in the protocol properties, and the device service should implement the logic to access secret store. This feature could be implemented when we have any volunteer.

I see.

Ldsystem commented 2 months ago

@cloudxxx8 I agree with you regarding credential management.

@jiekechoo The device-opc-ua has provision to add certificate and key in configuration.yml. validation also fine. But it is not properly passed or handled in getClient function in subscriptionlistener.go file. My suggestions to improve this is

  1. Adding additional keys under OPCUASERVER in configuration file such a way that OPCUAServer: DeviceName: SimulationServer Auth: None #Options - None / UserNamePassword / Certificate Policy: None Mode: None CertFile: '' KeyFile: '' UserName: '' PassWord: '' Writable: Resources: 'Counter,Random'
  2. Checking the Auth value in getClient. Based on Auth value adding options to opts[]. here is the sample //common opcua.SecurityPolicy(policy), opcua.SecurityModeString(mode), //Auth None opcua.AuthAnonymous(), opcua.SecurityFromEndpoint(ep, ua.UserTokenTypeAnonymous), //Auth Username Password opcua.AuthUsername(UserName,Password), opcua.SecurityFromEndpoint(ep, ua.UserTokenTypeUserName), //Auth Certificate opcua.CertificateFile(certFile), opcua.PrivateKeyFile(keyFile), opcua.SecurityFromEndpoint(ep, ua.UserTokenTypeCertificate),

The certificate paths were directly referring configuration.yml file content not secretstore. This need to be improved.

@Ldsystem I appreciate your effort. But in my experience I have seen connection pool for databases only not for OPCUA Servers. I am not sure it is required or not. please do a performance testing with and without connection pool. If there is some positive results we may consider connection pool as well.

@vadivelk2023 I did performance tests indeed, here are the results:

  1. Without reusable client:

    goos: darwin
goarch: arm64
pkg: github.com/edgexfoundry/device-opc-ua/internal/driver
Benchmark_HandleReadCommands_WithoutReuseClient
Listening on 0.0.0.0:48408
Benchmark_HandleReadCommands_WithoutReuseClient-8            638       1699040 ns/op
PASS

  2. With 1 reusable client, no encryption:

    goos: darwin
goarch: arm64
pkg: github.com/edgexfoundry/device-opc-ua/internal/driver
Benchmark_HandleReadCommands_ReuseClient
Listening on 0.0.0.0:48408
Benchmark_HandleReadCommands_ReuseClient-8          8966        128933 ns/op
PASS

  3. With 4 reusable clients, no encryption:

    goos: darwin
goarch: arm64
pkg: github.com/edgexfoundry/device-opc-ua/internal/driver
Benchmark_HandleReadCommands_ReuseClientAsync
Listening on 0.0.0.0:48408
Benchmark_HandleReadCommands_ReuseClientAsync-8        12667         92111 ns/op
PASS

  4. With 1 reusable client and Basic256Sha256 signing and encryption:

    goos: darwin
goarch: arm64
pkg: github.com/edgexfoundry/device-opc-ua/internal/driver
Benchmark_HandleReadCommands_ReuseClientWithEncryption
Listening on 0.0.0.0:48408
Benchmark_HandleReadCommands_ReuseClientWithEncryption-8        5190        216897 ns/op
PASS

  5. With 4 reusable clients and Basic256Sha256 signing and encryption:

    goos: darwin
goarch: arm64
pkg: github.com/edgexfoundry/device-opc-ua/internal/driver
Benchmark_HandleReadCommands_ReuseClientWithEncryptionAsync
Listening on 0.0.0.0:48408
Benchmark_HandleReadCommands_ReuseClientWithEncryptionAsync-8           6984        150175 ns/op
PASS

I also try #48 on my local computer, it performs nearly the same as my single reusable client test case:

goos: darwin
goarch: arm64
pkg: github.com/edgexfoundry/device-opc-ua/internal/driver
Benchmark_HandleReadCommands_ReuseClient
Listening on 0.0.0.0:48408
Benchmark_HandleReadCommands_ReuseClient-8          9033        128572 ns/op
PASS

As we can see, when running with SecurityPolicyNone, connection pool reduces ns per op by 30000 ns. when running with SecurityPolicyBasic256Sha256 signing and ecryption, pool reduce ns/op by 60000ns. Despite the small effect, it is still helpful for reducing race condition.