From 0490733e110ecce737adec94c78de8f6bb618384 Mon Sep 17 00:00:00 2001 From: Arthur Normand Date: Sat, 12 Nov 2022 16:57:02 -0500 Subject: [PATCH 1/6] OAuth now uses client TLS certs (if configured) --- cli/deployment/config.go | 10 ++++++++++ cli/server.go | 23 +++++++++++++++++++++++ codersdk/deploymentconfig.go | 14 ++++++++------ 3 files changed, 41 insertions(+), 6 deletions(-) diff --git a/cli/deployment/config.go b/cli/deployment/config.go index d7c7e4a6d33f2..43ed251b74140 100644 --- a/cli/deployment/config.go +++ b/cli/deployment/config.go @@ -282,6 +282,16 @@ func newConfig() *codersdk.DeploymentConfig { Flag: "tls-min-version", Default: "tls12", }, + ClientCertFile: &codersdk.DeploymentConfigField[string]{ + Name: "TLS Client Cert File", + Usage: "Path to certificate for client TLS authentication. It requires a PEM-encoded file.", + Flag: "tls-client-cert-file", + }, + ClientKeyFile: &codersdk.DeploymentConfigField[string]{ + Name: "TLS Client Key File", + Usage: "Path to key for client TLS authentication. It requires a PEM-encoded file.", + Flag: "tls-client-key-file", + }, }, Trace: &codersdk.TraceConfig{ Enable: &codersdk.DeploymentConfigField[bool]{ diff --git a/cli/server.go b/cli/server.go index 4c335d405cd8a..8fe6ba0173352 100644 --- a/cli/server.go +++ b/cli/server.go @@ -392,6 +392,11 @@ func Server(vip *viper.Viper, newAPI func(context.Context, *coderd.Options) (*co return xerrors.Errorf("OIDC issuer URL must be set!") } + ctx, err := handleOauth2ClientCertificates(cfg, ctx) + if err != nil { + return xerrors.Errorf("configure oidc client certificates: %w", err) + } + oidcProvider, err := oidc.NewProvider(ctx, cfg.OIDC.IssuerURL.Value) if err != nil { return xerrors.Errorf("configure oidc provider: %w", err) @@ -1248,3 +1253,21 @@ func startBuiltinPostgres(ctx context.Context, cfg config.Root, logger slog.Logg } return connectionURL, ep.Stop, nil } + +func handleOauth2ClientCertificates(cfg *codersdk.DeploymentConfig, ctx context.Context) (context.Context, error) { + if cfg.TLS.ClientCertFile.Value != "" && cfg.TLS.ClientKeyFile.Value != "" { + certificates, err := loadCertificates([]string{cfg.TLS.ClientCertFile.Value}, []string{cfg.TLS.ClientKeyFile.Value}) + if err != nil { + return nil, err + } + + ctx = context.WithValue(ctx, oauth2.HTTPClient, &http.Client{ + Transport: &http.Transport{ + TLSClientConfig: &tls.Config{ + Certificates: certificates, + }, + }, + }) + } + return ctx, nil +} diff --git a/codersdk/deploymentconfig.go b/codersdk/deploymentconfig.go index 301c2382df753..904838201dedf 100644 --- a/codersdk/deploymentconfig.go +++ b/codersdk/deploymentconfig.go @@ -101,12 +101,14 @@ type TelemetryConfig struct { } type TLSConfig struct { - Enable *DeploymentConfigField[bool] `json:"enable" typescript:",notnull"` - CertFiles *DeploymentConfigField[[]string] `json:"cert_file" typescript:",notnull"` - ClientAuth *DeploymentConfigField[string] `json:"client_auth" typescript:",notnull"` - ClientCAFile *DeploymentConfigField[string] `json:"client_ca_file" typescript:",notnull"` - KeyFiles *DeploymentConfigField[[]string] `json:"key_file" typescript:",notnull"` - MinVersion *DeploymentConfigField[string] `json:"min_version" typescript:",notnull"` + Enable *DeploymentConfigField[bool] `json:"enable" typescript:",notnull"` + CertFiles *DeploymentConfigField[[]string] `json:"cert_file" typescript:",notnull"` + ClientAuth *DeploymentConfigField[string] `json:"client_auth" typescript:",notnull"` + ClientCAFile *DeploymentConfigField[string] `json:"client_ca_file" typescript:",notnull"` + KeyFiles *DeploymentConfigField[[]string] `json:"key_file" typescript:",notnull"` + MinVersion *DeploymentConfigField[string] `json:"min_version" typescript:",notnull"` + ClientCertFile *DeploymentConfigField[string] `json:"client_cert_file" typescript:",notnull"` + ClientKeyFile *DeploymentConfigField[string] `json:"client_key_file" typescript:",notnull"` } type TraceConfig struct { From 54ee42918f91b4807bbf8c99f4b47e06edb07846 Mon Sep 17 00:00:00 2001 From: Arthur Normand Date: Sat, 12 Nov 2022 17:04:03 -0500 Subject: [PATCH 2/6] Update docs --- docs/admin/auth.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/docs/admin/auth.md b/docs/admin/auth.md index b9f7033dca33c..877ef1d8af929 100644 --- a/docs/admin/auth.md +++ b/docs/admin/auth.md @@ -75,6 +75,12 @@ Once complete, run `sudo service coder restart` to reboot Coder. > When a new user is created, the `preferred_username` claim becomes the username. If this claim is empty, the email address will be stripped of the domain, and become the username (e.g. `example@coder.com` becomes `example`). +If your OpenID Connect provider requires client TLS certificates for authentication, you can configure them like so: +```console +CODER_TLS_CLIENT_CERT_FILE=/path/to/cert.pem +CODER_TLS_CLIENT_KEY_FILE=/path/to/key.pem +``` + ## SCIM (enterprise) Coder supports user provisioning and deprovisioning via SCIM 2.0 with header From ae0c5dd97690bdfc77c039f43106b15406d52580 Mon Sep 17 00:00:00 2001 From: Arthur Normand Date: Sat, 12 Nov 2022 17:10:15 -0500 Subject: [PATCH 3/6] Cleaning --- cli/server.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cli/server.go b/cli/server.go index 8fe6ba0173352..67653a709e1ac 100644 --- a/cli/server.go +++ b/cli/server.go @@ -1261,13 +1261,13 @@ func handleOauth2ClientCertificates(cfg *codersdk.DeploymentConfig, ctx context. return nil, err } - ctx = context.WithValue(ctx, oauth2.HTTPClient, &http.Client{ + return context.WithValue(ctx, oauth2.HTTPClient, &http.Client{ Transport: &http.Transport{ TLSClientConfig: &tls.Config{ Certificates: certificates, }, }, - }) + }), nil } return ctx, nil } From 5df2ca8be5b611fd9eac1e8695083571aa6738f8 Mon Sep 17 00:00:00 2001 From: Arthur Normand Date: Sat, 12 Nov 2022 20:55:04 -0500 Subject: [PATCH 4/6] Fix lint errors and generate static files --- cli/server.go | 4 ++-- cli/testdata/coder_server_--help.golden | 8 ++++++++ scripts/apitypings/main.go | 2 +- site/src/api/typesGenerated.ts | 4 +++- 4 files changed, 14 insertions(+), 4 deletions(-) diff --git a/cli/server.go b/cli/server.go index 67653a709e1ac..bf9e31f8d7982 100644 --- a/cli/server.go +++ b/cli/server.go @@ -392,7 +392,7 @@ func Server(vip *viper.Viper, newAPI func(context.Context, *coderd.Options) (*co return xerrors.Errorf("OIDC issuer URL must be set!") } - ctx, err := handleOauth2ClientCertificates(cfg, ctx) + ctx, err := handleOauth2ClientCertificates(ctx, cfg) if err != nil { return xerrors.Errorf("configure oidc client certificates: %w", err) } @@ -1254,7 +1254,7 @@ func startBuiltinPostgres(ctx context.Context, cfg config.Root, logger slog.Logg return connectionURL, ep.Stop, nil } -func handleOauth2ClientCertificates(cfg *codersdk.DeploymentConfig, ctx context.Context) (context.Context, error) { +func handleOauth2ClientCertificates(ctx context.Context, cfg *codersdk.DeploymentConfig) (context.Context, error) { if cfg.TLS.ClientCertFile.Value != "" && cfg.TLS.ClientKeyFile.Value != "" { certificates, err := loadCertificates([]string{cfg.TLS.ClientCertFile.Value}, []string{cfg.TLS.ClientKeyFile.Value}) if err != nil { diff --git a/cli/testdata/coder_server_--help.golden b/cli/testdata/coder_server_--help.golden index d70e2e6f0cbad..3a9e6122e1e9b 100644 --- a/cli/testdata/coder_server_--help.golden +++ b/cli/testdata/coder_server_--help.golden @@ -177,6 +177,14 @@ Flags: used for checking the authenticity of client Consumes $CODER_TLS_CLIENT_CA_FILE + --tls-client-cert-file string Path to certificate for client TLS + authentication. It requires a PEM-encoded + file. + Consumes $CODER_TLS_CLIENT_CERT_FILE + --tls-client-key-file string Path to key for client TLS + authentication. It requires a PEM-encoded + file. + Consumes $CODER_TLS_CLIENT_KEY_FILE --tls-enable Whether TLS will be enabled. Consumes $CODER_TLS_ENABLE --tls-key-file strings Paths to the private keys for each of the diff --git a/scripts/apitypings/main.go b/scripts/apitypings/main.go index 49555b317210e..5351084a8c041 100644 --- a/scripts/apitypings/main.go +++ b/scripts/apitypings/main.go @@ -63,7 +63,7 @@ type TypescriptTypes struct { // String just combines all the codeblocks. func (t TypescriptTypes) String() string { var s strings.Builder - _, _ = s.WriteString("// Code generated by 'make coder/scripts/apitypings/main.go'. DO NOT EDIT.\n\n") + _, _ = s.WriteString("// Code generated by 'make site/src/api/typesGenerated.ts'. DO NOT EDIT.\n\n") sortedTypes := make([]string, 0, len(t.Types)) sortedEnums := make([]string, 0, len(t.Enums)) diff --git a/site/src/api/typesGenerated.ts b/site/src/api/typesGenerated.ts index 68362eb1ec265..7e0e10ded46f6 100644 --- a/site/src/api/typesGenerated.ts +++ b/site/src/api/typesGenerated.ts @@ -1,4 +1,4 @@ -// Code generated by 'make coder/scripts/apitypings/main.go'. DO NOT EDIT. +// Code generated by 'make site/src/api/typesGenerated.ts'. DO NOT EDIT. // From codersdk/apikey.go export interface APIKey { @@ -598,6 +598,8 @@ export interface TLSConfig { readonly client_ca_file: DeploymentConfigField readonly key_file: DeploymentConfigField readonly min_version: DeploymentConfigField + readonly client_cert_file: DeploymentConfigField + readonly client_key_file: DeploymentConfigField } // From codersdk/deploymentconfig.go From d71be7779014d33cc39b10bb0fae5fba4c50b0b5 Mon Sep 17 00:00:00 2001 From: Arthur Normand Date: Sat, 12 Nov 2022 21:06:39 -0500 Subject: [PATCH 5/6] Fix lint error and regenerate more static files --- cli/server.go | 1 + scripts/apitypings/testdata/enums/enums.ts | 2 +- scripts/apitypings/testdata/generics/generics.ts | 2 +- 3 files changed, 3 insertions(+), 2 deletions(-) diff --git a/cli/server.go b/cli/server.go index bf9e31f8d7982..df18c29647eca 100644 --- a/cli/server.go +++ b/cli/server.go @@ -1264,6 +1264,7 @@ func handleOauth2ClientCertificates(ctx context.Context, cfg *codersdk.Deploymen return context.WithValue(ctx, oauth2.HTTPClient, &http.Client{ Transport: &http.Transport{ TLSClientConfig: &tls.Config{ + //nolint:gosec Certificates: certificates, }, }, diff --git a/scripts/apitypings/testdata/enums/enums.ts b/scripts/apitypings/testdata/enums/enums.ts index da07185cbd701..0aca42a2feebd 100644 --- a/scripts/apitypings/testdata/enums/enums.ts +++ b/scripts/apitypings/testdata/enums/enums.ts @@ -1,4 +1,4 @@ -// Code generated by 'make coder/scripts/apitypings/main.go'. DO NOT EDIT. +// Code generated by 'make site/src/api/typesGenerated.ts'. DO NOT EDIT. // From codersdk/enums.go export type Enum = "bar" | "baz" | "foo" | "qux" diff --git a/scripts/apitypings/testdata/generics/generics.ts b/scripts/apitypings/testdata/generics/generics.ts index bd770148430fa..ce851f0cc6ff5 100644 --- a/scripts/apitypings/testdata/generics/generics.ts +++ b/scripts/apitypings/testdata/generics/generics.ts @@ -1,4 +1,4 @@ -// Code generated by 'make coder/scripts/apitypings/main.go'. DO NOT EDIT. +// Code generated by 'make site/src/api/typesGenerated.ts'. DO NOT EDIT. // From codersdk/generics.go export interface ComplexGeneric { From 83fcf35f3e1bc7a220e71abbc273784090313caa Mon Sep 17 00:00:00 2001 From: Arthur Normand Date: Sat, 12 Nov 2022 21:26:03 -0500 Subject: [PATCH 6/6] Suppress lint error --- cli/server.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/cli/server.go b/cli/server.go index df18c29647eca..628fcb419dd86 100644 --- a/cli/server.go +++ b/cli/server.go @@ -1263,8 +1263,7 @@ func handleOauth2ClientCertificates(ctx context.Context, cfg *codersdk.Deploymen return context.WithValue(ctx, oauth2.HTTPClient, &http.Client{ Transport: &http.Transport{ - TLSClientConfig: &tls.Config{ - //nolint:gosec + TLSClientConfig: &tls.Config{ //nolint:gosec Certificates: certificates, }, },