Bug 56035 - users/passwd exception in UDM REST API since UCS 5.0
users/passwd exception in UDM REST API since UCS 5.0
Status: NEEDMOREINFO
Product: UCS
Classification: Unclassified
Component: UDM - REST API
UCS 5.0
Other Linux
: P5 normal (vote)
: ---
Assigned To: UMC maintainers
UMC maintainers
https://git.knut.univention.de/univen...
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2023-05-03 11:20 CEST by Christina Scheinig
Modified: 2024-01-15 17:03 CET (History)
4 users (show)

See Also:
What kind of report is it?: Bug Report
What type of bug is this?: 3: Simply Wrong: The implementation doesn't match the docu
Who will be affected by this bug?: 1: Will affect a very few installed domains
How will those affected feel about the bug?: 2: A Pain – users won’t like this once they notice it
User Pain: 0.034
Enterprise Customer affected?:
School Customer affected?: Yes
ISV affected?:
Waiting Support:
Flags outvoted (downgraded) after PO Review:
Ticket number: 2023041421000054, 2023112921000343
Bug group (optional): Regression
Max CVSS v3 score:
best: Patch_Available+


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Christina Scheinig univentionstaff 2023-05-03 11:20:20 CEST
A customer reported, that the access to the udm-API in UCS5 is not working as it did in 4.4-9.

The Module users/password shows on a 4.4-9

#> curl -X GET "https://Administrator:XX@<ucs>/univention/udm/users/passwd/uid%3DAdministrator%2Ccn%3Dusers%2Cdc%3Dtest%2Cdc%3Dlocal" -H  "accept: application/json"

