Require verified certs for portal redirect hosts
AddCustomDomain only validates the domain's format before inserting the row; certificate issuance then runs asynchronously. Every host that row resolved to was accepted by the OIDC, magic-link, and compliance-portal OAuth `continue` redirect allowlists, so anyone could self-register an org, claim an arbitrary domain, and have users redirected there right after a real login. Found while re-checking GHSA-r9mf-88r7-g6j9 against the compliance portal rework: the original session-transfer leak is gone, but this open redirect on the same allowlist was not. Gate those allowlists on the domain's certificate having reached Active or Renewing status, which only happens once DNS has pointed at Probo's edge and an ACME challenge has actually succeeded. Signed-off-by: Bryan Frimin <bryan@probo.com>
This commit is contained in:
@@ -140,6 +140,24 @@ func (s *Service) RemoveCustomDomain(
|
||||
)
|
||||
}
|
||||
|
||||
func (s *Service) IsCustomDomainVerified(ctx context.Context, host string) (bool, error) {
|
||||
certificate, err := s.certManager.GetByHostname(ctx, host)
|
||||
if err != nil {
|
||||
if errors.Is(err, coredata.ErrResourceNotFound) {
|
||||
return false, nil
|
||||
}
|
||||
|
||||
return false, fmt.Errorf("cannot load certificate: %w", err)
|
||||
}
|
||||
|
||||
switch certificate.Status {
|
||||
case coredata.CertificateStatusActive, coredata.CertificateStatusRenewing:
|
||||
return true, nil
|
||||
default:
|
||||
return false, nil
|
||||
}
|
||||
}
|
||||
|
||||
// GetDomain returns a custom domain by ID.
|
||||
func (s *Service) GetDomain(
|
||||
ctx context.Context,
|
||||
|
||||
@@ -220,6 +220,20 @@ func (s *Service) GetPortalByDomainName(ctx context.Context, domain string) (*co
|
||||
return compliancePage, err
|
||||
}
|
||||
|
||||
func (s *Service) IsVerifiedRedirectHost(ctx context.Context, host string) bool {
|
||||
if _, err := s.GetPortalByDomainName(ctx, host); err != nil {
|
||||
return false
|
||||
}
|
||||
|
||||
verified, err := s.management.IsCustomDomainVerified(ctx, host)
|
||||
if err != nil {
|
||||
s.logger.ErrorCtx(ctx, "cannot check custom domain verification", log.Error(err), log.String("host", host))
|
||||
return false
|
||||
}
|
||||
|
||||
return verified
|
||||
}
|
||||
|
||||
// GetPortalEmailPresenterConfigByOrganizationID resolves the emails.PresenterConfig for
|
||||
// the compliance page that belongs to the given organization. This is used by the
|
||||
// esign certificate worker which needs per-org branding at render time.
|
||||
|
||||
@@ -257,9 +257,7 @@ func NewServer(cfg Config) (*Server, error) {
|
||||
return true
|
||||
}
|
||||
|
||||
_, err := cfg.Visitor.GetPortalByDomainName(ctx, host)
|
||||
|
||||
return err == nil
|
||||
return cfg.Visitor.IsVerifiedRedirectHost(ctx, host)
|
||||
},
|
||||
cfg.GraphQLLimits,
|
||||
),
|
||||
|
||||
@@ -67,8 +67,7 @@ func NewMux(cfg MuxConfig) (http.Handler, error) {
|
||||
r.Get("/sitemap.xml", markdownHandler.HandleSitemap)
|
||||
|
||||
allowedHost := func(ctx context.Context, host string) bool {
|
||||
_, err := cfg.Visitor.GetPortalByDomainName(ctx, host)
|
||||
return err == nil
|
||||
return cfg.Visitor.IsVerifiedRedirectHost(ctx, host)
|
||||
}
|
||||
|
||||
oauthInitiateHandler := NewOAuthInitiateHandler(
|
||||
|
||||
Reference in New Issue
Block a user