owncloud / ocis

:atom_symbol: ownCloud Infinite Scale Stack
https://doc.owncloud.com/ocis/next/
Apache License 2.0
1.26k stars 169 forks source link

Incorrectly converted AD objectGUID #7449

Open kenodai opened 9 months ago

kenodai commented 9 months ago

Hi oCIS-Team, oCIS is incorrectly converting an Active Directory objectGUID.

Raw data: xPYr6y1Rf0SKoM8kmfaYDw== oCIS: c4f62beb-2d51-7f44-8aa0-cf2499f6980f Apache Directory Studio: {eb2bf6c4-512d-447f-8aa0-cf2499f6980f}

$ echo "xPYr6y1Rf0SKoM8kmfaYDw==" |base64 -d -i |xxd 00000000: c4f6 2beb 2d51 7f44 8aa0 cf24 99f6 980f ..+.-Q.D...$....

2023-10-10T08:58:35Z ERR error using machine auth error="error: not found: unknown client id" authRes={"status":{"code":6,"message":"unknown client id","trace":"00000000000000000000000000000000"}} line=github.com/owncloud/ocis/v2/services/search/pkg/search/search.go:93 owner={"id":{"idp":"https://*/application/o/ocis-web/","opaque_id":"c4f62beb-2d51-7f44-8aa0-cf2499f6980f","type":1}} service=search 2023-10-10T08:58:35Z ERR error while indexing a space error="error: not found: unknown client id" line=github.com/owncloud/ocis/v2/services/search/pkg/search/events.go:69 service=search spaceID={"opaque_id":"b358e147-b109-4a7e-82b3-018d8d138342$c4f62beb-2d51-7f44-8aa0-cf2499f6980f"} userID={"idp":"https://*/application/o/ocis-web/","opaque_id":"c4f62beb-2d51-7f44-8aa0-cf2499f6980f","type":1} 2023-10-10T08:58:35Z DBG GetUserByClaim error="error: not found: (&(memberOf=CN=oCIS-User,OU=oCIS,OU=Applications,DC=*)(objectclass=user)(objectGUID=c4f62beb-2d51-7f44-8aa0-cf2499f6980f))" line=github.com/cs3org/reva/v2@v2.16.0/pkg/user/manager/ldap/ldap.go:140 pkg=rgrpc service=users traceid=00000000000000000000000000000000

Regards, Marc

rhafer commented 9 months ago

Was already discussed here: https://central.owncloud.org/t/ocis-with-samba-ldap-without-owncloud-schema/41377/18

For some reason MS decided to use a different byte-order on the upper 8 Bytes, for binary encoded UUIDs than what is defined in RFC4122. The golang UUID module we're using (github.com/google/uuid) doesn't know about that and produces a wrong string representation.

IMO the biggest issue of this is that it makes it particularly hard to find the right value to set OCIS_ADMIN_USER_ID for initializing the first ocis admin user. I am not yet sure how we should correctly fix this. Even less so how we should do that without breaking existing setups (it would cause the ocis uids to change).

@kenodai I am pretty sure the error you're seeing is not caused by this. For using the objectGUID in LDAP filters, we are using the (hex-escaped) binary value of the objectGUID (which should be in the right byte-order again). Can you share more details about your config?

kenodai commented 9 months ago

Sorry, missed that Thread.

Both error messages contain the incorrect GUID in the opaque_id fields. The DBG/error line for GetUserByClaim I posted also contains the incorrect GUID.

But since you mentioned, that's probably not an issue for the first two and just an output issue on the DBG message?

Setup is more or less still whats mention here: https://github.com/owncloud/ocis-charts/issues/397

Except for the schema part, which is now:

        schema:
          id: "objectGUID"
          idIsOctetString: true
[...]

To be quite honest, I don't see anything failing on my side. But seeing an error running through your console during testing is always something to take a dive in.

rhafer commented 9 months ago

But since you mentioned, that's probably not an issue for the first two and just an output issue on the DBG message?

Yes and no. Technically oCIS should just work fine even with those wrongly decoded UUID strings. All ocis needs is a unique string identifying a users. And as converting its wrongly decoded string UUID back into binary will still result in the correct binary representation functionality wise we should be fine.

However it can be quite an annoyance if you're trying to match those wrongly decoded UUID string against your AD users my means of other tools (e.g. Apache DS). So if possible we should try to fix that somehow. Which is going to be tricky (if not impossible) for the reasons I stated above.

To be quite honest, I don't see anything failing on my side.

Ok. Good to know.

But seeing an error running through your console during testing is always something to take a dive in.

Might it just be that the user with the GUID {eb2bf6c4-512d-447f-8aa0-cf2499f6980f} is just not a member of the CN=oCIS-User,OU=oCIS,OU=Applications,DC=* group?

kenodai commented 9 months ago

But since you mentioned, that's probably not an issue for the first two and just an output issue on the DBG message?

