Implement support for kerberos authentication #13

tsipinakis · 2021-11-25T12:36:03Z

Please describe the change you are making

Implement a kerberos authentication flow. It includes an additional webhook to the authentication server for authorization.
The authentication method requires the keytab to be mounted in the container (or be present on the host).

Example configuration:

    auth:
      method: kerberos
      kerberos:
        keytab: '/etc/krb5.keytab'
        authz:
          enable: true
          url: http://127.0.0.1:8080

In order to run this, the changes from ContainerSSH/agent#3 and ContainerSSH/gokrb5#2

TODO:

Test/Add support for docker backend
Add principalUser to authz request in case ticket username != ssh username
Finish off rough edges
- ConnectionId/Remote address not being sent to authz server
- ...
Documentation, look into all the files that need updating
Tests

Are you the owner of the code you are sending in, or do you have permission of the owner?

Sent with permission of the owner

The code will be published under the MIT-0 license. Have you read and understood this license?

Yes

ghost

Hey @tsipinakis thank you very much for your work, this looks awesome. A few minor changes.

auth/protocol.go

cmd/containerssh-testauthconfigserver/main.go

config/auth.go

internal/auth/client_kerberos_factory.go

internal/auth/handler.go

internal/kubernetes/networkHandler.go

ghost

Awesome work, only a few minor things to change.

auth/protocol.go

ghost · 2021-12-26T08:49:10Z

auth/webhook/client.go

@@ -3,6 +3,7 @@ package webhook
 import (
 	"net"

+	auth2 "github.com/containerssh/libcontainerssh/auth"


I would prefer aliasing the internal auth to internalAuth or this one to publicAuth. Using numbered aliases does not help readability.

This is following the existing naming used in other parts of the project (grep for config2). It should probably be changed globally later.

auth/webhook/server_test.go

internal/docker/docker_impl.go

internal/sshserver/dist/CHANGELOG.md

ghost · 2021-12-26T10:15:51Z

internal/test/krb/root/usr/local/bin/init.sh

@@ -25,6 +25,18 @@ kadmin.local -q "addprinc -pw ${KERBEROS_PASSWORD} ${KERBEROS_USERNAME}"
 echo -n "" >/etc/krb5kdc/kadm5.acl
 echo "${KERBEROS_USERNAME}@TESTING.CONTAINERSSH.IO *" >>/etc/krb5kdc/kadm5.acl

+echo -e "\e[32mAdding host principal testing.containerssh.io ...\e[0m"
+kadmin.local -q "addprinc -randkey host/testing.containerssh.io"


Ideally, we wouldn't hard-code credentials but pass them via environment variables like above. However, I can live with this.

message/auth.go

internal/auth/client_kerberos.go

tsipinakis · 2022-01-04T00:22:45Z

@janosdebugs Ready for another round of reviews :)

ghost

@tsipinakis a few minor changes.

auth/metadata.go

auth/protocol.go

config/auth.go

ghost · 2022-01-04T07:36:22Z

config/auth.go

+	AllowPassword bool `json:"allowPassword" yaml:"allowPassword" default:"true"`
+	// ConfigPath is the path of the kerberos configuration file. This is
+	// only used for password authentication.
+	ConfigPath string `json:"configPath" yaml:"configPath" default:"/etc/containerssh/krb5.conf"`


The main reason to do so would be centralized configuration. If people we add configuration via other means then having one centralized configuration structure would make that easier. (e.g. if we ever introduce a webhook on-connection) I'm fine leaving it as an external file for now, but we should keep this in mind for the future and not add too many external configuration files.

ghost · 2022-01-04T07:37:25Z

config/auth.go

+	EnforceUsername bool `json:"enforceUsername" yaml:"enforceUsername" default:"true"`
+	// CredentialCachePath is the path in which the kerberos credentials
+	// will be written inside the user containers.
+	CredentialCachePath string `json:"credentialCachePath" yaml:"credentialCachePath" default:"/tmp/krb5cc"`


Let's make sure this file has the proper permissions. (0600)

It does, the agent write-file command currently only writes with 0600 permissions.
https://github.com/ContainerSSH/agent/pull/3/files#diff-0965f1fa88ac40fcfc62a7eb27c953cf287c8d9a10e80f5d6e84aeb165322594R15

ghost · 2022-01-04T07:54:47Z

internal/auth/client_kerberos.go

+	if k.client.config.EnforceUsername && field.UserName != k.username {
+		return message.NewMessage(
+			message.EAuthKerberosVerificationFailed,
+			"Verify() returned unverified but no error",


Yes, down the line we want to create a web client and that client will support authentication error messages.

internal/auth/client_kerberos.go

ghost · 2022-01-04T07:57:11Z

internal/docker/docker_impl.go

+	if d.config.Execution.Mode == config.DockerExecutionModeSession && d.config.Execution.DisableAgent {
+		return message.NewMessage(
+			message.EDockerWriteFileFailed,
+			"Agent needs to be enable in order to write files",


Let's make sure this text is at least grammatically correct.

internal/auth/client_kerberos.go

auth/protocol.go

tsipinakis force-pushed the gssapi branch from a812822 to 3a027e7 Compare December 1, 2021 13:58

ghost suggested changes Dec 3, 2021

View reviewed changes

tsipinakis force-pushed the gssapi branch 12 times, most recently from 89c5fb1 to e19f4f1 Compare December 15, 2021 15:28

tsipinakis marked this pull request as ready for review December 17, 2021 12:43

tsipinakis changed the title ~~Implement kerberos authentication support~~ Implement support for kerberos authentication Dec 17, 2021

ghost previously approved these changes Dec 26, 2021

View reviewed changes

tsipinakis dismissed ghost ’s stale review via 1ebdc1f January 3, 2022 21:12

ghost suggested changes Jan 4, 2022

View reviewed changes

tsipinakis force-pushed the gssapi branch 3 times, most recently from 25d8fc2 to 4103ae1 Compare January 4, 2022 11:22

ghost enabled auto-merge (squash) January 4, 2022 11:23

ghost previously approved these changes Jan 4, 2022

View reviewed changes

Implement kerberos authentication

f84232d

tsipinakis dismissed ghost ’s stale review via f84232d January 4, 2022 11:24

tsipinakis force-pushed the gssapi branch from 4103ae1 to f84232d Compare January 4, 2022 11:24

ghost approved these changes Jan 4, 2022

View reviewed changes

ghost merged commit 625361f into ContainerSSH:main Jan 4, 2022

This pull request was closed.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement support for kerberos authentication #13

Implement support for kerberos authentication #13

tsipinakis commented Nov 25, 2021 •

edited

Loading

ghost left a comment

ghost left a comment

ghost Dec 26, 2021

tsipinakis Jan 4, 2022

ghost Dec 26, 2021

tsipinakis commented Jan 4, 2022

ghost left a comment

ghost Jan 4, 2022

ghost Jan 4, 2022

tsipinakis Jan 4, 2022

ghost Jan 4, 2022

ghost Jan 4, 2022

Implement support for kerberos authentication #13

Implement support for kerberos authentication #13

Conversation

tsipinakis commented Nov 25, 2021 • edited Loading

Please describe the change you are making

Are you the owner of the code you are sending in, or do you have permission of the owner?

The code will be published under the MIT-0 license. Have you read and understood this license?

ghost left a comment

Choose a reason for hiding this comment

ghost left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tsipinakis commented Jan 4, 2022

ghost left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tsipinakis commented Nov 25, 2021 •

edited

Loading