1
0
mirror of https://blitiri.com.ar/repos/chasquid synced 2025-12-17 14:37:02 +00:00

sts: Treat missing/empty "mx" list as invalid

The "mx" field is required, a policy without it is invalid, so add a
check for it.

See
https://mailarchive.ietf.org/arch/msg/uta/Omqo1Bw6rJbrTMl2Zo69IJr35Qo
for more background, in particular the following paragraph:

> The "mx" field is required, so if it is missing, the policy is invalid
> and should not be honored. (It doesn't make sense to honor the policy
> anyway, I would say, since a policy without allowed MXs is essentially a
> way of saying, "There should be TLS and the server identity should match
> the MX, whatever the MX is." I guess this prevents SSL stripping, but
> doesn't prevent DNS injection, so it's of relatively little value.)
This commit is contained in:
Alberto Bertogli
2017-01-05 14:45:50 -03:00
parent 933ab54cd8
commit fe00750e39
2 changed files with 13 additions and 4 deletions

View File

@@ -58,6 +58,7 @@ var (
ErrUnknownVersion = errors.New("unknown policy version") ErrUnknownVersion = errors.New("unknown policy version")
ErrInvalidMaxAge = errors.New("invalid max_age") ErrInvalidMaxAge = errors.New("invalid max_age")
ErrInvalidMode = errors.New("invalid mode") ErrInvalidMode = errors.New("invalid mode")
ErrInvalidMX = errors.New("invalid mx")
) )
// Check that the policy contents are valid. // Check that the policy contents are valid.
@@ -73,13 +74,18 @@ func (p *Policy) Check() error {
return ErrInvalidMode return ErrInvalidMode
} }
// "mx" field is required, and the policy is invalid if it's not present.
// https://mailarchive.ietf.org/arch/msg/uta/Omqo1Bw6rJbrTMl2Zo69IJr35Qo
if len(p.MXs) == 0 {
return ErrInvalidMX
}
return nil return nil
} }
// MXMatches checks if the given MX is allowed, according to the policy. // MXMatches checks if the given MX is allowed, according to the policy.
// https://tools.ietf.org/html/draft-ietf-uta-mta-sts-02#section-4.1 // https://tools.ietf.org/html/draft-ietf-uta-mta-sts-02#section-4.1
func (p *Policy) MXIsAllowed(mx string) bool { func (p *Policy) MXIsAllowed(mx string) bool {
// TODO: Clarify how we should treat an empty MX list.
for _, pattern := range p.MXs { for _, pattern := range p.MXs {
if matchDomain(mx, pattern) { if matchDomain(mx, pattern) {
return true return true

View File

@@ -24,10 +24,10 @@ func TestParsePolicy(t *testing.T) {
func TestCheckPolicy(t *testing.T) { func TestCheckPolicy(t *testing.T) {
validPs := []Policy{ validPs := []Policy{
{Version: "STSv1", Mode: "enforce", MaxAge: 1 * time.Hour}, {Version: "STSv1", Mode: "enforce", MaxAge: 1 * time.Hour,
{Version: "STSv1", Mode: "report", MaxAge: 1 * time.Hour},
{Version: "STSv1", Mode: "report", MaxAge: 1 * time.Hour,
MXs: []string{"mx1", "mx2"}}, MXs: []string{"mx1", "mx2"}},
{Version: "STSv1", Mode: "report", MaxAge: 1 * time.Hour,
MXs: []string{"mx1"}},
} }
for i, p := range validPs { for i, p := range validPs {
if err := p.Check(); err != nil { if err := p.Check(); err != nil {
@@ -42,6 +42,9 @@ func TestCheckPolicy(t *testing.T) {
{Policy{Version: "STSv2"}, ErrUnknownVersion}, {Policy{Version: "STSv2"}, ErrUnknownVersion},
{Policy{Version: "STSv1"}, ErrInvalidMaxAge}, {Policy{Version: "STSv1"}, ErrInvalidMaxAge},
{Policy{Version: "STSv1", MaxAge: 1, Mode: "blah"}, ErrInvalidMode}, {Policy{Version: "STSv1", MaxAge: 1, Mode: "blah"}, ErrInvalidMode},
{Policy{Version: "STSv1", MaxAge: 1, Mode: "enforce"}, ErrInvalidMX},
{Policy{Version: "STSv1", MaxAge: 1, Mode: "enforce", MXs: []string{}},
ErrInvalidMX},
} }
for i, c := range invalid { for i, c := range invalid {
if err := c.p.Check(); err != c.expected { if err := c.p.Check(); err != c.expected {