Skip to content

Commit

Permalink
fix: Certificater/key should be sanitized always (kyma-project#953)
Browse files Browse the repository at this point in the history
  • Loading branch information
rakesh-garimella authored Apr 9, 2024
1 parent bf93b9c commit 999330a
Show file tree
Hide file tree
Showing 7 changed files with 45 additions and 57 deletions.
8 changes: 7 additions & 1 deletion .gitleaksignore
Original file line number Diff line number Diff line change
@@ -1,3 +1,9 @@
791662e0244f1e326975d9010a611a55d4506623:internal/tls/validate_test.go:private-key:10
304d906b7d106982bf2fe8fe935a64da3b0fe22d:internal/tls/validate_test.go:private-key:23
f7d7d265cb40c46f876bc0a825d7efffbbcc05db:internal/tls/cert/tls_cert_validator_test.go:private-key:26
b2f904f28f760defe0cb8d9d48293c3d0537453f:internal/tls/validate_test.go:private-key:10
99a582ef47730f6775804381fa76ee8b2892ccb9:internal/tls/validate_test.go:private-key:10
dbf7571cb6ba7752b32eaf954c1547dc73f166d4:internal/tls/validate_test.go:private-key:10
f7d7d265cb40c46f876bc0a825d7efffbbcc05db:internal/tls/cert/tls_cert_validator_test.go:private-key:26
0c1b1be475538e896a0ef5173e2407b8754b6193:internal/tls/cert/tls_cert_validator_test.go:private-key:27
d1d091352e4a6e69c8877e4a104f333b54362295:internal/tls/cert/tls_cert_validator_test.go:private-key:178
d1d091352e4a6e69c8877e4a104f333b54362295:internal/tls/cert/tls_cert_validator_test.go:private-key:189
6 changes: 4 additions & 2 deletions internal/otelcollector/config/otlpexporter/env_vars.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package otlpexporter

import (
"bytes"
"context"
"encoding/base64"
"fmt"
Expand All @@ -12,7 +13,6 @@ import (

telemetryv1alpha1 "github.com/kyma-project/telemetry-manager/apis/telemetry/v1alpha1"
"github.com/kyma-project/telemetry-manager/internal/secretref"
"github.com/kyma-project/telemetry-manager/internal/tls"
"github.com/kyma-project/telemetry-manager/internal/utils/envvar"
)

Expand Down Expand Up @@ -109,7 +109,9 @@ func makeTLSEnvVar(ctx context.Context, c client.Reader, secretData map[string][
return err
}

sanitizedCert, sanitizedKey := tls.SanitizeSecret(cert, key)
// Make a best effort replacement of linebreaks in cert/key if present.
sanitizedCert := bytes.ReplaceAll(cert, []byte("\\n"), []byte("\n"))
sanitizedKey := bytes.ReplaceAll(key, []byte("\\n"), []byte("\n"))

tlsConfigCertVariable := makeTLSCertVariable(pipelineName)
secretData[tlsConfigCertVariable] = sanitizedCert
Expand Down
7 changes: 5 additions & 2 deletions internal/reconciler/logpipeline/sync.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package logpipeline

import (
"bytes"
"context"
"fmt"

Expand All @@ -13,7 +14,6 @@ import (
telemetryv1alpha1 "github.com/kyma-project/telemetry-manager/apis/telemetry/v1alpha1"
"github.com/kyma-project/telemetry-manager/internal/fluentbit/config/builder"
"github.com/kyma-project/telemetry-manager/internal/k8sutils"
"github.com/kyma-project/telemetry-manager/internal/tls"
"github.com/kyma-project/telemetry-manager/internal/utils/envvar"
)

Expand Down Expand Up @@ -194,7 +194,10 @@ func (s *syncer) syncTLSConfigSecret(ctx context.Context, logPipelines []telemet
return err
}

sanitizedCert, sanitizedKey := tls.SanitizeSecret(newSecret.Data[targetCertVariable], newSecret.Data[targetKeyVariable])
// Make a best effort replacement of linebreaks in cert/key if present.
sanitizedCert := bytes.ReplaceAll(newSecret.Data[targetCertVariable], []byte("\\n"), []byte("\n"))
sanitizedKey := bytes.ReplaceAll(newSecret.Data[targetKeyVariable], []byte("\\n"), []byte("\n"))

newSecret.Data[targetCertVariable] = sanitizedCert
newSecret.Data[targetKeyVariable] = sanitizedKey
}
Expand Down
9 changes: 7 additions & 2 deletions internal/tls/cert/tls_cert_validator.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package cert

import (
"bytes"
"crypto/x509"
"encoding/pem"
"fmt"
Expand All @@ -27,15 +28,19 @@ func (tcv *TLSCertValidator) ValidateCertificate(certPEM []byte, keyPEM []byte)
Validity: time.Now().Add(time.Hour * 24 * 365),
}

// Make a best effort replacement of linebreaks in cert/key if present.
sanitizedCert := bytes.ReplaceAll(certPEM, []byte("\\n"), []byte("\n"))
sanitizedKey := bytes.ReplaceAll(keyPEM, []byte("\\n"), []byte("\n"))

// Parse the certificate
cert, err := parseCertificate(certPEM)
cert, err := parseCertificate(sanitizedCert)
if err != nil {
result.CertValid = false
result.CertValidationMessage = err.Error()
}

// Parse the private key
if _, err := parsePrivateKey(keyPEM); err != nil {
if _, err := parsePrivateKey(sanitizedKey); err != nil {
result.PrivateKeyValid = false
result.PrivateKeyValidationMessage = err.Error()
}
Expand Down
22 changes: 22 additions & 0 deletions internal/tls/cert/tls_cert_validator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -173,3 +173,25 @@ ga5H3f7hUBINasQIdOGEAy3clqCBpLj2eUMXHHNxVsVGBnJOEqckn6fg6pcHnhmK
require.True(t, validationResult.CertValid, "Certificate is not valid")
require.False(t, validationResult.PrivateKeyValid, "Private Key is valid")
}

func TestSanitizeTLSSecretWithEscapedNewLine(t *testing.T) {
key := "-----BEGIN PRIVATE KEY-----\\nMIIEvQIBADANBgkqhkiG9w0BAQEFAASCBKcwggSjAgEAAoIBAQCu9t8rBBx0VlnA\nrkIqmjUcdh+nqFje5w4hNCWAxwZYhmLioldyTVGTAoZ/ivP/q2sqKgoLIKu+9LYu\n1k762T5xuhWUfW+BD4mVQuoF6NxJlOW1UgttThXlDD0/MWjQ4CL4+iLhlr7OK3Pp\naQwlak0nlgO4U6eK2vTjkTbgZYXRnRusW7OSfYDlcUSqU7TrmwJuXvug6QobmPWY\nSdqaTG7cLqF+5Voy1w2JKhceQ1DGvUZ8qVFNuUZivytdEsGpnxaTEURK0scgroYX\npP/j1W9CV2eJYGI/YMGrtuMz0SRTP7zh/ZyUs3xtUcpnEkASuELd7GKWQ+JliLnH\nlTqBh/LfAgMBAAECggEAF7pWMKIUQ84UH6E3oEnH1c4f49+yUbnEZ59dTgDFF7X9\n7Mi3Eu1xGH6kA+GxznoOWf+tMK4jweKztFgPCk5HqCpkKSj2ZBUvLkVkBxzPMdFr\nckgn/Du16hlbUwRzN7ngiaMj84kQjlWZQ3h3aQ7osMDTof4CVOlLLcjbSC+sfW0s\nJacmcgP1rYVAcBo2IAo60WAxZBQzKbSHRSVS0g9khUoinfcdKDeyYGze//AiXXXR\nriJ9AYjGTsafV9+VCk6crcznk66eeJXAatecy4Jeu0k5vTta2GAeVkTviNQ4WlQA\nJIFd9uRAUAO5opra/MG9aDi62XcpvDTq8h6Clk5zqQKBgQD27KUIOrIlOnfeosGz\nIjNTMHXrDFG04t05jClSO+6D7mBr7l+t5rS7tPIsKtyibVcYggZKFa8jIYl30dxE\nL3/A8WVRRTZ6AckznzhyIbF1imTzK1sDbNSWwHvoICQ71DPO4nWwD/HnF5owgq9o\npAD/1vnAJLOgO9b/MsN4kip2twKBgQC1ZSRpjRkLgPO5uNxWVjSCf1s0fY9o6Byi\ngNmpsBwQ4by3+VOL+G0JrX1puZV/QL4vNZZ4fNo8xK9b16FmvWwImqadVr6AOzU7\nfMSHo5DUv+mXjA0KHBkdqbWLg3ppGicGYohnotq2lx+2Kubl039OdB+KtXF2OVgx\nnX2PKO19GQKBgGw1aF0i287UwJMgYCJQao2aPxKyY1wRz0DY24LeILhQTpD99ZAP\n+kQIF9ijL+0+XVywHnF47zdGCygnH5ACAMpc/zmOS0FMZw/oRqQ9f7cy3upxpYDq\nwH8P+zzOWRKe+9U+CLUPR8Mt5LQ9kQEaXhW/79L0QoOFtcJATMkZxOIhAoGBAIAe\ntQ48U5E1fnASKsZsUuBNNc0oVi+Bqh/5JEPfGKOv3UyQNLtrNxCb0jXnl7jusKXF\nksb9YGOFhFo5Pk3Dwtd86+u7hggqSZn/sQwgsj4iYsngaKFYYUD7SjgFIGO1zhSL\nac7RTuuiaAqR2M5BiOyPxmuBZmdbb3hzxWhlPwCZAoGAfYn3JgsVTp+DnXHOU9j5\nt1tKHLafi7nRVXmnHbFABmLC0KyJfWZqaCdUoJHRNhggAu918BWA2hlEWjmIG+id\nWiJ4y5oa63TrG/cr9zvdNHBy30KzvpK+aTgV5uPni7iV7URC1EEhghn6stB3LuvB\n0oPRUIeUnfrscWwxggtUGFE=\\n-----END PRIVATE KEY-----\n"
cert := "-----BEGIN CERTIFICATE-----\\nMIIDLzCCAhegAwIBAgIUTDC2e9uCi0ggzWiI7XkkXNWmMY0wDQYJKoZIhvcNAQEL\nBQAwJzELMAkGA1UEBhMCVVMxGDAWBgNVBAMMD0V4YW1wbGUtZm9vLWJhcjAeFw0y\nMzEyMjgxMzUwNDNaFw0yNjEwMTcxMzUwNDNaMCcxCzAJBgNVBAYTAlVTMRgwFgYD\nVQQDDA9FeGFtcGxlLWZvby1iYXIwggEiMA0GCSqGSIb3DQEBAQUAA4IBDwAwggEK\nAoIBAQCu9t8rBBx0VlnArkIqmjUcdh+nqFje5w4hNCWAxwZYhmLioldyTVGTAoZ/\nivP/q2sqKgoLIKu+9LYu1k762T5xuhWUfW+BD4mVQuoF6NxJlOW1UgttThXlDD0/\nMWjQ4CL4+iLhlr7OK3PpaQwlak0nlgO4U6eK2vTjkTbgZYXRnRusW7OSfYDlcUSq\nU7TrmwJuXvug6QobmPWYSdqaTG7cLqF+5Voy1w2JKhceQ1DGvUZ8qVFNuUZivytd\nEsGpnxaTEURK0scgroYXpP/j1W9CV2eJYGI/YMGrtuMz0SRTP7zh/ZyUs3xtUcpn\nEkASuELd7GKWQ+JliLnHlTqBh/LfAgMBAAGjUzBRMB0GA1UdDgQWBBTYsQEqc5CX\nzjBGv/O04Qd5sOu5QjAfBgNVHSMEGDAWgBTYsQEqc5CXzjBGv/O04Qd5sOu5QjAP\nBgNVHRMBAf8EBTADAQH/MA0GCSqGSIb3DQEBCwUAA4IBAQA1L7IsQ9GTFl9GQMGo\n+JOffZxhR9AiwnCPXTMWF8qYC99F39i946wJIkgJN6wm8Rt46gA65EfJw6YdjdB6\n8kjg3CDRDIFn2QRrP4x8tS4EBu9tUkss/2h0I16MEEB9RV8adjH0lkiPwQwP50uW\nwLlwMHw9KsxA1MATzSmBruzW//gyoJFaBKYsYqYa7VKcEyQqKgiQypBN2O01twF3\ntahLFTIeeD0e4fMe++mwJh8rT5sRpCLmFDIoajmLkjj48P7hvgtLFN+vRTqgqViq\nySngIMt75xyXeTm15o7LrEe4B4HkpWt4CbeUW/44HrCUoItuhyea7baGecLx8VoS\nR3xg\\n-----END CERTIFICATE-----\n"

validator := TLSCertValidator{}
validationResult := validator.ValidateCertificate([]byte(cert), []byte(key))

require.True(t, validationResult.CertValid)
require.True(t, validationResult.PrivateKeyValid)
}

func TestSanitizeValidTLSSecret(t *testing.T) {
key := "-----BEGIN PRIVATE KEY-----\nMIIEvQIBADANBgkqhkiG9w0BAQEFAASCBKcwggSjAgEAAoIBAQCu9t8rBBx0VlnA\nrkIqmjUcdh+nqFje5w4hNCWAxwZYhmLioldyTVGTAoZ/ivP/q2sqKgoLIKu+9LYu\n1k762T5xuhWUfW+BD4mVQuoF6NxJlOW1UgttThXlDD0/MWjQ4CL4+iLhlr7OK3Pp\naQwlak0nlgO4U6eK2vTjkTbgZYXRnRusW7OSfYDlcUSqU7TrmwJuXvug6QobmPWY\nSdqaTG7cLqF+5Voy1w2JKhceQ1DGvUZ8qVFNuUZivytdEsGpnxaTEURK0scgroYX\npP/j1W9CV2eJYGI/YMGrtuMz0SRTP7zh/ZyUs3xtUcpnEkASuELd7GKWQ+JliLnH\nlTqBh/LfAgMBAAECggEAF7pWMKIUQ84UH6E3oEnH1c4f49+yUbnEZ59dTgDFF7X9\n7Mi3Eu1xGH6kA+GxznoOWf+tMK4jweKztFgPCk5HqCpkKSj2ZBUvLkVkBxzPMdFr\nckgn/Du16hlbUwRzN7ngiaMj84kQjlWZQ3h3aQ7osMDTof4CVOlLLcjbSC+sfW0s\nJacmcgP1rYVAcBo2IAo60WAxZBQzKbSHRSVS0g9khUoinfcdKDeyYGze//AiXXXR\nriJ9AYjGTsafV9+VCk6crcznk66eeJXAatecy4Jeu0k5vTta2GAeVkTviNQ4WlQA\nJIFd9uRAUAO5opra/MG9aDi62XcpvDTq8h6Clk5zqQKBgQD27KUIOrIlOnfeosGz\nIjNTMHXrDFG04t05jClSO+6D7mBr7l+t5rS7tPIsKtyibVcYggZKFa8jIYl30dxE\nL3/A8WVRRTZ6AckznzhyIbF1imTzK1sDbNSWwHvoICQ71DPO4nWwD/HnF5owgq9o\npAD/1vnAJLOgO9b/MsN4kip2twKBgQC1ZSRpjRkLgPO5uNxWVjSCf1s0fY9o6Byi\ngNmpsBwQ4by3+VOL+G0JrX1puZV/QL4vNZZ4fNo8xK9b16FmvWwImqadVr6AOzU7\nfMSHo5DUv+mXjA0KHBkdqbWLg3ppGicGYohnotq2lx+2Kubl039OdB+KtXF2OVgx\nnX2PKO19GQKBgGw1aF0i287UwJMgYCJQao2aPxKyY1wRz0DY24LeILhQTpD99ZAP\n+kQIF9ijL+0+XVywHnF47zdGCygnH5ACAMpc/zmOS0FMZw/oRqQ9f7cy3upxpYDq\nwH8P+zzOWRKe+9U+CLUPR8Mt5LQ9kQEaXhW/79L0QoOFtcJATMkZxOIhAoGBAIAe\ntQ48U5E1fnASKsZsUuBNNc0oVi+Bqh/5JEPfGKOv3UyQNLtrNxCb0jXnl7jusKXF\nksb9YGOFhFo5Pk3Dwtd86+u7hggqSZn/sQwgsj4iYsngaKFYYUD7SjgFIGO1zhSL\nac7RTuuiaAqR2M5BiOyPxmuBZmdbb3hzxWhlPwCZAoGAfYn3JgsVTp+DnXHOU9j5\nt1tKHLafi7nRVXmnHbFABmLC0KyJfWZqaCdUoJHRNhggAu918BWA2hlEWjmIG+id\nWiJ4y5oa63TrG/cr9zvdNHBy30KzvpK+aTgV5uPni7iV7URC1EEhghn6stB3LuvB\n0oPRUIeUnfrscWwxggtUGFE=\n-----END PRIVATE KEY-----\n"
cert := "-----BEGIN CERTIFICATE-----\nMIIDLzCCAhegAwIBAgIUTDC2e9uCi0ggzWiI7XkkXNWmMY0wDQYJKoZIhvcNAQEL\nBQAwJzELMAkGA1UEBhMCVVMxGDAWBgNVBAMMD0V4YW1wbGUtZm9vLWJhcjAeFw0y\nMzEyMjgxMzUwNDNaFw0yNjEwMTcxMzUwNDNaMCcxCzAJBgNVBAYTAlVTMRgwFgYD\nVQQDDA9FeGFtcGxlLWZvby1iYXIwggEiMA0GCSqGSIb3DQEBAQUAA4IBDwAwggEK\nAoIBAQCu9t8rBBx0VlnArkIqmjUcdh+nqFje5w4hNCWAxwZYhmLioldyTVGTAoZ/\nivP/q2sqKgoLIKu+9LYu1k762T5xuhWUfW+BD4mVQuoF6NxJlOW1UgttThXlDD0/\nMWjQ4CL4+iLhlr7OK3PpaQwlak0nlgO4U6eK2vTjkTbgZYXRnRusW7OSfYDlcUSq\nU7TrmwJuXvug6QobmPWYSdqaTG7cLqF+5Voy1w2JKhceQ1DGvUZ8qVFNuUZivytd\nEsGpnxaTEURK0scgroYXpP/j1W9CV2eJYGI/YMGrtuMz0SRTP7zh/ZyUs3xtUcpn\nEkASuELd7GKWQ+JliLnHlTqBh/LfAgMBAAGjUzBRMB0GA1UdDgQWBBTYsQEqc5CX\nzjBGv/O04Qd5sOu5QjAfBgNVHSMEGDAWgBTYsQEqc5CXzjBGv/O04Qd5sOu5QjAP\nBgNVHRMBAf8EBTADAQH/MA0GCSqGSIb3DQEBCwUAA4IBAQA1L7IsQ9GTFl9GQMGo\n+JOffZxhR9AiwnCPXTMWF8qYC99F39i946wJIkgJN6wm8Rt46gA65EfJw6YdjdB6\n8kjg3CDRDIFn2QRrP4x8tS4EBu9tUkss/2h0I16MEEB9RV8adjH0lkiPwQwP50uW\nwLlwMHw9KsxA1MATzSmBruzW//gyoJFaBKYsYqYa7VKcEyQqKgiQypBN2O01twF3\ntahLFTIeeD0e4fMe++mwJh8rT5sRpCLmFDIoajmLkjj48P7hvgtLFN+vRTqgqViq\nySngIMt75xyXeTm15o7LrEe4B4HkpWt4CbeUW/44HrCUoItuhyea7baGecLx8VoS\nR3xg\n-----END CERTIFICATE-----\n"

validator := TLSCertValidator{}
validationResult := validator.ValidateCertificate([]byte(cert), []byte(key))

require.True(t, validationResult.CertValid)
require.True(t, validationResult.PrivateKeyValid)
}
17 changes: 0 additions & 17 deletions internal/tls/validate.go

This file was deleted.

Loading

0 comments on commit 999330a

Please sign in to comment.