From 638cf96f1945f4f81352f86fa7f022fb2e6951f3 Mon Sep 17 00:00:00 2001 From: Magnus Kaiser Date: Mon, 28 Feb 2022 16:27:00 +0100 Subject: [PATCH] added suggestions from PR --- cmd/thanos/receive.go | 6 +-- pkg/receive/handler.go | 97 ++++++++++++++++++++++++------------------ 2 files changed, 58 insertions(+), 45 deletions(-) diff --git a/cmd/thanos/receive.go b/cmd/thanos/receive.go index 2fd4f8b86ac..8d0cd52ab77 100644 --- a/cmd/thanos/receive.go +++ b/cmd/thanos/receive.go @@ -191,7 +191,7 @@ func runReceive( Registry: reg, Endpoint: conf.endpoint, TenantHeader: conf.tenantHeader, - TenantAttribute: conf.tenantAttribute, + TenantField: conf.tenantField, DefaultTenantID: conf.defaultTenantID, ReplicaHeader: conf.replicaHeader, ReplicationFactor: conf.replicationFactor, @@ -705,7 +705,7 @@ type receiveConfig struct { refreshInterval *model.Duration endpoint string tenantHeader string - tenantAttribute string + tenantField string tenantLabelName string defaultTenantID string replicaHeader string @@ -769,7 +769,7 @@ func (rc *receiveConfig) registerFlag(cmd extkingpin.FlagClause) { cmd.Flag("receive.tenant-header", "HTTP header to determine tenant for write requests.").Default(receive.DefaultTenantHeader).StringVar(&rc.tenantHeader) - cmd.Flag("receive.tenant-certificate-attribute", "Use TLS client certificate's attribute to determine tenant for write requests. Must be one of organization, organizationalUnit or commonName.").Default("").EnumVar(&rc.tenantAttribute, "", "organization", "organizationalUnit", "commonName") + cmd.Flag("receive.tenant-certificate-field", "Use TLS client's certificate field to determine tenant for write requests. Must be one of "+receive.CertificateFieldOrganization+", "+receive.CertificateFieldOrganizationalUnit+" or "+receive.CertificateFieldCommonName+". This setting takes will cause the receive.tenant-header flag value to be ignored.").Default("").EnumVar(&rc.tenantField, "", receive.CertificateFieldOrganization, receive.CertificateFieldOrganizationalUnit, receive.CertificateFieldCommonName) cmd.Flag("receive.default-tenant-id", "Default tenant ID to use when none is provided via a header.").Default(receive.DefaultTenant).StringVar(&rc.defaultTenantID) diff --git a/pkg/receive/handler.go b/pkg/receive/handler.go index 364998e4cd5..7667211c990 100644 --- a/pkg/receive/handler.go +++ b/pkg/receive/handler.go @@ -57,6 +57,13 @@ const ( labelError = "error" ) +// Allowed fields in client certificates. +const ( + CertificateFieldOrganization = "organization" + CertificateFieldOrganizationalUnit = "organizationalUnit" + CertificateFieldCommonName = "commonName" +) + var ( // errConflict is returned whenever an operation fails due to any conflict-type error. errConflict = errors.New("conflict") @@ -72,7 +79,7 @@ type Options struct { ListenAddress string Registry prometheus.Registerer TenantHeader string - TenantAttribute string + TenantField string DefaultTenantID string ReplicaHeader string Endpoint string @@ -340,47 +347,11 @@ func (h *Handler) receiveHTTP(w http.ResponseWriter, r *http.Request) { tenant = h.options.DefaultTenantID } - // Checking certs for possible value. - if h.options.TenantAttribute != "" { - if len(r.TLS.PeerCertificates) > 0 { - // First cert is the leaf authenticated against. - cert := r.TLS.PeerCertificates[0] - - switch h.options.TenantAttribute { - - case "organization": - if len(cert.Subject.Organization) > 0 { - tenant = cert.Subject.Organization[0] - } else { - http.Error(w, "could not get organization attribute from client cert", http.StatusBadRequest) - return - } - - case "organizationalUnit": - if len(cert.Subject.OrganizationalUnit) > 0 { - tenant = cert.Subject.OrganizationalUnit[0] - } else { - http.Error(w, "could not get organizationalUnit attribute from client cert", http.StatusBadRequest) - return - } - - case "commonName": - if cert.Subject.CommonName != "" { - tenant = cert.Subject.CommonName - } else { - http.Error(w, "could not get commonName attribute from client cert", http.StatusBadRequest) - return - } - - default: - // Unknown/unsupported attribute requested, can't continue. - level.Error(h.logger).Log("err", err, "msg", "tls client cert attribute requested is not supported") - http.Error(w, err.Error(), http.StatusInternalServerError) - return - } - - } else { - http.Error(w, "could not get required certificate attribute from client cert", http.StatusBadRequest) + if h.options.TenantField != "" { + tenant, err = h.getTenantFromCertificate(r) + if err != nil { + // This must hard fail to ensure hard tenancy when feature is enabled. + http.Error(w, err.Error(), http.StatusBadRequest) return } } @@ -845,3 +816,45 @@ func (p *peerGroup) get(ctx context.Context, addr string) (storepb.WriteableStor p.cache[addr] = client return client, nil } + +// GetTenantFromCertificate extracts the tenant value from a client's presented certificate. The x509 field to use as +// value can be configured with Options.TenantField. An error is returned when the extraction has not succeeded. +func (h *Handler) getTenantFromCertificate(r *http.Request) (string, error) { + var tenant string + + if len(r.TLS.PeerCertificates) == 0 { + return "", errors.New("could not get required certificate field from client cert") + } + + // First cert is the leaf authenticated against. + cert := r.TLS.PeerCertificates[0] + + switch h.options.TenantField { + + case CertificateFieldOrganization: + if len(cert.Subject.Organization) > 0 { + tenant = cert.Subject.Organization[0] + } else { + return "", errors.New("could not get organization field from client cert") + } + + case CertificateFieldOrganizationalUnit: + if len(cert.Subject.OrganizationalUnit) > 0 { + tenant = cert.Subject.OrganizationalUnit[0] + } else { + return "", errors.New("could not get organizationalUnit field from client cert") + } + + case CertificateFieldCommonName: + if cert.Subject.CommonName != "" { + tenant = cert.Subject.CommonName + } else { + return "", errors.New("could not get commonName field from client cert") + } + + default: + return "", errors.New("tls client cert field requested is not supported") + } + + return tenant, nil +}