Bug 54644 - Strange behaviors when user has multiple userCertificate
Strange behaviors when user has multiple userCertificate
Status: CLOSED DUPLICATE of bug 38762
Product: UCS
Classification: Unclassified
Component: UMC - Users
UCS 5.0
Other Linux
: P5 normal (vote)
: ---
Assigned To: Florian Best
Carlos García-Mauriño
https://git.knut.univention.de/univen...
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2022-04-06 13:31 CEST by Carlos García-Mauriño
Modified: 2022-09-27 17:28 CEST (History)
2 users (show)

See Also:
What kind of report is it?: Development Internal
What type of bug is this?: ---
Who will be affected by this bug?: ---
How will those affected feel about the bug?: ---
User Pain:
Enterprise Customer affected?:
School Customer affected?:
ISV affected?:
Waiting Support:
Flags outvoted (downgraded) after PO Review:
Ticket number:
Bug group (optional): API change
Max CVSS v3 score:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Carlos García-Mauriño univentionstaff 2022-04-06 13:31:23 CEST
When a user has several `userCertificate`s:
  * Replacing the certificate from the web interface removes all other certificates and only saves the new one.
  * Deleting the certificate shown in the web panel causes all certificates to be removed.

It can be replicated by creating a user with a certificate and then adding another one manually via LDAP (since neither the web interface nor UDM allow it). It can be done with the following commands:

```bash
udm users/user create --set username=test2 --set lastname=name1 --set password=univention --set userCertificate="$(openssl x509 -in /etc/univention/ssl/"$(hostname -f)"/cert.pem | head -n -1 | tail -n +2)"

openssl x509 -in /etc/univention/ssl/ucsCA/CAcert.pem -outform der -out /tmp/1.der
```

then add the following content to `1.ldif`:

```ldif
dn: uid=test2,dc=mydomain,dc=intranet
changetype: modify
add: userCertificate;binary
userCertificate;binary:< file:///tmp/1.der
```

and then execute:

```bash
ldapmodify -D cn=admin,dc=mydomain,dc=intranet -y /etc/ldap.secret -f 1.ldif
```

Check the number of certificates with:

```bash
univention-ldapsearch uid=test2 userCertificate -LLL | grep -e "^userCertificate;binary::" | wc -l
```

Once this is done, open the user for editing in the web interface, go to Advanced Settings>Certificate and upload a new one. Run the previous command again and you'll see that there is only one certificate (instead of two). If you then add a second certificate again (with `ldapmodify` as explained above), and then delete the certificate from the web interface, you will see that all certificates are removed.
Comment 1 Carlos García-Mauriño univentionstaff 2022-04-06 13:48:56 CEST
Furthermore, if a certificate is added via UDM command line,some worrying warning messages appear. Steps for replication:

```bash
udm users/user modify --dn "uid=test2,dc=mydomain,dc=intranet" --append userCertificate="$(base64 -i < /tmp/1.der | tr -d '\n')"
```

Output:

```text
[...]
WARNING: cannot append S to userCertificate, value exists
WARNING: cannot append w to userCertificate, value exists
WARNING: cannot append c to userCertificate, value exists
WARNING: cannot append u to userCertificate, value exists
WARNING: cannot append A to userCertificate, value exists
WARNING: cannot append y to userCertificate, value exists
WARNING: cannot append z to userCertificate, value exists
WARNING: cannot append b to userCertificate, value exists
WARNING: using --append on a single value property (userCertificate) is not supported.
E: Invalid Syntax: PKI user certificate (DER format): Not a valid Base64 string: /
```
Comment 2 Arvid Requate univentionstaff 2022-04-06 16:39:39 CEST
Even worse, attempting to append an additional valid DER, base64 enocoded but with newlines even removes the existing userCertificate:

```bash
udm users/user modify --dn "uid=test4,dc=ucs50domain,dc=net" --append userCertificate="$(openssl x509 -in /etc/univention/ssl/ucsCA/CAcert.pem | head -n -1 | tail -n +2)"
```

