Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[preview env] extend deny egress block #8476

Merged
merged 1 commit into from
Mar 1, 2022
Merged

[preview env] extend deny egress block #8476

merged 1 commit into from
Mar 1, 2022

Conversation

iQQBot
Copy link
Contributor

@iQQBot iQQBot commented Feb 28, 2022

Description

From #8336 We added GCP metadata security detection to the Preview environment, so that if GCP metadata is accessible in the workspace, the supervisor will immediately end the process to prevent sensitive information from being leaked.

But if we only block 169.254.169.254, it still can access by HTTP request, I don't know why

Perhaps in GKE, this IP is special and may not be controlled by the network policy

After testing, I found that changing the blocked IP to 169.254.169.254/30 solves the problem, I don't know the exact reason, but it does work

I am not sure this will affect any GKE cluster or just our preview environment, if all GKE clusters affect, I think changing installer network policy is a better way

We have two ways to temporarily alleviate this problem

  1. disable metadata detection in the Preview environment
  2. use 169.254.169.254/30 as a mitigation measure

This PR use option 2

Related Issue(s)

Fixes #

How to test

Start a workspace, workspace can run

Release Notes

NONE

Documentation

@codecov
Copy link

codecov bot commented Feb 28, 2022

Codecov Report

Merging #8476 (3b6111a) into main (ba168ed) will decrease coverage by 1.13%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #8476      +/-   ##
==========================================
- Coverage   12.31%   11.17%   -1.14%     
==========================================
  Files          20       18       -2     
  Lines        1161      993     -168     
==========================================
- Hits          143      111      -32     
+ Misses       1014      880     -134     
+ Partials        4        2       -2     
Flag Coverage Δ
components-gitpod-cli-app 11.17% <ø> (ø)
components-local-app-app-darwin-amd64 ?
components-local-app-app-darwin-arm64 ?
components-local-app-app-linux-amd64 ?
components-local-app-app-linux-arm64 ?
components-local-app-app-windows-386 ?
components-local-app-app-windows-amd64 ?
components-local-app-app-windows-arm64 ?

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
components/local-app/pkg/auth/auth.go
components/local-app/pkg/auth/pkce.go

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ba168ed...3b6111a. Read the comment docs.

@iQQBot
Copy link
Contributor Author

iQQBot commented Feb 28, 2022

CC @mrsimonemms I am not sure this will affect any GKE cluster or just our preview environment, if all GKE cluster affect, I think changing installer network policy is a better way

@iQQBot iQQBot requested review from a team February 28, 2022 08:27
@iQQBot iQQBot self-assigned this Feb 28, 2022
@github-actions github-actions bot added team: workspace Issue belongs to the Workspace team team: devx labels Feb 28, 2022
@mrsimonemms
Copy link
Contributor

@iQQBot I'm not sure of the underlying cause, but this looks to be ok.

Looking through the code, we have this IP in twice

@iQQBot
Copy link
Contributor Author

iQQBot commented Feb 28, 2022

@mrsimonemms workspace and imagebuild both controlled by network policy workspace-default
in fact, image builder mk3 not need block, because it's our control part, not user control

@iQQBot
Copy link
Contributor Author

iQQBot commented Feb 28, 2022

@mrsimonemms can you use GKE start a gitpod cluster, and test it if it can access 169.254.169.254 ?

Copy link
Contributor

@mustard-mh mustard-mh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Test ok, approve to make preview env works

@roboquat roboquat merged commit 7cc1647 into main Mar 1, 2022
@roboquat roboquat deleted the pd/fix-env branch March 1, 2022 20:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

5 participants