HDDS-6942. Ozone Buckets/Objects created via S3 should not allow group access

[kerneltime]

Buckets created via S3 should not allow read access for usersin same group

What changes were proposed in this pull request?

Buckets created via S3 should not allow read access for users

What is the link to the Apache JIRA

https://issues.apache.org/jira/browse/HDDS-6942

How was this patch tested?

Bucket created via S3 vs ozone sh

bash-4.2$ aws s3api --endpoint-url http://localhost:9878 create-bucket --bucket bucket2
bash-4.2$ ozone sh bucket getacl s3v/bucket2
[ {
  "type" : "USER",
  "name" : "key",
  "aclScope" : "ACCESS",
  "aclList" : [ "ALL" ]
} ]
bash-4.2$ ozone sh bucket create s3v/bucket
bash-4.2$ ozone sh bucket getacl s3v/bucket 
[ {
  "type" : "USER",
  "name" : "hadoop",
  "aclScope" : "ACCESS",
  "aclList" : [ "ALL" ]
}, {
  "type" : "GROUP",
  "name" : "hadoop",
  "aclScope" : "ACCESS",
  "aclList" : [ "ALL" ]
} ]
bash-4.2$ 

[adoroszlai]

It may be better to set this config from OzoneClientCache before any client is instantiated.

ozone/hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/OzoneClientCache.java

Lines 58 to 92 in a8808d1

	private OzoneClientCache(OzoneConfiguration ozoneConfiguration)
	throws IOException {
	// Set the expected OM version if not set via config.
	ozoneConfiguration.setIfUnset(OZONE_CLIENT_REQUIRED_OM_VERSION_MIN_KEY,
	OZONE_CLIENT_REQUIRED_OM_VERSION_MIN_DEFAULT);
	String omServiceID = OmUtils.getOzoneManagerServiceId(ozoneConfiguration);
	secConfig = new SecurityConfig(ozoneConfiguration);
	client = null;
	try {
	if (secConfig.isGrpcTlsEnabled()) {
	if (ozoneConfiguration
	.get(OZONE_OM_TRANSPORT_CLASS,
	OZONE_OM_TRANSPORT_CLASS_DEFAULT) !=
	OZONE_OM_TRANSPORT_CLASS_DEFAULT) {
	// Grpc transport selected
	// need to get certificate for TLS through
	// hadoop rpc first via ServiceInfo
	setCertificate(omServiceID,
	ozoneConfiguration);
	}
	}
	if (omServiceID == null) {
	client = OzoneClientFactory.getRpcClient(ozoneConfiguration);
	} else {
	// As in HA case, we need to pass om service ID.
	client = OzoneClientFactory.getRpcClient(omServiceID,
	ozoneConfiguration);
	}
	} catch (IOException e) {
	LOG.warn("cannot create OzoneClient", e);
	throw e;
	}
	// S3 Gateway should always set the S3 Auth.
	ozoneConfiguration.setBoolean(S3Auth.S3_AUTH_CHECK, true);
	}

[kerneltime]

It may be better to set this config from OzoneClientCache before any client is instantiated.

ozone/hadoop-ozone/s3gateway/src/main/java/org/apache/hadoop/ozone/s3/OzoneClientCache.java

Lines 58 to 92 in a8808d1

	private OzoneClientCache(OzoneConfiguration ozoneConfiguration)
	throws IOException {
	// Set the expected OM version if not set via config.
	ozoneConfiguration.setIfUnset(OZONE_CLIENT_REQUIRED_OM_VERSION_MIN_KEY,
	OZONE_CLIENT_REQUIRED_OM_VERSION_MIN_DEFAULT);
	String omServiceID = OmUtils.getOzoneManagerServiceId(ozoneConfiguration);
	secConfig = new SecurityConfig(ozoneConfiguration);
	client = null;
	try {
	if (secConfig.isGrpcTlsEnabled()) {
	if (ozoneConfiguration
	.get(OZONE_OM_TRANSPORT_CLASS,
	OZONE_OM_TRANSPORT_CLASS_DEFAULT) !=
	OZONE_OM_TRANSPORT_CLASS_DEFAULT) {
	// Grpc transport selected
	// need to get certificate for TLS through
	// hadoop rpc first via ServiceInfo
	setCertificate(omServiceID,
	ozoneConfiguration);
	}
	}
	if (omServiceID == null) {
	client = OzoneClientFactory.getRpcClient(ozoneConfiguration);
	} else {
	// As in HA case, we need to pass om service ID.
	client = OzoneClientFactory.getRpcClient(omServiceID,
	ozoneConfiguration);
	}
	} catch (IOException e) {
	LOG.warn("cannot create OzoneClient", e);
	throw e;
	}
	// S3 Gateway should always set the S3 Auth.
	ozoneConfiguration.setBoolean(S3Auth.S3_AUTH_CHECK, true);
	}