{"_links": {"udm:object-modules": [{"title": "All modules", "href": …
----------------

But on a 5.0-2

{"error": {"traceback": "Traceback (most recent call last):\n  File \"/usr/lib/python3/dist-packages/tornado/web.py\", line 1592, in _execute\n    result = yield result\n  File \"/usr/lib/python3/dist-packages/tornado/gen.py\", line 1133, in run\n    value = future.result()\n  File \"/usr/lib/python3/dist-packages/univention/admin/rest/module.py\", line 3087, in get\n    props.update(self.get_representation(module, obj, ['*'], self.ldap_connection, copy))\n  File \"/usr/lib/python3/dist-packages/univention/admin/rest/module.py\", line 3195, in get_representation\n

values = dict(decode_properties(module, obj, values))\n  File \"/usr/lib/python3/dist-packages/univention/admin/rest/module.py\", line 4276, in decode_properties\n    codec = udm_types.TypeHint.detect(prop, key)\n  File \"/usr/lib/python3/dist-packages/univention/admin/types.py\",

line 262, in detect\n    if property.type_class:\nAttributeError: 'NoneType' object has no attribute 'type_class'\n", "code": 500, "title": "", "message": "'NoneType' object has no attribute 'type_class'", "error": {}}, "_links": {"self": [{"title": "HTTP-Error 500: ", "href": "https://<ucs>/univention/udm/users/passwd/uid%3DAdministrator%2Ccn%3Dusers%2Cdc%3Dtest%2Cdc%3Dlocal"}], "curies": [{"name": "udm", "templated": true, "href": "https:// 
----------------

I could reproduce this:

root@primary:/etc/ldap # curl -X GET "https://Administrator:univention@primary.schein.ig/univention/udm/users/passwd/uid=Administrator,cn=users,dc%3Dschein%2Cdc%3Dig" -H  "accept: application/json" -i
HTTP/1.1 500 Internal Server Error
Date: Mon, 17 Apr 2023 14:50:49 GMT
Server: Univention/1.0
X-Permitted-Cross-Domain-Policies: master-only
X-XSS-Protection: 1; mode=block
X-Content-Type-Options: nosniff
Content-Security-Policy: frame-ancestors 'none';
Content-Language: en-US
Content-Type: application/json
Access-Control-Expose-Headers: *, Authorization, X-Request-Id
X-Request-Id: edd06589-31c9-4e27-89c5-07cba03b1780
Link: <https://primary.schein.ig/univention/udm/users/passwd/uid%3DAdministrator%2Ccn%3Dusers%2Cdc%3Dschein%2Cdc%3Dig>; rel="self"; title="HTTP-Error 500: "
Cache-Control: private, must-revalidate, no-cache, no-store
Vary: Accept,Accept-Language,Accept-Encoding,Authorization
Link-Template: <https://primary.schein.ig/univention/udm/relation/{rel}>; rel="curies"; name="udm"
Content-Length: 1365
Via: 1.1 primary.schein.ig
Connection: close

{"error": {"traceback": "Traceback (most recent call last):\n  File \"/usr/lib/python3/dist-packages/tornado/web.py\", line 1592, in _execute\n    result = yield result\
n  File \"/usr/lib/python3/dist-packages/tornado/gen.py\", line 1133, in run\n    value = future.result()\n  File \"/usr/lib/python3/dist-packages/univention/admin/rest/
module.py\", line 3079, in get\n    props.update(self.get_representation(module, obj, ['*'], self.ldap_connection, copy))\n  File \"/usr/lib/python3/dist-packages/univen
tion/admin/rest/module.py\", line 3187, in get_representation\n    values = dict(decode_properties(module, obj, values))\n  File \"/usr/lib/python3/dist-packages/univent
ion/admin/rest/module.py\", line 4266, in decode_properties\n    codec = udm_types.TypeHint.detect(prop, key)\n  File \"/usr/lib/python3/dist-packages/univention/admin/t
ypes.py\", line 260, in detect\n    if property.type_class:\nAttributeError: 'NoneType' object has no attribute 'type_class'\n", "code": 500, "title": "", "message": "'N
oneType' object has no attribute 'type_class'", "error": {}}, "_links": {"self": [{"title": "HTTP-Error 500: ", "href": "https://primary.schein.ig/univention/udm/users/p
asswd/uid%3DAdministrator%2Ccn%3Dusers%2Cdc%3Dschein%2Cdc%3Dig"}], "curies": [{"name": "udm", "templated": true, "href": "https://primary.schein.ig/univention/udm/relati
on/{rel}"}]}}
Comment 1 Florian Best univentionstaff 2024-01-08 13:47:47 CET
@support: When you attach tickets to this can you elaborate what this module was used for?
All things provided can be done via users/user as well.
Comment 2 Daniel Tröder univentionstaff 2024-01-09 09:13:01 CET
This is not necessarily a breaking API change but merely a bug.
Only after fixing the bug can we know if the intended behavior changed.

There are two issues here:

1. HTTP 500 because: »AttributeError: 'NoneType' object has no attribute 'type_class'«
2. The traceback was sent with the HTTP 500 response. TB should never be sent to clients, as it can reveal internal data or state to attackers.

If _after_ fixing the above, the behavior is still different from 4.4; then it is a breaking API change. But we don't know that yet.
Comment 3 Florian Best univentionstaff 2024-01-09 10:07:58 CET
(In reply to Daniel Tröder from comment #2)
> 2. The traceback was sent with the HTTP 500 response. TB should never be
> sent to clients, as it can reveal internal data or state to attackers.
No, we don't expose sensitive internal state via tracebacks. The source code is open source therefore it also doesn't really matter. A attacker can get the traceback on his own by just having a own UCS system.
Tracebacks can already be deactivated via the UCR variable directory/manager/rest/show-tracebacks. If we disable them by default we don't receive useful support tickets anymore or it takes longer to get the valuable information.
Comment 4 Daniel Tröder univentionstaff 2024-01-09 10:32:24 CET
I disagree. An attacker may not have direct access to the UDM REST API but will get the error message forwarded by a UI or some other intermediate system.
The attacker will thus have gained knowledge about a bug in a backend system, which otherwise would have been hidden by a generic error message.
Knowledge about the UDM version reveals the update state of the UCS system. Enabling the attacker to look for not yet fixed known vulnerabilities.

Revealing details about internal errors via the network is generally bad practice.
See OWASP: https://owasp.org/www-community/Improper_Error_Handling

IMHO, higher network security trumps reduced support handling time (asking for a log file).

So the UCR variable directory/manager/rest/show-tracebacks should be set to 'false' by default. I have created a separate bug for that: But 56967.
Comment 5 Florian Best univentionstaff 2024-01-09 11:55:15 CET
I have a working patch:
MR: https://git.knut.univention.de/univention/ucs/-/merge_requests/1012

Maintaining users/passwd is hard and we should drop it imho.
The patch adds all users properties to users/passwd - otherwise for different operations we get a lot of other exceptions.
This makes it basically equal to users/self - so customers should be advised to use that instead or users/user directly.
Comment 6 Daniel Tröder univentionstaff 2024-01-09 14:53:20 CET
> The patch adds all users properties to users/passwd

That _expands_ the API so that in the future, we have to maintain that bigger API without any win for the customer.

> otherwise for different operations we get a lot of other exceptions.

Can you explain this?
Has that module always had broken operations or is that new?
Or.. did you mean with "exceptions" only the Python classes, not the error scenarios?

> Maintaining users/passwd is hard and we should drop it IMHO.

→ What are the use cases for users/passwd?
→ Can those use cases be implemented by "users/self" or "users/user"? Which ones cannot?

If this is a candidate for depreciation, we should push that.
Comment 7 Mirac Erdemiroglu univentionstaff 2024-01-11 12:35:33 CET
Another customer affected 2023112921000343

DC-Master

UCS: 5.0-5 errata857
Installed: admindiary-backend=1.0 admindiary-frontend=1.0 self-service-backend=5.0
Upgradable:

DC-Backup

UCS: 5.0-5 errata857
Installed: adconnector=12.0 admindiary-frontend=1.0 self-service=5.0
Upgradable:
Comment 8 Mirac Erdemiroglu univentionstaff 2024-01-11 12:39:50 CET
(In reply to Mirac Erdemiroglu from comment #7)
> Another customer affected 2023112921000343
> 
> DC-Master
> 
> UCS: 5.0-5 errata857
> Installed: admindiary-backend=1.0 admindiary-frontend=1.0
> self-service-backend=5.0
> Upgradable:
> 
> DC-Backup
> 
> UCS: 5.0-5 errata857
> Installed: adconnector=12.0 admindiary-frontend=1.0 self-service=5.0
> Upgradable:

Here is some customer feedback:

If you find a method specifically for passwords, you are potentially stuck with it.


Since there is a possibility in the product, but it is faulty, a solution for a fix should be created.
Comment 9 Florian Best univentionstaff 2024-01-11 12:46:53 CET
Can you ask the affected customers what they use it for and why they cannot use users/user or users/self?
We need this information to make future decisions about this module.
Comment 10 Florian Best univentionstaff 2024-01-12 12:48:31 CET
The users/passwd did make sense in the past: It was displayed in the UDM-UMC module and users could set/reset their password.
Nowadays we have the self-service for this. And also a menu entry in the UMC itself, to change the own password.
The module is just about the displayed properties, not about the functionality. users/user provides the same functionality.
The self-service/UMC-password-change gues through PAM and therefore doesn't bypass krb5 restrictions, e.g. checking of some password policies from Samba, checking of if the old password was provided and matches the current one, …. Therefore that's more preferred.
Using users/passwd via UDM REST API doesn't make much sense.