Skip to content
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

attempts to probe advertise address on startup to ensure the SANS is correct #2723

Merged
merged 7 commits into from
Feb 6, 2025
Merged

attempts to probe advertise address on startup to ensure the SANS is correct #2723

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
94ee830
fixes #2723 - check router and controller advertised addresses compar…
dovholuknf Feb 5, 2025
2527896
fixes #2723 - don't verify cert/addresses when loadIdentity=false
dovholuknf Feb 5, 2025
bb04bbe
fixes #2723 - don't check ctrl endpoint using router certs
dovholuknf Feb 6, 2025
af644ec
fixes #2723 - advertiese -> advertise
dovholuknf Feb 6, 2025
bd3be1b
fixes #2722 - cleanup/rework to use xweb
dovholuknf Feb 6, 2025
52520f1
fixes #2722 - actually remove unused code
dovholuknf Feb 6, 2025
834cf09
placating linter
dovholuknf Feb 6, 2025
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
56 changes: 56 additions & 0 deletions controller/config/config.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -585,6 +585,14 @@ func LoadConfig(path string) (*Config, error) {
return nil, fmt.Errorf("error loading channel options for [ctrl/options] (%v)", err)
}
}
if value != nil {
m := value.(map[interface{}]interface{})
a := strings.TrimPrefix(m["advertiseAddress"].(string), "tls:")
v := controllerConfig.Id.ValidFor(strings.Split(a, ":")[0])
Copy link
Member

@andrewpmartinez andrewpmartinez Feb 6, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

I think this can be removed.

This will fail if the advertised address goes to an xweb server that uses an alternate identity.

Since xweb now verifies its public address is represented by a SAN on its identity, we can rely on that to make sure the xweb config is sane.

There is already code to check that the advertise address is present on an xweb server that 1) shares the same address and 2) has the edge-api.

Copy link
Member Author

@dovholuknf dovholuknf Feb 6, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

what about routers or controllers connecting? those aren't xweb, right?

Copy link
Member Author

Choose a reason for hiding this comment

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

but - routers/controllers are always specified outside of this config file? I just didn't know (still don't) if this address is used outside of xweb

Copy link
Member

Choose a reason for hiding this comment

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

This section is only for controllers xweb powered APIs. So it won't affect anything else.

Copy link
Member

Choose a reason for hiding this comment

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

I see your point about the ctrl listener using the root identity though. Which is a reasonable check.

if v != nil {
pfxlog.Logger().Fatalf("provided value for ctrl/options/advertiseAddress is invalid (%v)", v)
}
}
}
if controllerConfig.Raft != nil && controllerConfig.Raft.AdvertiseAddress == nil {
return nil, errors.New("[ctrl/options/advertiseAddress] is required when raft is enabled")
Expand Down Expand Up @@ -720,6 +728,14 @@ func LoadConfig(path string) (*Config, error) {
}
}

bpValidation := validateBindPoints(cfgmap)
if len(bpValidation) > 0 {
for _, bp := range bpValidation {
pfxlog.Logger().Errorf("invalid address in bindPoint: %v", bp)
}
pfxlog.Logger().Fatal("bindPoints validation failed")
}