I think decision for what the ACL config should be for entities created should reside closer to the request processing. The Client and it's cache should avoid deciding defaults for the requests.

It is much easier to evaluate all the outcomes of an API call if the choices in defaults are where the processing of the API is done. I would prefer to leave it here.

It makes sense to evaluate connection level setting in the Client Cache (TLS etc) but should a bucket have read access to all in the same group is really a S3 API level decision and BaseEndpoint is a good place to store that logic.

[kerneltime]

The CI failures seem unrelated.

[adoroszlai]

The Client and it's cache should avoid deciding defaults for the requests.

I was concerned that the setting may not be effective, as the client may already be created when endpoint's initialization() is called (since client needs to be injected). Now that @neils-dev pointed to OzoneConfigurationHolder, I see that the original approach works because the same instance of OzoneConfiguration is shared in S3 Gateway for all injections.

Can you please add an assertion for the expected ACL in S3 smoketests? hadoop-ozone/dist/src/main/smoketest/s3/bucketcreate.robot or hadoop-ozone/dist/src/main/smoketest/security/ozone-secure-s3.robot might be a good candidate for that.

[adoroszlai]

Thanks @kerneltime for adding the test. Seems like in secure case (in ozonesecure-ha) group ACL is still set:

Create new bucket and check no group ACL                              | FAIL |
'[ {
  "type" : "USER",
  "name" : "testuser/[email protected]",
  "aclScope" : "ACCESS",
  "aclList" : [ "ALL" ]
}, {
  "type" : "GROUP",
  "name" : "testuser",
  "aclScope" : "ACCESS",
  "aclList" : [ "ALL" ]
} ]' contains '"type" : "GROUP"'

https://github.com/apache/ozone/runs/7104193656#step:5:599

[kerneltime]

I think I know why some tests pass and this one fails.
The config for default rights is invoked on the client-side RpcClient.java

  public RpcClient(ConfigurationSource conf, String omServiceId)
      throws IOException {
    Preconditions.checkNotNull(conf);
    this.conf = conf;
    this.ugi = UserGroupInformation.getCurrentUser();
    // Get default acl rights for user and group.
    OzoneAclConfig aclConfig = this.conf.getObject(OzoneAclConfig.class);
    this.userRights = aclConfig.getUserDefaultRights();
    this.groupRights = aclConfig.getGroupDefaultRights();
...
...
...
  private List<OzoneAcl> getAclList() {
    if (ozoneManagerClient.getThreadLocalS3Auth() != null) {
      UserGroupInformation aclUgi =
          UserGroupInformation.createRemoteUser(
             ozoneManagerClient.getThreadLocalS3Auth().getAccessID());
      OzoneAclConfig aclConfig = this.conf.getObject(OzoneAclConfig.class);
      return OzoneAclUtil.getAclList(
          aclUgi.getUserName(),
          aclUgi.getGroupNames(),
         userRights, groupRights);
    }
    return OzoneAclUtil.getAclList(ugi.getUserName(), ugi.getGroupNames(),
        userRights, groupRights);

but the config is only applicable for OM OzoneAclConfig.java

  @Config(key = "group.rights",
      defaultValue = "ALL",
      type = ConfigType.STRING,
      tags = {ConfigTag.OM, ConfigTag.SECURITY},
      description = "Default group permissions set for an object in " +
          "OzoneManager."
  )
  private String groupDefaultRights;

which seems an odd mismatch. If the user used does not have any groups when queried by OM it skips adding the group permissions. The robot tests for non-secure clusters default to a random user who has no groups and the test passes.

[adoroszlai]

Thanks @kerneltime for iterating on this patch.