OPCFoundation / UA-Java-Legacy

This repository is provided by OPC Foundation as legacy support for an Java version for OPC UA.
https://github.com/OPCFoundation/UA-.NETStandard
Other
355 stars 227 forks source link

The activate method in SessionChannel class accepts username and password in string format #226

Closed pradeipk closed 1 year ago

pradeipk commented 1 year ago

The activate method in SessionChannel class accepts username and password in string format which makes the password available in memory dump. It is security issue and needs to be fixed. ActivateSessionResponse activate(String username, String password)

jouniaro commented 1 year ago

It works as specified.

https://opcua.info/Core/Part4/v105/docs/7.41

"Each UserIdentityToken allowed by an Endpoint shall have a UserTokenPolicy specified in the EndpointDescription. The UserTokenPolicy specifies what SecurityPolicy to use when encrypting or signing. If this SecurityPolicy is null or empty then the Client uses the SecurityPolicy in the EndpointDescription. If the matching SecurityPolicy is set to None then no encryption or signature is required."

But you are right, the server should not define any UserTokenPolicy with SecurityPolicy None.

pradeipk commented 1 year ago

Thanks @jouniaro for your reply. For user name and password based connection even when security policy is not defined we can use some encryption technique so that password is not passed through in plain text. As when we do this the string literal is available in memory dump. When we are capturing the password from user we are storing it in some secure vault but when the classes are instantiated and the connection is to be created password is passed to the opc stack API in plain text. This is where this password is exposed to the memory and is available for the memory dump. As a fix to this, we can keep the password encrypted even deep inside the stack code until it is being set to token.setPassword method. image

//password is passed in plain text, it should be encrypted before it is actually consumed. image

image

pradeipk commented 1 year ago

Hi @jouniaro, any update on the above query, should we fix and raise the PR ?

jouniaro commented 1 year ago

OK, now I got it what you mean. And yes, you are right that using String for passwords leads to the fact that the password may be left in memory after use. The simple solution is to change the parameter to char[] - which will enable different tactics to hide the password away from the stack.

So, yes, you can fix this and create a PR.

pradeipk commented 1 year ago

thanks @jouniaro

jouniaro commented 1 year ago

Fixed in PR #230