Improve permission checks in File connector [HZ-2991] #25348

frant-hartm · 2023-08-31T19:13:52Z

SqlConnector#resolveAndValidateFields may try to resolve field names from metadata and a sample. This is not covered by any permission check.

Added SqlConnector#permissionsForResolve that returns permissions required to run the resolveAndValidateFields. Propagation of SqlSecurityContext was required for the actuall permission check.

Added permission to ProcessorSupplier returned by ReadFilesP.MetaSupplier (previously this allowed constructing unsecured DAG):

hazelcast/hazelcast/src/main/java/com/hazelcast/jet/impl/connector/ReadFilesP.java

Lines 179 to 191 in ee7e969

    
           public Function<? super Address, ? extends ProcessorSupplier> get(@Nonnull List<Address> addresses) { 
        
               return address -> ProcessorSupplier.of(new SupplierEx<>() { 
        
                   @Override 
        
                   public Processor getEx() { 
        
                       return new ReadFilesP<>(directory, glob, sharedFileSystem, ignoreFileNotFound, readFileFn); 
        
                   } 
        
                   @Override 
        
                   public List<Permission> permissions() { 
        
                       return singletonList(ConnectorPermission.file(directory, ACTION_READ)); 
        
                   } 
        
               }); 
        
           }

EE PR https://github.com/hazelcast/hazelcast-enterprise/pull/6421 (adds security tests)

Checklist:

Labels (Team:, Type:, Source:, Module:) and Milestone set
Label Add to Release Notes or Not Release Notes content set
Request reviewers if possible
Send backports/forwardports if fix needs to be applied to past/future releases

Backports will be sent to valid branches (need to figure out - all 5.x branches?).

`SqlConnector#resolveAndValidateFields` may try to resolve field names from metadata and a sample. This is not covered by any permission check. Added `SqlConnector#permissionsForResolve` that returns permissions required to run the `resolveAndValidateFields`. Propagation of SqlSecurityContext was required for the actuall permission check. EE PR #... (adds security tests)

hazelcast-sql/src/main/java/com/hazelcast/jet/sql/impl/connector/jdbc/JdbcSqlConnector.java

hazelcast-sql/src/main/java/com/hazelcast/jet/sql/impl/connector/SqlConnector.java

k-jamroz · 2023-09-29T11:45:55Z

you need to merge current master but that will unfortunately make backporting the change harder due to changes for GET_DDL security.

hazelcast/src/main/java/com/hazelcast/jet/impl/connector/ReadFilesP.java

k-jamroz

LGTM, however it might be better to move ReadFileP supplier to SecuredFunctions

`SqlConnector#resolveAndValidateFields` may try to resolve field names from metadata and a sample. This is not covered by any permission check. Added `SqlConnector#permissionsForResolve` that returns permissions required to run the `resolveAndValidateFields`. Propagation of SqlSecurityContext was required for the actuall permission check. EE PR #... (adds security tests) Backport of hazelcast#25348

k-jamroz · 2023-10-02T14:50:30Z

hazelcast/src/main/java/com/hazelcast/jet/impl/connector/ReadFilesP.java

