From eaf450e92f59605df41648266051d1db0fdc6927 Mon Sep 17 00:00:00 2001 From: Pavol Baran <73115616+xbaran4@users.noreply.github.com> Date: Tue, 8 Jun 2021 14:45:20 +0200 Subject: [PATCH] fix: Adding ability to use kubernetes API proxy on Kubernetes (#6) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * enable unsupported/api on kubernetes Signed-off-by: xbaran4 Co-authored-by: Fabrice Flore-Thébault Co-authored-by: Sergii Kabashniuk --- .../che/api/deploy/WsMasterModule.java | 4 +- .../webapp/WEB-INF/classes/che/che.properties | 7 +++ .../DirectKubernetesAPIAccessHelper.java | 8 +-- .../kubernetes/KubernetesClientFactory.java | 9 ++-- .../openshift/OpenShiftClientFactory.java | 5 +- .../server/InfrastructureApiService.java | 25 ++++++--- .../server/InfrastructureApiServiceTest.java | 52 +++++++++++++++++-- 7 files changed, 88 insertions(+), 22 deletions(-) diff --git a/assembly/assembly-wsmaster-war/src/main/java/org/eclipse/che/api/deploy/WsMasterModule.java b/assembly/assembly-wsmaster-war/src/main/java/org/eclipse/che/api/deploy/WsMasterModule.java index d6819211626..459ec52ef81 100644 --- a/assembly/assembly-wsmaster-war/src/main/java/org/eclipse/che/api/deploy/WsMasterModule.java +++ b/assembly/assembly-wsmaster-war/src/main/java/org/eclipse/che/api/deploy/WsMasterModule.java @@ -445,9 +445,7 @@ private void configureMultiUserMode( bindConstant().annotatedWith(Names.named("che.agents.auth_enabled")).to(true); - if (OpenShiftInfrastructure.NAME.equals(infrastructure)) { - install(new InfraProxyModule()); - } + install(new InfraProxyModule()); } private void configureJwtProxySecureProvisioner(String infrastructure) { diff --git a/assembly/assembly-wsmaster-war/src/main/webapp/WEB-INF/classes/che/che.properties b/assembly/assembly-wsmaster-war/src/main/webapp/WEB-INF/classes/che/che.properties index bbf6cadb3d9..f8a055decf3 100644 --- a/assembly/assembly-wsmaster-war/src/main/webapp/WEB-INF/classes/che/che.properties +++ b/assembly/assembly-wsmaster-war/src/main/webapp/WEB-INF/classes/che/che.properties @@ -535,6 +535,13 @@ che.infra.kubernetes.trusted_ca.mount_path=/public-certs # See the `che.infra.kubernetes.trusted_ca.dest_configmap` property. che.infra.kubernetes.trusted_ca.dest_configmap_labels= +# Enables the `/unsupported/k8s` endpoint to resolve calls on Kubernetes infrastructure. +# Provides direct access to the underlying infrastructure REST API. +# This results in huge privilege escalation. +# It impacts only Kubernetes infrastructure. Therefore it implies no security risk on OpenShift with OAuth. +# Do not enable this, unless you understand the risks. +che.infra.kubernetes.enable_unsupported_k8s=false + ### OpenShift Infra parameters # Comma separated list of labels to add to the CA certificates ConfigMap in user workspace. diff --git a/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/DirectKubernetesAPIAccessHelper.java b/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/DirectKubernetesAPIAccessHelper.java index fbc8e5a4d76..38fcda69588 100644 --- a/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/DirectKubernetesAPIAccessHelper.java +++ b/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/DirectKubernetesAPIAccessHelper.java @@ -151,9 +151,11 @@ private static Headers toOkHttpHeaders(HttpHeaders headers) { if (headers != null) { for (Map.Entry> e : headers.getRequestHeaders().entrySet()) { String name = e.getKey(); - List values = e.getValue(); - for (String value : values) { - headersBuilder.add(name, value); + if (HttpHeaders.CONTENT_TYPE.equalsIgnoreCase(name)) { + List values = e.getValue(); + for (String value : values) { + headersBuilder.add(name, value); + } } } } diff --git a/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/KubernetesClientFactory.java b/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/KubernetesClientFactory.java index 94da034e7ff..6311a5a4b7d 100644 --- a/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/KubernetesClientFactory.java +++ b/infrastructures/kubernetes/src/main/java/org/eclipse/che/workspace/infrastructure/kubernetes/KubernetesClientFactory.java @@ -142,13 +142,16 @@ protected OkHttpClient getHttpClient() { *

Unlike {@link #getHttpClient()}, this method creates a new HTTP client instance each time it * is called. * + *

This is a workaround to allow direct k8s API access on Kubernetes infrastructure, since + * impersonating the current user is not supported in the Kubernetes Client. This is insecure and + * will only be called if permitted by enabling it in Che properties. + * * @return HTTP client with authorization set up * @throws InfrastructureException if it is not possible to build the client with authentication * infromation */ public OkHttpClient getAuthenticatedHttpClient() throws InfrastructureException { - throw new InfrastructureException( - "Impersonating the current user is not supported in the Kubernetes Client."); + return create(getDefaultConfig()).getHttpClient(); } /** @@ -216,7 +219,7 @@ protected Interceptor buildKubernetesInterceptor(Config config) { * authenticate with the credentials (user/password or Oauth token) contained in the {@code * config} parameter. */ - private KubernetesClient create(Config config) { + private DefaultKubernetesClient create(Config config) { OkHttpClient clientHttpClient = httpClient.newBuilder().authenticator(Authenticator.NONE).build(); OkHttpClient.Builder builder = clientHttpClient.newBuilder(); diff --git a/infrastructures/openshift/src/main/java/org/eclipse/che/workspace/infrastructure/openshift/OpenShiftClientFactory.java b/infrastructures/openshift/src/main/java/org/eclipse/che/workspace/infrastructure/openshift/OpenShiftClientFactory.java index aad654ca5ce..73805fc824d 100644 --- a/infrastructures/openshift/src/main/java/org/eclipse/che/workspace/infrastructure/openshift/OpenShiftClientFactory.java +++ b/infrastructures/openshift/src/main/java/org/eclipse/che/workspace/infrastructure/openshift/OpenShiftClientFactory.java @@ -129,7 +129,8 @@ public OkHttpClient getAuthenticatedHttpClient() throws InfrastructureException throw new InfrastructureException( "Not able to construct impersonating openshift API client."); } - return clientForConfig(buildConfig(getDefaultConfig(), null)); + // Ensure to get OkHttpClient with all necessary interceptors. + return createOC(buildConfig(getDefaultConfig(), null)).getHttpClient(); } @Override @@ -222,7 +223,7 @@ protected Interceptor buildKubernetesInterceptor(Config config) { }; } - private OpenShiftClient createOC(Config config) { + private DefaultOpenShiftClient createOC(Config config) { return new UnclosableOpenShiftClient( clientForConfig(config), config, this::initializeRequestTracing); } diff --git a/wsmaster/che-core-api-infraproxy/src/main/java/org/eclipse/che/api/infraproxy/server/InfrastructureApiService.java b/wsmaster/che-core-api-infraproxy/src/main/java/org/eclipse/che/api/infraproxy/server/InfrastructureApiService.java index 22a5301bad2..7d0fd1fcaec 100644 --- a/wsmaster/che-core-api-infraproxy/src/main/java/org/eclipse/che/api/infraproxy/server/InfrastructureApiService.java +++ b/wsmaster/che-core-api-infraproxy/src/main/java/org/eclipse/che/api/infraproxy/server/InfrastructureApiService.java @@ -56,24 +56,35 @@ public class InfrastructureApiService extends Service { @Context private MediaType mediaType; - private static boolean determineAllowed(String infra, String identityProvider) { - return "openshift".equals(infra) - && identityProvider != null - && identityProvider.startsWith("openshift"); + private static boolean determineAllowed( + String infra, String identityProvider, boolean allowedForKubernetes) { + if ("openshift".equals(infra)) { + return identityProvider != null && identityProvider.startsWith("openshift"); + } + return allowedForKubernetes; } @Inject public InfrastructureApiService( @Nullable @Named("che.infra.openshift.oauth_identity_provider") String identityProvider, + @Named("che.infra.kubernetes.enable_unsupported_k8s") boolean allowedForKubernetes, RuntimeInfrastructure runtimeInfrastructure) { - this(System.getenv("CHE_INFRASTRUCTURE_ACTIVE"), identityProvider, runtimeInfrastructure); + this( + System.getenv("CHE_INFRASTRUCTURE_ACTIVE"), + allowedForKubernetes, + identityProvider, + runtimeInfrastructure); } @VisibleForTesting - InfrastructureApiService(String infraName, String identityProvider, RuntimeInfrastructure infra) { + InfrastructureApiService( + String infraName, + boolean allowedForKubernetes, + String identityProvider, + RuntimeInfrastructure infra) { this.runtimeInfrastructure = infra; this.mapper = new ObjectMapper(); - this.allowed = determineAllowed(infraName, identityProvider); + this.allowed = determineAllowed(infraName, identityProvider, allowedForKubernetes); } @GET diff --git a/wsmaster/che-core-api-infraproxy/src/test/java/org/eclipse/che/api/infraproxy/server/InfrastructureApiServiceTest.java b/wsmaster/che-core-api-infraproxy/src/test/java/org/eclipse/che/api/infraproxy/server/InfrastructureApiServiceTest.java index f933df6205f..ae67fb35e5f 100644 --- a/wsmaster/che-core-api-infraproxy/src/test/java/org/eclipse/che/api/infraproxy/server/InfrastructureApiServiceTest.java +++ b/wsmaster/che-core-api-infraproxy/src/test/java/org/eclipse/che/api/infraproxy/server/InfrastructureApiServiceTest.java @@ -37,13 +37,15 @@ public class InfrastructureApiServiceTest { @BeforeMethod public void setup() throws Exception { - apiService = new InfrastructureApiService("openshift", "openshift-identityProvider", infra); + apiService = + new InfrastructureApiService("openshift", false, "openshift-identityProvider", infra); } @Test - public void testFailsAuthWhenNotOnOpenShift() throws Exception { + public void testFailsAuthWhenNotAllowedForKubernetesAndNotOnOpenShift() throws Exception { // given - apiService = new InfrastructureApiService("not-openshift", "openshift-identityProvider", infra); + apiService = + new InfrastructureApiService("not-openshift", false, "openshift-identityProvider", infra); // when Response response = @@ -59,7 +61,49 @@ public void testFailsAuthWhenNotOnOpenShift() throws Exception { @Test public void testFailsAuthWhenNotUsingOpenShiftIdentityProvider() throws Exception { // given - apiService = new InfrastructureApiService("openshift", "not-openshift-identityProvider", infra); + apiService = + new InfrastructureApiService("openshift", false, "not-openshift-identityProvider", infra); + + // when + Response response = + given() + .contentType("application/json; charset=utf-8") + .when() + .get("/unsupported/k8s/nazdar/"); + + // then + assertEquals(response.getStatusCode(), 403); + } + + @Test + public void testResolvesCallWhenAllowedForKubernetesOnKubernetes() throws Exception { + // given + apiService = + new InfrastructureApiService("kubernetes", true, "not-openshift-identityProvider", infra); + when(infra.sendDirectInfrastructureRequest(any(), any(), any(), any())) + .thenReturn( + javax.ws.rs.core.Response.ok() + .header("Content-Type", "application/json; charset=utf-8") + .build()); + + // when + Response response = + given() + .contentType("application/json; charset=utf-8") + .when() + .get("/unsupported/k8s/nazdar/"); + + // then + assertEquals(response.getStatusCode(), 200); + assertEquals(response.getContentType(), "application/json;charset=utf-8"); + } + + @Test + public void testFailsAuthWhenAllowedForKubernetesOnOpenshiftWithNonOpenshiftIdentityProvider() + throws Exception { + // given + apiService = + new InfrastructureApiService("openshift", true, "not-openshift-identityProvider", infra); // when Response response =