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

[WIP] Authenticated agent #1247

Draft
wants to merge 8 commits into
base: feature/authenticated-microservices
Choose a base branch
from
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
apiVersion: networking.k8s.io/v1
kind: NetworkPolicy
metadata:
namespace: save-agent-network-policy
spec:
podSelector:
matchLabels:
io.kompose.service: save-agent
policyTypes:
- Egress
egress:
- to:
- podSelector:
matchLabels:
io.kompose.service: orchestrator
- to:
# https://stackoverflow.com/q/73049535
- ipBlock:
cidr: 0.0.0.0/0
# Forbid private IP ranges effectively allowing only egress to the Internet
except:
- 10.0.0.0/8
- 172.16.0.0/12
- 192.168.0.0/16
- to:
- namespaceSelector:
matchLabels:
kubernetes.io/metadata.name: "kube-system"
- podSelector:
matchLabels:
k8s-app: "kube-dns"
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,8 @@ import javax.persistence.ManyToOne
/**
* @property containerId id of the container, inside which the agent is running
* @property execution id of the execution, which the agent is serving
* @property version
* @property version version of the agent binary
* @property isAuthenticated whether this agent has already received a token from orchestrator
*/
@Entity
class Agent(
Expand All @@ -19,4 +20,6 @@ class Agent(
var execution: Execution,

var version: String? = null,

var isAuthenticated: Boolean,
) : BaseEntity()
6 changes: 5 additions & 1 deletion save-deploy/base-images/Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,13 @@ RUN if [ "$BASE_IMAGE_NAME" = "python" ]; then \
fi

RUN groupadd --gid 1100 save-agent && \
groupadd --gid 1200 save-executor && \
useradd --uid 1100 --gid 1100 --create-home --shell /bin/sh save-agent && \
useradd --uid 1200 --gid 1100 --create-home --shell /bin/sh save-agent && \
usermod -aG save-executor save-agent && \
Copy link
Member

Choose a reason for hiding this comment

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

I cannot understand why do we create a new user and remove the key from the pod?
As far as I can see, there are two ways:

  1. Leave the key in /var/run/secrets/kubernetes.io/serviceaccount/token and forbid the child process of save-agent accessing it (probably create another user as you do here);
  2. Forget about Unix permission system and delegate token management to orchestrating pod.

To my mind second way brings less problems as in case of orchestrator sending token to agent might cause problems with save-demo-agent (it has ktor server running and so we might need to implement auth for ktor server also). What was your idea?

Sorry for bothering and thanks in advance

Copy link
Member Author

Choose a reason for hiding this comment

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

The problem with the second approach is that you'd need to reliably detect requests from not-yet-authorized agents and distinguish them from similar requests from a tested tool. One idea (I believe, that's why I've added Agent.isAuthenticated) was to initialize agents with isAuthenticated = false, then these agents would send request for a token (which they would do strictly before starting a tested tool), orchestrator would update isAuthenticated = true and won't return token for any new requests from this agent. However, there is no guarantee that already running tool will be able to mimic as a newly created agent and get the token.
That's why the first approach seemed more viable: The agent would read token from a file, that has restricted access for a user, that runs a tested tool.

Copy link
Member

Choose a reason for hiding this comment

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

Thanks! Now I got the idea

# `WORKDIR` directive creates the directory as `root` user unless the directory already exists
mkdir /home/save-agent/save-execution && \
chown -R 1100:1100 /home/save-agent/save-execution
chown -R 1100:1100 /home/save-agent/save-execution && \
echo 'save-agent ALL = (save-executor) ALL' >> /etc/sudoers
USER save-agent
WORKDIR /home/save-agent/save-execution
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,8 @@ class KubernetesManager(
}
// If agent fails, we should handle it manually (update statuses, attempt restart etc.)
restartPolicy = "Never"
// save-agent pods shouldn't have access to valid cluster tokens
automountServiceAccountToken = false
containers = listOf(
agentContainerSpec(baseImageTag, agentRunCmd, workingDir, configuration.env)
)
Expand Down