Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

User authentication has security bug #1457

Closed
ryangu18 opened this issue Apr 26, 2022 · 9 comments · Fixed by #1458
Closed

User authentication has security bug #1457

ryangu18 opened this issue Apr 26, 2022 · 9 comments · Fixed by #1458

Comments

@ryangu18
Copy link

Describe the bug

User authentication has security bug, even enable server.set_security_IDs(“Username”),But I can log still in anonymously

To Reproduce

Steps to reproduce the behavior incl code.

The code comes from here #1153 (comment)
it is Mr. AndreasHeine's demonstration code, I didn't make any changes.

Expected behavior

Create a ua server, the security policy is Basic256Sha256_SignAndEncrypt , the authentication user/pwd is 'user1' and 'pw1'

Screenshots

image

This screenshot is as expected, we see the security policy, and can only authenticate with username and password, anonymous is disabled, Select ok, then, we do not connect to this server, but right-click to open properties,
image
At this time I see that Anonymous has been enabled, choose anonymous,
image

ok , and connect to the server, we see that the server has been connected without entering a password.

image

Version

Python-Version: python 3.9 64bit

python-opcua Version (e.g. master branch, 0.9): 0.98.13
image

@schroeder-
Copy link
Contributor

schroeder- commented Apr 26, 2022

UAExpert allows to select every Security Policy even if it is not support, if you go over the Properties menu.

@AndreasHeine
Copy link
Member

nether the less it should not be able to connect with anonymus...

@ryangu18
Copy link
Author

Note that the security policy and authorization for this server are set in the code

server.set_security_policy([
# ua.SecurityPolicyType.NoSecurity,
# ua.SecurityPolicyType.Basic128Rsa15_Sign,
# ua.SecurityPolicyType.Basic128Rsa15_SignAndEncrypt,
# ua.SecurityPolicyType.Basic256Sha256_Sign,
ua.SecurityPolicyType.Basic256Sha256_SignAndEncrypt
])
policyIDs = ["Username"]

My problem was that I bypassed the password entry using the method above.
If it's not a bug, sorry for my bad submission, but how do I make my expectations valid? "Anonymous login is prohibited and must be authorized with username, password

@schroeder-
Copy link
Contributor

schroeder- commented Apr 26, 2022

I quickly scanned through the code and I think the server always accepts ever UserToken even if it is disabled via set_security_policy. The server only provides the supported UserTokens for discovery/GetEndpoints, but on create_session this is not checked.

@AndreasHeine
Copy link
Member

should be checked in activate session!
image

@schroeder-
Copy link
Contributor

schroeder- commented Apr 26, 2022

Its not see:

id_token = params.UserIdentityToken
if isinstance(id_token, ua.UserNameIdentityToken):
if self.user_manager.check_user_token(self, id_token) == False:
raise utils.ServiceError(ua.StatusCodes.BadUserAccessDenied)
self.logger.info("Activated internal session %s for user %s", self.name, self.user)
return result

I can provide a pr in the next days.

@AndreasHeine
Copy link
Member

@schroeder- could you check asyncua aswell probably the same there!

@schroeder-
Copy link
Contributor

@AndreasHeine same, I can provide the same fix there.

@floriandorre
Copy link

Hi,

Is there a chance that this PR will be merged even if this package is not maintained anymore ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants