forked from TrueCloudLab/certificates
Strip ports on audience check.
Services might have proxies behind them so we cannot rely on them. Fixes #17
This commit is contained in:
parent
9b87e08faf
commit
7e95fc0e45
6 changed files with 87 additions and 33 deletions
|
@ -5,11 +5,9 @@ import (
|
|||
realx509 "crypto/x509"
|
||||
"encoding/hex"
|
||||
"fmt"
|
||||
"net"
|
||||
"sync"
|
||||
"time"
|
||||
|
||||
"github.com/pkg/errors"
|
||||
"github.com/smallstep/cli/crypto/pemutil"
|
||||
"github.com/smallstep/cli/crypto/x509util"
|
||||
)
|
||||
|
@ -50,15 +48,11 @@ func New(config *Config) (*Authority, error) {
|
|||
}
|
||||
}
|
||||
|
||||
// Define audiences: legacy + possible urls
|
||||
_, port, err := net.SplitHostPort(config.Address)
|
||||
if err != nil {
|
||||
return nil, errors.Wrapf(err, "error parsing %s", config.Address)
|
||||
}
|
||||
// Define audiences: legacy + possible urls without the ports.
|
||||
// The CA might have proxies in front so we cannot rely on the port.
|
||||
audiences := []string{legacyAuthority}
|
||||
for _, name := range config.DNSNames {
|
||||
audiences = append(audiences, fmt.Sprintf("https://%s/sign", name), fmt.Sprintf("https://%s/1.0/sign", name))
|
||||
audiences = append(audiences, fmt.Sprintf("https://%s:%s/sign", name, port), fmt.Sprintf("https://%s:%s/1.0/sign", name, port))
|
||||
}
|
||||
|
||||
var a = &Authority{
|
||||
|
|
|
@ -74,15 +74,6 @@ func TestAuthorityNew(t *testing.T) {
|
|||
err: errors.New("open foo failed: no such file or directory"),
|
||||
}
|
||||
},
|
||||
"fail bad address": func(t *testing.T) *newTest {
|
||||
c, err := LoadConfiguration("../ca/testdata/ca.json")
|
||||
assert.FatalError(t, err)
|
||||
c.Address = "127.0.0.1"
|
||||
return &newTest{
|
||||
config: c,
|
||||
err: errors.New("error parsing 127.0.0.1: address 127.0.0.1: missing port in address"),
|
||||
}
|
||||
},
|
||||
"fail bad password": func(t *testing.T) *newTest {
|
||||
c, err := LoadConfiguration("../ca/testdata/ca.json")
|
||||
assert.FatalError(t, err)
|
||||
|
@ -137,8 +128,8 @@ func TestAuthorityNew(t *testing.T) {
|
|||
|
||||
assert.Equals(t, auth.audiences, []string{
|
||||
"step-certificate-authority",
|
||||
"https://127.0.0.1:0/sign",
|
||||
"https://127.0.0.1:0/1.0/sign",
|
||||
"https://127.0.0.1/sign",
|
||||
"https://127.0.0.1/1.0/sign",
|
||||
})
|
||||
}
|
||||
}
|
||||
|
|
|
@ -4,6 +4,7 @@ import (
|
|||
"crypto/x509"
|
||||
"encoding/asn1"
|
||||
"net/http"
|
||||
"net/url"
|
||||
"time"
|
||||
|
||||
"github.com/pkg/errors"
|
||||
|
@ -15,15 +16,15 @@ type idUsed struct {
|
|||
Subject string `json:"sub,omitempty"`
|
||||
}
|
||||
|
||||
// matchesOne returns true if A and B share at least one element.
|
||||
func matchesOne(as, bs []string) bool {
|
||||
// matchesAudience returns true if A and B share at least one element.
|
||||
func matchesAudience(as, bs []string) bool {
|
||||
if len(bs) == 0 || len(as) == 0 {
|
||||
return false
|
||||
}
|
||||
|
||||
for _, b := range bs {
|
||||
for _, a := range as {
|
||||
if b == a {
|
||||
if b == a || stripPort(a) == stripPort(b) {
|
||||
return true
|
||||
}
|
||||
}
|
||||
|
@ -31,6 +32,17 @@ func matchesOne(as, bs []string) bool {
|
|||
return false
|
||||
}
|
||||
|
||||
// stripPort attempts to strip the port from the given url. If parsing the url
|
||||
// produces errors it will just return the passed argument.
|
||||
func stripPort(rawurl string) string {
|
||||
u, err := url.Parse(rawurl)
|
||||
if err != nil {
|
||||
return rawurl
|
||||
}
|
||||
u.Host = u.Hostname()
|
||||
return u.String()
|
||||
}
|
||||
|
||||
// Authorize authorizes a signature request by validating and authenticating
|
||||
// a OTT that must be sent w/ the request.
|
||||
func (a *Authority) Authorize(ott string) ([]interface{}, error) {
|
||||
|
@ -91,7 +103,7 @@ func (a *Authority) Authorize(ott string) ([]interface{}, error) {
|
|||
}
|
||||
}
|
||||
|
||||
if !matchesOne(claims.Audience, a.audiences) {
|
||||
if !matchesAudience(claims.Audience, a.audiences) {
|
||||
return nil, &apiError{errors.New("authorize: token audience invalid"), http.StatusUnauthorized,
|
||||
errContext}
|
||||
}
|
||||
|
|
|
@ -13,7 +13,7 @@ import (
|
|||
"gopkg.in/square/go-jose.v2/jwt"
|
||||
)
|
||||
|
||||
func TestMatchesOne(t *testing.T) {
|
||||
func TestMatchesAudience(t *testing.T) {
|
||||
type matchesTest struct {
|
||||
a, b []string
|
||||
exp bool
|
||||
|
@ -44,10 +44,47 @@ func TestMatchesOne(t *testing.T) {
|
|||
b: []string{"https://127.0.0.1:0/sign", "https://test.ca.smallstep.com/sign"},
|
||||
exp: true,
|
||||
},
|
||||
"true,portsA": {
|
||||
a: []string{"step-gateway", "https://test.ca.smallstep.com:9000/sign"},
|
||||
b: []string{"https://127.0.0.1:0/sign", "https://test.ca.smallstep.com/sign"},
|
||||
exp: true,
|
||||
},
|
||||
"true,portsB": {
|
||||
a: []string{"step-gateway", "https://test.ca.smallstep.com/sign"},
|
||||
b: []string{"https://127.0.0.1:0/sign", "https://test.ca.smallstep.com:9000/sign"},
|
||||
exp: true,
|
||||
},
|
||||
"true,portsAB": {
|
||||
a: []string{"step-gateway", "https://test.ca.smallstep.com:9000/sign"},
|
||||
b: []string{"https://127.0.0.1:0/sign", "https://test.ca.smallstep.com:8000/sign"},
|
||||
exp: true,
|
||||
},
|
||||
}
|
||||
for name, tc := range tests {
|
||||
t.Run(name, func(t *testing.T) {
|
||||
assert.Equals(t, tc.exp, matchesOne(tc.a, tc.b))
|
||||
assert.Equals(t, tc.exp, matchesAudience(tc.a, tc.b))
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestStripPort(t *testing.T) {
|
||||
type args struct {
|
||||
rawurl string
|
||||
}
|
||||
tests := []struct {
|
||||
name string
|
||||
args args
|
||||
want string
|
||||
}{
|
||||
{"with port", args{"https://ca.smallstep.com:9000/sign"}, "https://ca.smallstep.com/sign"},
|
||||
{"with no port", args{"https://ca.smallstep.com/sign/"}, "https://ca.smallstep.com/sign/"},
|
||||
{"bad url", args{"https://a bad url:9000"}, "https://a bad url:9000"},
|
||||
}
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
if got := stripPort(tt.args.rawurl); got != tt.want {
|
||||
t.Errorf("stripPort() = %v, want %v", got, tt.want)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
|
|
@ -2,6 +2,7 @@ package authority
|
|||
|
||||
import (
|
||||
"encoding/json"
|
||||
"net"
|
||||
"os"
|
||||
"time"
|
||||
|
||||
|
@ -157,6 +158,11 @@ func (c *Config) Validate() error {
|
|||
return errors.New("dnsNames cannot be empty")
|
||||
}
|
||||
|
||||
// Validate address (a port is required)
|
||||
if _, _, err := net.SplitHostPort(c.Address); err != nil {
|
||||
return errors.Errorf("invalid address %s", c.Address)
|
||||
}
|
||||
|
||||
if c.TLS == nil {
|
||||
c.TLS = &DefaultTLSOptions
|
||||
} else {
|
||||
|
|
|
@ -50,10 +50,24 @@ func TestConfigValidate(t *testing.T) {
|
|||
err: errors.New("address cannot be empty"),
|
||||
}
|
||||
},
|
||||
"empty-root": func(t *testing.T) ConfigValidateTest {
|
||||
"invalid-address": func(t *testing.T) ConfigValidateTest {
|
||||
return ConfigValidateTest{
|
||||
config: &Config{
|
||||
Address: "127.0.0.1",
|
||||
Root: "testdata/secrets/root_ca.crt",
|
||||
IntermediateCert: "testdata/secrets/intermediate_ca.crt",
|
||||
IntermediateKey: "testdata/secrets/intermediate_ca_key",
|
||||
DNSNames: []string{"test.smallstep.com"},
|
||||
Password: "pass",
|
||||
AuthorityConfig: ac,
|
||||
},
|
||||
err: errors.New("invalid address 127.0.0.1"),
|
||||
}
|
||||
},
|
||||
"empty-root": func(t *testing.T) ConfigValidateTest {
|
||||
return ConfigValidateTest{
|
||||
config: &Config{
|
||||
Address: "127.0.0.1:443",
|
||||
IntermediateCert: "testdata/secrets/intermediate_ca.crt",
|
||||
IntermediateKey: "testdata/secrets/intermediate_ca_key",
|
||||
DNSNames: []string{"test.smallstep.com"},
|
||||
|
@ -66,7 +80,7 @@ func TestConfigValidate(t *testing.T) {
|
|||
"empty-intermediate-cert": func(t *testing.T) ConfigValidateTest {
|
||||
return ConfigValidateTest{
|
||||
config: &Config{
|
||||
Address: "127.0.0.1",
|
||||
Address: "127.0.0.1:443",
|
||||
Root: "testdata/secrets/root_ca.crt",
|
||||
IntermediateKey: "testdata/secrets/intermediate_ca_key",
|
||||
DNSNames: []string{"test.smallstep.com"},
|
||||
|
@ -79,7 +93,7 @@ func TestConfigValidate(t *testing.T) {
|
|||
"empty-intermediate-key": func(t *testing.T) ConfigValidateTest {
|
||||
return ConfigValidateTest{
|
||||
config: &Config{
|
||||
Address: "127.0.0.1",
|
||||
Address: "127.0.0.1:443",
|
||||
Root: "testdata/secrets/root_ca.crt",
|
||||
IntermediateCert: "testdata/secrets/intermediate_ca.crt",
|
||||
DNSNames: []string{"test.smallstep.com"},
|
||||
|
@ -92,7 +106,7 @@ func TestConfigValidate(t *testing.T) {
|
|||
"empty-dnsNames": func(t *testing.T) ConfigValidateTest {
|
||||
return ConfigValidateTest{
|
||||
config: &Config{
|
||||
Address: "127.0.0.1",
|
||||
Address: "127.0.0.1:443",
|
||||
Root: "testdata/secrets/root_ca.crt",
|
||||
IntermediateCert: "testdata/secrets/intermediate_ca.crt",
|
||||
IntermediateKey: "testdata/secrets/intermediate_ca_key",
|
||||
|
@ -105,7 +119,7 @@ func TestConfigValidate(t *testing.T) {
|
|||
"empty-TLS": func(t *testing.T) ConfigValidateTest {
|
||||
return ConfigValidateTest{
|
||||
config: &Config{
|
||||
Address: "127.0.0.1",
|
||||
Address: "127.0.0.1:443",
|
||||
Root: "testdata/secrets/root_ca.crt",
|
||||
IntermediateCert: "testdata/secrets/intermediate_ca.crt",
|
||||
IntermediateKey: "testdata/secrets/intermediate_ca_key",
|
||||
|
@ -119,7 +133,7 @@ func TestConfigValidate(t *testing.T) {
|
|||
"empty-TLS-values": func(t *testing.T) ConfigValidateTest {
|
||||
return ConfigValidateTest{
|
||||
config: &Config{
|
||||
Address: "127.0.0.1",
|
||||
Address: "127.0.0.1:443",
|
||||
Root: "testdata/secrets/root_ca.crt",
|
||||
IntermediateCert: "testdata/secrets/intermediate_ca.crt",
|
||||
IntermediateKey: "testdata/secrets/intermediate_ca_key",
|
||||
|
@ -134,7 +148,7 @@ func TestConfigValidate(t *testing.T) {
|
|||
"custom-tls-values": func(t *testing.T) ConfigValidateTest {
|
||||
return ConfigValidateTest{
|
||||
config: &Config{
|
||||
Address: "127.0.0.1",
|
||||
Address: "127.0.0.1:443",
|
||||
Root: "testdata/secrets/root_ca.crt",
|
||||
IntermediateCert: "testdata/secrets/intermediate_ca.crt",
|
||||
IntermediateKey: "testdata/secrets/intermediate_ca_key",
|
||||
|
@ -163,7 +177,7 @@ func TestConfigValidate(t *testing.T) {
|
|||
"tls-min>max": func(t *testing.T) ConfigValidateTest {
|
||||
return ConfigValidateTest{
|
||||
config: &Config{
|
||||
Address: "127.0.0.1",
|
||||
Address: "127.0.0.1:443",
|
||||
Root: "testdata/secrets/root_ca.crt",
|
||||
IntermediateCert: "testdata/secrets/intermediate_ca.crt",
|
||||
IntermediateKey: "testdata/secrets/intermediate_ca_key",
|
||||
|
|
Loading…
Reference in a new issue