@@ -174,8 +175,9 @@ private MetaSupplier(
        @Nonnull
        @Override
        public Function<? super Address, ? extends ProcessorSupplier> get(@Nonnull List<Address> addresses) {
-            return address -> ProcessorSupplier.of(() -> new ReadFilesP<>(directory, glob, sharedFileSystem,
-                    ignoreFileNotFound, readFileFn));
+            return address -> ProcessorSupplier.of(SecuredFunctions.readFilesProcessorFn(


when backported this might be a breaking change for an upgrade - old lambda will be not present in the code

`SqlConnector#resolveAndValidateFields` may try to resolve field names from metadata and a sample. This is not covered by any permission check. Added `SqlConnector#permissionsForResolve` that returns permissions required to run the `resolveAndValidateFields`. Propagation of SqlSecurityContext was required for the actuall permission check. Added permission to ProcessorSupplier returned by ReadFilesP.MetaSupplier (previously this allowed constructing unsecured DAG): https://github.com/hazelcast/hazelcast/blob/ee7e96933748283a01428e306e1b886b06b8a5ba/hazelcast/src/main/java/com/hazelcast/jet/impl/connector/ReadFilesP.java#L179-L191 EE PR hazelcast/hazelcast-enterprise#6421 (adds security tests)

Backport of #25348 to 5.3.z EE PR hazelcast/hazelcast-enterprise#6621 --------- Co-authored-by: František Hartman <frant.hartm@gmail.com>

Backport of #25348 EE PR: hazelcast/hazelcast-enterprise#6622

Backport of hazelcast#25348 EE PR: hazelcast/hazelcast-enterprise#6631

Backport of hazelcast#25348 EE PR: https://github.com/hazelcast/hazelcast-enterprise/pull/XXX

Backport of #25348 EE PR: hazelcast/hazelcast-enterprise#6631

Backport of #25348 EE PR: hazelcast/hazelcast-enterprise#6632

Backport port of: #25674 and https://github.com/hazelcast/hazelcast-enterprise/pull/6631 Original PRs in master: #25348 and https://github.com/hazelcast/hazelcast-enterprise/pull/6421 --------- Co-authored-by: František Hartman <frant.hartm@gmail.com> GitOrigin-RevId: 836979eec9b74102f67e9f2a44f49c1544dad014

frant-hartm added Type: Defect Source: Internal PR or issue was opened by an employee Add to Release Notes Module: Jet Issues/PRs for Jet Team: Integration labels Aug 31, 2023

frant-hartm added this to the 5.4.0 milestone Aug 31, 2023

frant-hartm requested review from kwart and k-jamroz August 31, 2023 19:13

k-jamroz reviewed Sep 1, 2023

View reviewed changes

hazelcast-sql/src/main/java/com/hazelcast/jet/sql/impl/connector/jdbc/JdbcSqlConnector.java Outdated Show resolved Hide resolved

k-jamroz reviewed Sep 1, 2023

View reviewed changes

hazelcast-sql/src/main/java/com/hazelcast/jet/sql/impl/connector/SqlConnector.java Show resolved Hide resolved

kwart approved these changes Sep 6, 2023

View reviewed changes

degerhz assigned frant-hartm Sep 10, 2023

Merge branch 'master' into fix/HZ-2991-permission-bypass-file-read

191bebc

Prevent another file read

ee7e969

frant-hartm changed the title ~~Check permission before resolving mapping fields [HZ-2991]~~ Improve permission checks in File connector [HZ-2991] Oct 2, 2023

Merge branch 'master' into fix/HZ-2991-permission-bypass-file-read

49a116d

frant-hartm requested review from k-jamroz and kwart October 2, 2023 07:25

k-jamroz reviewed Oct 2, 2023

View reviewed changes

hazelcast/src/main/java/com/hazelcast/jet/impl/connector/ReadFilesP.java Outdated Show resolved Hide resolved

k-jamroz approved these changes Oct 2, 2023

View reviewed changes

kwart approved these changes Oct 2, 2023

View reviewed changes

Move PS to SecuredFunctions

5bebb05

k-jamroz reviewed Oct 2, 2023

View reviewed changes

kwart merged commit 98be233 into hazelcast:master Oct 4, 2023
8 checks passed

kwart mentioned this pull request Oct 4, 2023

Improve permission checks in File connector [HZ-2991] [5.3.z] #25603

Merged

frant-hartm mentioned this pull request Oct 10, 2023

Improve permission checks in File connector [HZ-2991] [5.3.3] #25661

Merged

4 tasks

frant-hartm added a commit that referenced this pull request Oct 10, 2023

Improve permission checks in File connector [HZ-2991] [5.3.z] (#25603)

da9feb2

Backport of #25348 to 5.3.z EE PR hazelcast/hazelcast-enterprise#6621 --------- Co-authored-by: František Hartman <frant.hartm@gmail.com>

frant-hartm added a commit that referenced this pull request Oct 10, 2023

Improve permission checks in File connector [HZ-2991] [5.3.3] (#25661)

d0028c5

Backport of #25348 EE PR: hazelcast/hazelcast-enterprise#6622

frant-hartm added a commit to frant-hartm/hazelcast that referenced this pull request Oct 11, 2023

Improve permission checks in File connector [HZ-2991] [5.2.z]

9b872d3

Backport of hazelcast#25348 EE PR: hazelcast/hazelcast-enterprise#6631

frant-hartm mentioned this pull request Oct 11, 2023

Improve permission checks in File connector [HZ-2991] [5.2.z] #25674

Merged

4 tasks

frant-hartm added a commit to frant-hartm/hazelcast that referenced this pull request Oct 11, 2023

Improve permission checks in File connector [HZ-2991] [5.1.z]

9245ee5

Backport of hazelcast#25348 EE PR: https://github.com/hazelcast/hazelcast-enterprise/pull/XXX

frant-hartm mentioned this pull request Oct 11, 2023

Improve permission checks in File connector [HZ-2991] [5.1.z] #25676

Merged

4 tasks

frant-hartm added a commit that referenced this pull request Oct 13, 2023

Improve permission checks in File connector [HZ-2991] [5.2.z] (#25674)

884cf4e

Backport of #25348 EE PR: hazelcast/hazelcast-enterprise#6631

frant-hartm added a commit that referenced this pull request Oct 13, 2023

Improve permission checks in File connector [HZ-2991] [5.1.z] (#25676)

317cdee

Backport of #25348 EE PR: hazelcast/hazelcast-enterprise#6632

github-actions bot mentioned this pull request Mar 25, 2024

Improper Access Control SNYK-JAVA-COMHAZELCAST-6249443 RADAR-base/ManagementPortal#771

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Improve permission checks in File connector [HZ-2991] #25348

Improve permission checks in File connector [HZ-2991] #25348

frant-hartm commented Aug 31, 2023 •

edited

k-jamroz commented Sep 29, 2023

k-jamroz left a comment

k-jamroz Oct 2, 2023

	public Function<? super Address, ? extends ProcessorSupplier> get(@Nonnull List<Address> addresses) {
	return address -> ProcessorSupplier.of(new SupplierEx<>() {
	@Override
	public Processor getEx() {
	return new ReadFilesP<>(directory, glob, sharedFileSystem, ignoreFileNotFound, readFileFn);
	}

	@Override
	public List<Permission> permissions() {
	return singletonList(ConnectorPermission.file(directory, ACTION_READ));
	}
	});
	}

Improve permission checks in File connector [HZ-2991] #25348

Improve permission checks in File connector [HZ-2991] #25348

Conversation

frant-hartm commented Aug 31, 2023 • edited

k-jamroz commented Sep 29, 2023

k-jamroz left a comment

Choose a reason for hiding this comment

k-jamroz Oct 2, 2023

Choose a reason for hiding this comment

frant-hartm commented Aug 31, 2023 •

edited