[OPENDJ-6418] backup tool with non-admin user fails with a version error message Created: 28/Jun/19  Updated: 08/Nov/19  Resolved: 20/Aug/19

Status: Done
Project: OpenDJ
Component/s: access control, backends, regression, tools
Affects Version/s: 7.0.0
Fix Version/s: 7.0.0

Type: Bug Priority: Major
Reporter: carole forel Assignee: Gaetan Boismal [X] (Inactive)
Resolution: Fixed Votes: 0
Labels: Verified

Epic Link: Bugs 7.0
Story Points: 2
QA Assignee: carole forel

 Description   

Found with rev 662cb20ef80d9fab0fbe801335931f63594eb7e9

We have a test that tries a backup command as an unprivileged user.
This used to fail with an "insufficient access rights" error message but now fails and displays a message that suggests it cannot retrieve the version of the server:

/local/GIT/pyforge/results/20190628-171539/privileges_group/DJ1/opendj/bin/backup -h openam.example.com -p 4444 -D "uid=auser,o=Privileges Tests,dc=example,dc=com" -w ACIRules -X -d /tmp -a

Other: There was an error trying to read the remote server version: Other:
Tool version does not match remote server version. Please use the same tool
version as the server one.
Tool version: '7.0.0.662cb20ef80d9fab0fbe801335931f63594eb7e9'
Server version: 'Unknown'

To reproduce:

./run-pybot.py -n -v -s privileges_group -t Backend_Backup opendj


 Comments   
Comment by Gaetan Boismal [X] (Inactive) [ 28/Jun/19 ]

This must be a consequence of change made for OPENDJ-6152, I will try to fix that next week.

Comment by Gaetan Boismal [X] (Inactive) [ 04/Jul/19 ]

First investigation results

The version mismatch check when the server is online has been added by commit 859c3c05f6 (see OPENDJ-6152 description and commit message for motivation).
It is performed by this code:

    public static void checkVersionMismatchWithServer(Version clientVersion, Connection connection)
            throws LdapException {
        final Version serverVersion;
        try {
            // Here, a non admin user would not be able to read fullVendorVersion hence serverVersion will be null
            serverVersion = readVersion(connection, "", "fullVendorVersion");
        } catch (final LdapException e) {
            throw newLdapException(OTHER, ERR_CANNOT_READ_REMOTE_SERVER_VERSION.get(e.getMessageObject()), e);
        }

        if (!equalsIncludingScmRevision(clientVersion, serverVersion)) {
           // This misleading error will be printed on the tool output hence the issue
            throw newLdapException(
                    OTHER,
                    ERR_TOOL_AND_SERVER_VERSIONS_MISMATCH.get(clientVersion, toStringOrUnknown(serverVersion)));
        }
    }

Obviously we have added the version check before any other operations , hence when a non admin user try to use the backup tool (or any other task tools, I suspect dsconfig is impacted as well), it is not allowed to read the fullVendorVersion attribute on root DSE entry because of this ACI:

ds-cfg-global-aci: (target="ldap:///")(targetscope="base")(targetattr="objectClass||namingContexts||supportedAuthPasswordSchemes||supportedControl||supportedExtension||supportedFeatures||supportedLDAPVersion||supportedSASLMechanisms||supportedTLSCiphers||supportedTLSProtocols||vendorName||vendorVersion")(version 3.0; acl "User-Visible Root DSE Operational Attributes"; allow (read,search,compare) userdn="ldap:///anyone";)

Indeed, since commit 329b0f8b882 , we read remote server version from attribute fullVendorVersion of the root DSE entry.
Today the following tools use this method to determine remote server version (i.e --offline mode is not impacted):

  • dsconfig
  • status
  • backup
  • restore
  • import-ldif
  • export-ldif
  • rebuild-index

Here is two options to solve this issue:
1. Add fullVendorVersion in the default ACI mentioned above so that the attribute can be read by any user.
+ I guess this would not be a security issue, vendorVersion attribute is already visible without restriction, fullVendorVersion only adds the SCM revision.
+ Code change would be very easy and minor (only update a default ACI)
- If default ACIs are updated, the weird error mentioned in the issue may come back

2. Adapt code to see if the user has enough privilege to read the field
+ This will work regardless of the ACI configuration
- More complicated to implement code wise, I'm not even sure how to do this (search request with a get effective rights control and interpret the response to generate an error message as meaningful as possible)

3. Hybrid approach
Combine both advantages of options above. Make fullVendorVersion visible, perform the same request as before (without any control nor smart response parsing) and add an if statement to catch null values. In that case displays a general message saying that the user is not allowed to read server full version and so tool cannot proceed.

WDYT ? /cc Matthew Swift carole forel

Comment by Jean-Noël Rouvignac [ 04/Jul/19 ]

2. would require that you add a catch (AuthorizationException e) in the code and possibly check the result code too (AUTHORIZATION_DENIED?)

Then what do we do if we receive this exception?

Comment by Gaetan Boismal [X] (Inactive) [ 04/Jul/19 ]