results in:

```
WARNING: cannot append I to userCertificate, value exists
WARNING: cannot append C to userCertificate, value exists
WARNING: cannot append F to userCertificate, value exists
[...]
WARNING: cannot append F to userCertificate, value exists
WARNING: cannot append 
 to userCertificate, value exists
WARNING: cannot append A to userCertificate, value exists
WARNING: cannot append Q to userCertificate, value exists
[...]
WARNING: cannot append S to userCertificate, value exists
WARNING: cannot append K to userCertificate, value exists
WARNING: cannot append l to userCertificate, value exists
WARNING: cannot append U to userCertificate, value exists
WARNING: cannot append N to userCertificate, value exists
WARNING: cannot append g to userCertificate, value exists
WARNING: cannot append / to userCertificate, value exists
WARNING: using --append on a single value property (userCertificate) is not supported.
Object modified: uid=test5,dc=ucs50domain,dc=net
```

And the previous userCertificate is then removed from the object..
Comment 3 Carlos García-Mauriño univentionstaff 2022-04-08 17:49:49 CEST
(In reply to Arvid Requate from comment #2)
> Even worse, attempting to append an additional valid DER, base64 enocoded
> but with newlines even removes the existing userCertificate:
> 
> ```bash
> udm users/user modify --dn "uid=test4,dc=ucs50domain,dc=net" --append
> userCertificate="$(openssl x509 -in /etc/univention/ssl/ucsCA/CAcert.pem |
> head -n -1 | tail -n +2)"
> ```
> 
> results in:
> 
> ```
> WARNING: cannot append I to userCertificate, value exists
> WARNING: cannot append C to userCertificate, value exists
> WARNING: cannot append F to userCertificate, value exists
> [...]
> WARNING: cannot append F to userCertificate, value exists
> WARNING: cannot append 
>  to userCertificate, value exists
> WARNING: cannot append A to userCertificate, value exists
> WARNING: cannot append Q to userCertificate, value exists
> [...]
> WARNING: cannot append S to userCertificate, value exists
> WARNING: cannot append K to userCertificate, value exists
> WARNING: cannot append l to userCertificate, value exists
> WARNING: cannot append U to userCertificate, value exists
> WARNING: cannot append N to userCertificate, value exists
> WARNING: cannot append g to userCertificate, value exists
> WARNING: cannot append / to userCertificate, value exists
> WARNING: using --append on a single value property (userCertificate) is not
> supported.
> Object modified: uid=test5,dc=ucs50domain,dc=net
> ```
> 
> And the previous userCertificate is then removed from the object..

Apparently, this occurs because `object_input` function of the `admincli` module expects the value of `userCertificate` passed on the argument `append` to be a list (for example a list of certificates) but is getting a `str` directly instead. Furthermore, it checks that `userCertificate` is a single value propoerty AFTER the mess. See https://git.knut.univention.de/univention/ucs/-/blob/5.0-1/management/univention-directory-manager-modules/modules/univention/admincli/admin.py#L248

I'm not sure about which function should change. One option would be to check first if the property allows for multiple values before anything else and raise an exception if a user tries to append. Another option would be to check if the value to append is indeed a list before iterating it. And another option more would be to ensure that it's a list in the caller function or even before it.
Comment 4 Florian Best univentionstaff 2022-04-08 22:00:38 CEST
A fix for at least commment 1 is already part of my MR: https://git.knut.univention.de/univention/ucs/-/merge_requests/279/diffs?commit_id=9a48523b74c44180753fdf5c6fcf5bebeb670665
The QA for that is already ongoing.
Comment 5 Florian Best univentionstaff 2022-04-22 11:47:33 CEST
This has been fixed via Bug #38762 git:42c76995c60ff77196a84f08a3040c39aa9b216a git:dd9ac1745eb18052dd4dfed3bf808e47155d91f0.

*** This bug has been marked as a duplicate of bug 38762 ***