Yes and no. Technically oCIS should just work fine even with those wrongly decoded UUID strings. All ocis needs is a unique string identifying a users. And as converting its wrongly decoded string UUID back into binary will still result in the correct binary representation functionality wise we should be fine.

However it can be quite an annoyance if you're trying to match those wrongly decoded UUID string against your AD users my means of other tools (e.g. Apache DS). So if possible we should try to fix that somehow.

That's what triggered me.

Which is going to be tricky (if not impossible) for the reasons I stated above.

Maybe just add an option, if one is dealing with RFC or MS GUIDs.

But seeing an error running through your console during testing is always something to take a dive in.

Might it just be that the user with the GUID {eb2bf6c4-512d-447f-8aa0-cf2499f6980f} is just not a member of the CN=oCIS-User,OU=oCIS,OU=Applications,DC=* group?

The GUID is a member of that group.

[2023/10/10 14:40:40.931097,  5, pid=104028, effective(0, 0), real(0, 0)] ../../source4/ldap_server/ldap_backend.c:975(ldapsrv_SearchRequest)
  ldapsrv_SearchRequest: LDAP Query: Duration was 0.00s, SearchRequest by S-1-5-21-1309045910-852615573-1332660097-1111 from ipv4:10.50.128.16:58570 filter: [(&(memberOf=CN=oCIS-User,OU=oCIS,OU=Applications,DC=*)(objectclass=user)(objectGUID=c4f62beb-2d51-7f44-8aa0-cf2499f6980f))] basedn: [OU=Accounts,DC=*] scope: [SUB] result: Success

That's from my samba log. That search does not look correct.

Just noticed, ocis-charts are still referencing 4.0.1. Has anything been changed in that area in 4.0.2?

rhafer commented 9 months ago

Just noticed, ocis-charts are still referencing 4.0.1. Has anything been changed in that area in 4.0.2?

No. Not that I am aware of.


  ldapsrv_SearchRequest: LDAP Query: Duration was 0.00s, SearchRequest by S-1-5-21-1309045910-852615573-1332660097-1111 from ipv4:10.50.128.16:58570 filter: [(&(memberOf=CN=oCIS-User,OU=oCIS,OU=Applications,DC=*)(objectclass=user)(objectGUID=c4f62beb-2d51-7f44-8aa0-cf2499f6980f))] basedn: [OU=Accounts,DC=*] scope: [SUB] result: Success

That's from my samba log. That search does not look correct.

True. Something is borked there.

Btw, where/how did you set that memberOf=CN=.... filter? That does appear the the config you pasted in the other issue.

kenodai commented 9 months ago

You are right. Here is the full version:

externalDomain: ocis.owncloud.test

insecure:
  ocisHttpApiInsecure: true

logging:
  level: "debug"
  pretty: "true"
  color: "true"

cache:
  type: "redis-sentinel"
  nodes:
    - redis.ocis-redis:26379/mymaster

services:
  nats:
    persistence:
      enabled: true
  store:
    persistence:
      enabled: true
  search:
    persistence:
      enabled: true
  storagesystem:
    persistence:
      enabled: true
  storageusers:
    storageBackend:
      driver: s3ng
      driverConfig:
        s3ng:
          metadataBackend: messagepack
          endpoint: http://s3.*
          bucket: ocis
    persistence:
      enabled: true
  web:
    persistence:
      enabled: true

features:
  sharing:
    publiclink:
      writeableShareMustHavePassword: true
  externalUserManagement:
    enabled: true
    oidc:
      issuerURI: https://sso.*/application/o/ocis-web/
      webClientID: *
      userIDClaim: preferred_username
      userIDClaimAttributeMapping: username
      roleAssignment:
        enabled: true
        claim: roles
        mapping:
          - role_name: admin
            claim_value: oCIS-Administrator
          - role_name: spaceadmin
            claim_value: oCIS-SpaceAdmin
          - role_name: user
            claim_value: oCIS-User
          - role_name: guest
            claim_value: oCIS-Guest
    ldap:
      writeable: false
      uri: ldaps://dc.*/
      certTrusted: true
      #insecure: true
      bindDN: "CN=tu_ldap,ou=ressource,dc=*"
      useServerUUID: true
      refintEnabled: true
      user:
        objectClass: user
        baseDN: "OU=Accounts,DC=*"
        filter: '(memberOf=CN=oCIS-User,OU=oCIS,OU=Applications,DC=*)'
        schema:
          id: "objectGUID"
          idIsOctetString: true
          userName: sAMAccountName
      group:
        objectClass: group
        baseDN: "OU=oCIS,OU=Applications,DC=*"
        schema:
          id: "objectGUID"
          idIsOctetString: true
          groupName: cn

ingress:
  enabled: true
  annotations:
    nginx.ingress.kubernetes.io/proxy-body-size: 1024m
  tls:
    - secretName: ocis-tls
      hosts:
        - ocis.owncloud.test

