Skip to content

observer: Fix possible panic in TLSProbe check #8836

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
beautifulentropy wants to merge 1 commit into
base: main
Choose a base branch
from
+9 −4
Open

observer: Fix possible panic in TLSProbe check #8836

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 9 additions & 4 deletions observer/probers/tls/tls.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"context"
"crypto/tls"
"crypto/x509"
"crypto/x509/pkix"
"encoding/base64"
"errors"
"fmt"
Expand Down Expand Up @@ -139,8 +140,12 @@ func checkCRL(ctx context.Context, cert, issuer *x509.Certificate, want int) (bo

// Return an error if the root settings are nonempty and do not match the
// expected root.
func (p TLSProbe) checkRoot(rootOrg, rootCN string) error {
if (p.rootCN == "" && p.rootOrg == "") || (rootOrg == p.rootOrg && rootCN == p.rootCN) {
func (p TLSProbe) checkRoot(root pkix.Name) error {
var rootOrg string
if len(root.Organization) > 0 {

@aarongable aarongable Jul 2, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd say if len(root.Organization) != 1 { return errors.New("...") }, and then proceed with root.Organization[0] after that.

rootOrg = root.Organization[0]
}
if (p.rootCN == "" && p.rootOrg == "") || (rootOrg == p.rootOrg && root.CommonName == p.rootCN) {
return nil
}
return fmt.Errorf("Expected root does not match.")
Expand Down Expand Up @@ -205,7 +210,7 @@ func (p TLSProbe) probeExpired(ctx context.Context) error {
}

root := peers[len(peers)-1].Issuer
err = p.checkRoot(root.Organization[0], root.CommonName)
err = p.checkRoot(root)
Comment on lines 212 to +213

@aarongable aarongable Jul 2, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The root variable is only used in one place now; inline it into p.checkRoot(peers[len(peers)-1].Issuer).

if err != nil {
p.exportMetrics(peers[0], rootDidNotMatch)
return err
Expand Down Expand Up @@ -257,7 +262,7 @@ func (p TLSProbe) probeUnexpired(ctx context.Context) error {
// tls.Dialer.DialContext is documented to always return *tls.Conn
peers := conn.(*tls.Conn).ConnectionState().PeerCertificates
root := peers[len(peers)-1].Issuer
err = p.checkRoot(root.Organization[0], root.CommonName)
err = p.checkRoot(root)
if err != nil {
p.exportMetrics(peers[0], rootDidNotMatch)
return err
Expand Down