edgeConfig, err := LoadEdgeConfigFromMap(cfgmap)
if err != nil {
return nil, err
Expand All @@ -728,6 +744,46 @@ func LoadConfig(path string) (*Config, error) {

return controllerConfig, nil
}
func validateBindPoints(m map[interface{}]interface{}) []error {
dovholuknf marked this conversation as resolved.
Show resolved Hide resolved
var errs []error

if webList, ok := m["web"].([]interface{}); ok {
for _, entry := range webList {
if entryMap, ok := entry.(map[interface{}]interface{}); ok {
if bindPoints, found := entryMap["bindPoints"].([]interface{}); found {

if value, found := entryMap["identity"]; found {
subMap := value.(map[interface{}]interface{})
identityConfig, err1 := identity.NewConfigFromMapWithPathContext(subMap, "identity")

if err1 != nil {
errs = append(errs, err1)
continue
}
id2, err2 := identity.LoadIdentity(*identityConfig)
if err2 != nil {
errs = append(errs, err2)
continue
}

for _, bp := range bindPoints {
if bpMap, ok := bp.(map[interface{}]interface{}); ok {
if address, exists := bpMap["address"].(string); exists {
err3 := id2.ValidFor(strings.Split(address, ":")[0])
if err3 != nil {
errs = append(errs, err3)
}
}
}
}
}
}
}
}
}

return errs
}

// isSelfSigned checks if the given certificate is self-signed.
func isSelfSigned(cert *x509.Certificate) (bool, error) {
Expand Down
10 changes: 5 additions & 5 deletions go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,9 +55,9 @@ require (
github.com/openziti/agent v1.0.23
github.com/openziti/channel/v3 v3.0.26
github.com/openziti/cobra-to-md v1.0.1
github.com/openziti/edge-api v0.26.38
github.com/openziti/edge-api v0.26.39
github.com/openziti/foundation/v2 v2.0.56
github.com/openziti/identity v1.0.94
github.com/openziti/identity v1.0.95
github.com/openziti/jwks v1.0.6
github.com/openziti/metrics v1.2.65
github.com/openziti/runzmd v1.0.59
Expand Down Expand Up @@ -90,7 +90,7 @@ require (
golang.org/x/net v0.34.0
golang.org/x/oauth2 v0.25.0
golang.org/x/sync v0.10.0
golang.org/x/sys v0.29.0
golang.org/x/sys v0.30.0
golang.org/x/text v0.21.0
google.golang.org/protobuf v1.36.4
gopkg.in/AlecAivazis/survey.v1 v1.8.8
Expand Down Expand Up @@ -150,7 +150,7 @@ require (
github.com/lufia/plan9stats v0.0.0-20240909124753-873cd0166683 // indirect
github.com/magiconair/properties v1.8.7 // indirect
github.com/mailru/easyjson v0.7.7 // indirect
github.com/mattn/go-colorable v0.1.13 // indirect
github.com/mattn/go-colorable v0.1.14 // indirect
github.com/mattn/go-isatty v0.0.20 // indirect
github.com/mattn/go-runewidth v0.0.15 // indirect
github.com/mattn/go-tty v0.0.3 // indirect
Expand Down Expand Up @@ -200,7 +200,7 @@ require (
go.uber.org/multierr v1.9.0 // indirect
golang.org/x/image v0.18.0 // indirect
golang.org/x/mod v0.22.0 // indirect
golang.org/x/term v0.28.0 // indirect
golang.org/x/term v0.29.0 // indirect
golang.org/x/tools v0.28.0 // indirect
gopkg.in/ini.v1 v1.67.0 // indirect
gopkg.in/natefinch/lumberjack.v2 v2.0.0 // indirect
Expand Down
22 changes: 10 additions & 12 deletions go.sum
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -499,8 +499,8 @@ github.com/mattn/go-colorable v0.1.4/go.mod h1:U0ppj6V5qS13XJ6of8GYAs25YV2eR4EVc
github.com/mattn/go-colorable v0.1.7/go.mod h1:u6P/XSegPjTcexA+o6vUJrdnUu04hMope9wVRipJSqc=
github.com/mattn/go-colorable v0.1.9/go.mod h1:u6P/XSegPjTcexA+o6vUJrdnUu04hMope9wVRipJSqc=
github.com/mattn/go-colorable v0.1.12/go.mod h1:u5H1YNBxpqRaxsYJYSkiCWKzEfiAb1Gb520KVy5xxl4=
github.com/mattn/go-colorable v0.1.13 h1:fFA4WZxdEF4tXPZVKMLwD8oUnCTTo08duU7wxecdEvA=
github.com/mattn/go-colorable v0.1.13/go.mod h1:7S9/ev0klgBDR4GtXTXX8a3vIGJpMovkB8vQcUbaXHg=
github.com/mattn/go-colorable v0.1.14 h1:9A9LHSqF/7dyVVX6g0U9cwm9pG3kP9gSzcuIPHPsaIE=
github.com/mattn/go-colorable v0.1.14/go.mod h1:6LmQG8QLFO4G5z1gPvYEzlUgJ2wF+stgPZH1UqBm1s8=
github.com/mattn/go-isatty v0.0.3/go.mod h1:M+lRXTBqGeGNdLjl/ufCoiOlB5xdOkqRJdNxMWT7Zi4=
github.com/mattn/go-isatty v0.0.4/go.mod h1:M+lRXTBqGeGNdLjl/ufCoiOlB5xdOkqRJdNxMWT7Zi4=
github.com/mattn/go-isatty v0.0.5/go.mod h1:Iq45c/XA43vh69/j3iqttzPXn0bhXyGjM0Hdxcsrc5s=
Expand All @@ -511,7 +511,6 @@ github.com/mattn/go-isatty v0.0.10/go.mod h1:qgIWMr58cqv1PHHyhnkY9lrL7etaEgOFcME
github.com/mattn/go-isatty v0.0.11/go.mod h1:PhnuNfih5lzO57/f3n+odYbM4JtupLOxQOAqxQCu2WE=
github.com/mattn/go-isatty v0.0.12/go.mod h1:cbi8OIDigv2wuxKPP5vlRcQ1OAZbq2CE4Kysco4FUpU=
github.com/mattn/go-isatty v0.0.14/go.mod h1:7GGIvUiUoEMVVmxf/4nioHXj79iQHKdU27kJ6hsGG94=
github.com/mattn/go-isatty v0.0.16/go.mod h1:kYGgaQfpe5nmfYZH+SKPsOc2e4SrIfOl2e/yFXSvRLM=
github.com/mattn/go-isatty v0.0.20 h1:xfD0iDuEKnDkl03q4limB+vH+GxLEtL/jb4xVJSWWEY=
github.com/mattn/go-isatty v0.0.20/go.mod h1:W+V8PltTTMOvKvAeJH7IuucS94S2C6jfK/D7dTCTo3Y=
github.com/mattn/go-runewidth v0.0.6/go.mod h1:H031xJmbD/WCDINGzjvQ9THkh0rPKHF+m2gUSrubnMI=
Expand Down Expand Up @@ -595,12 +594,12 @@ github.com/openziti/cobra-to-md v1.0.1 h1:WRinNoIRmwWUSJm+pSNXMjOrtU48oxXDZgeCYQ
github.com/openziti/cobra-to-md v1.0.1/go.mod h1:FjCpk/yzHF7/r28oSTNr5P57yN5VolpdAtS/g7KNi2c=
github.com/openziti/dilithium v0.3.5 h1:+envGNzxc3OyVPiuvtxivQmCsOjdZjtOMLpQBeMz7eM=
github.com/openziti/dilithium v0.3.5/go.mod h1:XONq1iK6te/WwNzkgZHfIDHordMPqb0hMwJ8bs9EfSk=
github.com/openziti/edge-api v0.26.38 h1:3xDWC5SFn3qUVR428TIBpRc2lrjVV7Gz0Rx4pQx0JSg=
github.com/openziti/edge-api v0.26.38/go.mod h1:sYHVpm26Jr1u7VooNJzTb2b2nGSlmCHMnbGC8XfWSng=
github.com/openziti/edge-api v0.26.39 h1:4hb1RqjgpaTJorvbWTL6f2QcjJDn4BXDbLwes8DpM2U=
github.com/openziti/edge-api v0.26.39/go.mod h1:sYHVpm26Jr1u7VooNJzTb2b2nGSlmCHMnbGC8XfWSng=
github.com/openziti/foundation/v2 v2.0.56 h1:YXqBmkrN0fYr3TqIlWZSZGluE2QpJxlA29Z6okZyQ5I=
github.com/openziti/foundation/v2 v2.0.56/go.mod h1:f12R1pwEod348qONZr6esZgackX1ScLGDcEyPF2G5/w=
github.com/openziti/identity v1.0.94 h1:nF4etu/5LmOlbT24lpSKq9p+90A9jeyLr5U23LemgD4=
github.com/openziti/identity v1.0.94/go.mod h1:3VGYqa9E26zPPA8lJwE7eUPvRH2Oz8ZAd46cUCWKz/M=
github.com/openziti/identity v1.0.95 h1:1IpYBCgmqmSscnGFHoadHSJPfmViqf7Xl6gvE7fLLmU=
github.com/openziti/identity v1.0.95/go.mod h1:3VGYqa9E26zPPA8lJwE7eUPvRH2Oz8ZAd46cUCWKz/M=
github.com/openziti/jwks v1.0.6 h1:PR+9OVaMO8oHEoVQmHqeUBExWwLWyODEGJQK2DXHaqE=
github.com/openziti/jwks v1.0.6/go.mod h1:t4xxq8vlXGsPn29kiQVnZBBDDnEoOFqtJoHibkJunQQ=
github.com/openziti/metrics v1.2.65 h1:Jhhbds+BUbywfspxcb9oyz9p9LI/oERT9lbeDpnNpmY=
Expand Down Expand Up @@ -1103,16 +1102,15 @@ golang.org/x/sys v0.0.0-20220503163025-988cb79eb6c6/go.mod h1:oPkhp1MJrh7nUepCBc
golang.org/x/sys v0.0.0-20220520151302-bc2c85ada10a/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
golang.org/x/sys v0.0.0-20220715151400-c0bba94af5f8/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
golang.org/x/sys v0.0.0-20220722155257-8c9f86f7a55f/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
golang.org/x/sys v0.0.0-20220811171246-fbc7d0a398ab/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
golang.org/x/sys v0.1.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
golang.org/x/sys v0.6.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
golang.org/x/sys v0.29.0 h1:TPYlXGxvx1MGTn2GiZDhnjPA9wZzZeGKHHmKhHYvgaU=
golang.org/x/sys v0.29.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA=
golang.org/x/sys v0.30.0 h1:QjkSwP/36a20jFYWkSue1YwXzLmsV5Gfq7Eiy72C1uc=
golang.org/x/sys v0.30.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA=
golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1/go.mod h1:bj7SfCRtBDWHUb9snDiAeCFNEtKQo2Wmx5Cou7ajbmo=
golang.org/x/term v0.0.0-20210927222741-03fcf44c2211/go.mod h1:jbD1KX2456YbFQfuXm/mYQcufACuNUgVhRMnK/tPxf8=
golang.org/x/term v0.1.0/go.mod h1:jbD1KX2456YbFQfuXm/mYQcufACuNUgVhRMnK/tPxf8=
golang.org/x/term v0.28.0 h1:/Ts8HFuMR2E6IP/jlo7QVLZHggjKQbhu/7H0LJFr3Gg=
golang.org/x/term v0.28.0/go.mod h1:Sw/lC2IAUZ92udQNf3WodGtn4k/XoLyZoh8v/8uiwek=
golang.org/x/term v0.29.0 h1:L6pJp37ocefwRRtYPKSWOWzOtWSxVajvz2ldH/xi3iU=
golang.org/x/term v0.29.0/go.mod h1:6bl4lRlvVuDgSf3179VpIxBF0o10JUpXWOnI7nErv7s=
golang.org/x/text v0.0.0-20170915032832-14c0d48ead0c/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ=
golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ=
golang.org/x/text v0.3.1-0.20180807135948-17ff2d5776d2/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ=
Expand Down
36 changes: 36 additions & 0 deletions router/config.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -880,6 +880,42 @@ func LoadConfigWithOptions(path string, loadIdentity bool) (*Config, error) {
return nil, err
}

if loadIdentity {
var errs []error
for _, c := range cfg.Link.Listeners {
a := c["advertise"]
if a != nil {
// should start with tls:
Copy link
Member

Choose a reason for hiding this comment

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

"Should" or "can?" My naive observation has been that the tls: prefix is implied if omitted, and that we don't currently support any alternatives, so there's not currently any ambiguity, but there will be ambiguity in the future if we support another on-the-wire protocol, e.g., dtls.

addy := strings.TrimPrefix(a.(string), "tls:")
addy = strings.Split(addy, ":")[0]
e := cfg.Id.ValidFor(addy)
if e != nil {
errs = append(errs, fmt.Errorf("invalid link.listeners.advertise: %s, error: %v", a.(string), e))
}
}
}

for _, c := range cfg.Listeners {
opts := c.options["options"]
if opts != nil {
optOpts := opts.(map[interface{}]interface{})
o := optOpts["advertise"]
if o != nil {
// should start with tls:
addy := strings.TrimPrefix(o.(string), "tls:")
addy = strings.Split(addy, ":")[0]
e := cfg.Id.ValidFor(addy)
if e != nil {
errs = append(errs, fmt.Errorf("invalid listeners.binding.advertise: %s, error: %v", o.(string), e))
}
}
}
}

if len(errs) > 0 {
pfxlog.Logger().Fatalf("one or more advertise addresses are invalid: %v", errs)
}
}
return cfg, nil
}

Expand Down
2 changes: 2 additions & 0 deletions router/xgress_edge/certchecker_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -407,6 +407,8 @@ func (s *SimpleTestIdentity) CA() *x509.CertPool {
return s.CertPool
}

func (s *SimpleTestIdentity) ValidFor(_ string) error { return nil }

func (s *SimpleTestIdentity) ServerTLSConfig() *tls.Config {
var certs []tls.Certificate

Expand Down
Loading