secretRefs:
  ldapSecretRef: "ldap-secret"
  s3CredentialsSecretRef: "s3-secret"

That's the config I'm playing with right now. The S3 part was added somewhen today, after I've opened the ticket. But it doesn't really change anything.

rhafer commented 9 months ago

Thanks for the updated config.


  ldapsrv_SearchRequest: LDAP Query: Duration was 0.00s, SearchRequest by S-1-5-21-1309045910-852615573-1332660097-1111 from ipv4:10.50.128.16:58570 filter: [(&(memberOf=CN=oCIS-User,OU=oCIS,OU=Applications,DC=*)(objectclass=user)(objectGUID=c4f62beb-2d51-7f44-8aa0-cf2499f6980f))] basedn: [OU=Accounts,DC=*] scope: [SUB] result: Success

That's from my samba log. That search does not look correct.

True. Something is borked there.

What is really weird that you shouldn't be seeing the string formatted UUID in the LDAP filter coming from ocis. When idIsOctetString: true we're using the hex escaped binary value of the UUID . And that's also what is appearing the the logs of my test setup:

  ldapsrv_SearchRequest: LDAP Query: Duration was 0.00s, SearchRequest by S-1-5-21-399570549-1318098479-901577043-500 from ipv4:10.100.0.1:42962 filter: [(&(&(objectclass=user)(memberof=CN=Domain\20Admins,CN=Users,DC=owncloud,DC=test))(objectGUID=\03\84\09\A6O\26\8AD\8C\9A\0F\C7Y\D2\00\15))] basedn: [dc=owncloud,dc=test] scope: [SUB] result: Success

It looks a bit as if somewhere in the helmcharts (or in ocis) the idIsOctetString setting is not correctly picked up. Still need to check that ...

kenodai commented 9 months ago

kubectl get po users-78c8c9f869-gmcjs -o yaml

    - name: OCIS_LDAP_USER_SCHEMA_ID
      value: objectGUID
    - name: USERS_LDAP_GROUP_SCHEMA_ID
      value: objectGUID
    - name: USERS_LDAP_USER_SCHEMA_ID_IS_OCTETSTRING
      value: "true"
    - name: USERS_LDAP_GROUP_SCHEMA_ID_IS_OCTETSTRING
      value: "true"
kubectl exec -it users-78c8c9f869-gmcjs -- env |grep -i schema
USERS_LDAP_GROUP_SCHEMA_GROUPNAME=cn
OCIS_LDAP_USER_SCHEMA_ID=objectGUID
USERS_LDAP_USER_SCHEMA_MAIL=mail
USERS_LDAP_GROUP_SCHEMA_ID=objectGUID
USERS_LDAP_USER_SCHEMA_ID_IS_OCTETSTRING=true
USERS_LDAP_GROUP_SCHEMA_ID_IS_OCTETSTRING=true
USERS_LDAP_GROUP_SCHEMA_MEMBER=member
USERS_LDAP_GROUP_SCHEMA_MAIL=mail
USERS_LDAP_USER_SCHEMA_DISPLAYNAME=displayname
USERS_LDAP_USER_SCHEMA_USERNAME=sAMAccountName
USERS_LDAP_GROUP_SCHEMA_DISPLAYNAME=cn

Judging by that. The charts provide the correct values.

Which version are you running in your lab? Your search string is constructed differently as well.

kenodai commented 9 months ago

Did some further digging within the logs.

Found this line: 2023-10-10T18:52:59Z DBG getEntryByFilter attributes=["displayname","objectGUID","mail","sAMAccountName","sn","givenname","ownCloudUserEnabled","ownCloudUserType"] backend=ldap base=OU=Accounts,DC=* filter="(&(memberOf=CN=oCIS-User,OU=oCIS,OU=Applications,DC=*)(objectClass=user)(|(sAMAccountName=c4f62beb-2d51-7f44-8aa0-cf2499f6980f)(objectGUID=\\c4\\f6\\2b\\eb\\2d\\51\\7f\\44\\8a\\a0\\cf\\24\\99\\f6\\98\\0f)))" line=github.com/owncloud/ocis/v2/services/graph/pkg/identity/ldap.go:459 scope=2 service=graph sizelimit=1

That's the graph service, but everything related to users is always using the formatted string.

Meanwhile I've also switched to 4.0.2, but the issue remains.

rhafer commented 9 months ago

Judging by that. The charts provide the correct values.

Yes

Which version are you running in your lab? Your search string is constructed differently as well.

I tried with 4.0.1, 4.0.2 and master. I probably just tried the wrong thing, it seems to only show up after uploading. And I was just able to reproduce it. There's still a bug remaining in the users service. Which causes some user lookups done by the search and userlog (via authmachine) service to fail. I'll open a separate issue, as it is unrelated to the byte-order mess with AD objectGUID.