We do not have any exception thrown, just an empty response:

    public static Version readVersion(Connection connection, String entryDn, String attributeDescription)
            throws LdapException {
        try {
            return connection.readEntry(entryDn, attributeDescription)
                             .parseAttribute(attributeDescription)
                             .asString(Version::version);
            // Here we return null since parseAttribute() as returned an emptyAttribute                 
        } catch (EntryNotFoundException e) {
            // We never end here and no LdapException is thrown neither
            return null;
        }
    }

In effect, the request is successfully performed but because of the User-Visible Root DSE Operational Attributes ACI, the attribute we want to read is not part of the response.
Hence the (3) suggestion which would look like:

    public static Version readVersion(Connection connection, String entryDn, String attributeDescription)
            throws LdapException {
        try {
            final Version v  = connection.readEntry(entryDn, attributeDescription)
                             .parseAttribute(attributeDescription)
                             .asString(Version::version);
            if (v == null) {
                throw new WhateverTypeException("Unable to read server version, please ensure you have enough privileges to run the tool.");
            }
        } catch (EntryNotFoundException e) { 
            return null;
        }
    }
Comment by Matthew Swift [ 05/Jul/19 ]

I think it is reasonable to make the fullVendorVersion public by default seeing as vendorVersion already is. What do you think Ludovic Poitou?

What error message is output if the user provides invalid credentials (e.g. wrong password)? Hopefully the error is pretty clear and doesn't mislead the user by saying that there is a version mismatch.

I think the hybrid (3) approach is correct. If the tool cannot get the version information then fail with a message like the one you suggest above.

Comment by Ludovic Poitou [ 05/Jul/19 ]

Usually fullVendorVersion is not available publicly because it shouldn't be of interest of any regular user, and it's not a standard attribute.

But I agree it is reasonable to make it publicly visible by default.

Comment by Gaetan Boismal [X] (Inactive) [ 09/Jul/19 ]

Resolution status

Option (3) mentioned in this comment has been implemented.
Hence fullVendorVersion is now publicly visible by default and when a client tool cannot read it, a more helpful than before error message is printed.

Comment by carole forel [ 11/Jul/19 ]

verified with rev 71e4856907f

Comment by carole forel [ 08/Aug/19 ]

we have quite the same issue when upgrading a server.
For instance on an upgraded instance from 5.0.0 to OpenDJ 7.0.0-SNAPSHOT (0f06286fb31):

/root/workspace/OpenDJ-7.0.x/tests_full_linux_upgrade/results/20190804-023643/privileges_group/DJ1/opendj/bin/backup -h openam.example.com -p 4476 -D "uid=auser,o=Privileges Tests,dc=example,dc=com" -w ACIRules -X -n userRoot -d /root/workspace/OpenDJ-7.0.x/tests_full_linux_upgrade/results/20190804-023643/privileges_group/tmp 	
11:39:32.162 	WARN 	ERROR:
-- rc --
returned 9, expected to be in [1]
-- stdout --

-- stderr --
Unable to read the remote server version. Please ensure that you have
sufficient privileges to run the tool

To reproduce:

in config.cfg:

[OpenDJ]
...
version = ["4.0.0","7.0.0-SNAPSHOT"]

Then:
./run-pybot.py -n -s privileges_group.BackupTask -t Backend_backup opendj
Comment by Gaetan Boismal [X] (Inactive) [ 19/Aug/19 ]

Indeed, after quick investigation this is exactly the same issue as described in comments above but this time on an upgraded instance.
Because we do not have an upgrade task which update the default ACI updated in this commit .
Is it ok to add an upgrade task to update the default ACI? If yes is it worth it? I would say yes but I'm puzzled.
Otherwise I'm not sure about what we can do to solve such kind of issue since indeed as the error message says, the user just does not have sufficient privileges to run the tool.
Either updating "manually" the default ACI to include fullVendorVersion or running the backup command with admin credentials can fix the issue.

Do you guys think we should update the default ACI in an upgrade task? cc Ludovic Poitou Matthew Swift

Comment by Ludovic Poitou [ 19/Aug/19 ]

If the fullVendorVersion attribute is required to be read by any user to check the version, then yes, I think the rootDSE ACI should be subject of an upgrade task. 

I think it could be a little bit of a challenge though, because the upgrade just needs to add fullVendorVersion to the global-aci that grants read access to the rootDSE (target="ldap:///").

Comment by Matthew Swift [ 19/Aug/19 ]

Alternative solution suggested during sprint planning meeting: during upgrade just add a single ACI value that grants access to fullVendorVersion rather than attempting to modify existing ACI values.

Comment by Gaetan Boismal [X] (Inactive) [ 20/Aug/19 ]

(Second) Resolution Status

As suggested above by Matt, issue on upgraded instances is fixed by the add of the following ACI during upgrade:

(target="ldap:///")(targetscope="base")(targetattr="fullVendorVersion")(version 3.0; acl "User-Visible Root DSE Operational Attribute fullVendorVersion"; allow (read,search,compare) userdn="ldap:///anyone";)
Comment by carole forel [ 26/Aug/19 ]

verified in upgraded mode with rev 673e36c829a

Generated at Sat Nov 28 22:38:24 UTC 2020 using Jira 7.13.12#713012-sha1:6e07c38070d5191bbf7353952ed38f111754533a.