"veyron/runtimes/google/security": Mistrusted IDs should be as
authorized as Unknown IDs
Andres cleverly pointed out that there is no gain in forbidding
Mistrusted PublicIDs and allowing Unknown PublicIDs -- an agent
with a Mistrusted id can always create an Unknown id (e.g., a
self-signed id) and get through.
This CL makes the following changes:
1) Modifies the implementation of the Authorize method for
chainPublicIDs to not take the trust-level of the identity
provider into account. The trust-level is instead checked
by the Names method.
2) Remove the UntrustedIDProviderPrefix constant from the
wire package and define two prefix constants for unknown
and mistrusted ids in the runtimes/security package.
3) Fix a bug in newChainPrivateID so that it returns an error
when the provided name is not a valid blessing name.
Change-Id: I29dc637dcdbbf225bcbd9411926061f79093944a
diff --git a/runtimes/google/ipc/server.go b/runtimes/google/ipc/server.go
index f1c8718..fb4406d 100644
--- a/runtimes/google/ipc/server.go
+++ b/runtimes/google/ipc/server.go
@@ -370,9 +370,7 @@
}
acl := make(security.ACL)
for _, n := range id.Names() {
- if !strings.HasPrefix(n, wire.UntrustedIDProviderPrefix) {
- acl[security.PrincipalPattern(n+wire.ChainSeparator+security.AllPrincipals)] = security.AllLabels
- }
+ acl[security.PrincipalPattern(n+wire.ChainSeparator+security.AllPrincipals)] = security.AllLabels
}
return acl
}
diff --git a/runtimes/google/security/identity_chain.go b/runtimes/google/security/identity_chain.go
index b8439e0..b6ad17b 100644
--- a/runtimes/google/security/identity_chain.go
+++ b/runtimes/google/security/identity_chain.go
@@ -19,6 +19,17 @@
"veyron2/vom"
)
+const (
+ // unknownIDProviderPrefix is the prefix added when stringifying
+ // an identity for which there is no entry for the root certificate
+ // in the trusted keys set.
+ unknownIDProviderPrefix = "unknown/"
+ // mistrustedIDProviderPrefix is the prefix added when stringifying
+ // an identity whose root certificate has a public key that does
+ // not exist in the (non-empty) set of trusted keys for that root.
+ mistrustedIDProviderPrefix = "mistrusted/"
+)
+
// chainPublicID implements security.PublicID.
type chainPublicID struct {
certificates []wire.Certificate
@@ -50,10 +61,14 @@
func (id *chainPublicID) String() string {
// Add a prefix if the identity provider is not trusted.
- if keys.LevelOfTrust(id.rootKey, id.certificates[0].Name) != keys.Trusted {
- return wire.UntrustedIDProviderPrefix + id.name
+ switch keys.LevelOfTrust(id.rootKey, id.certificates[0].Name) {
+ case keys.Trusted:
+ return id.name
+ case keys.Mistrusted:
+ return mistrustedIDProviderPrefix + id.name
+ default:
+ return unknownIDProviderPrefix + id.name
}
- return id.name
}
func (id *chainPublicID) VomEncode() (*wire.ChainPublicID, error) {
@@ -80,24 +95,10 @@
}
// Authorize checks if all caveats on the PublicID validate with respect to the
-// provided context and that the identity provider (root public key) is not
-// mistrusted. If so returns the original PublicID. This method assumes that
+// provided context and if so returns the original PublicID. This method assumes that
// the existing PublicID was obtained after successfully decoding a serialized
// PublicID and hence has integrity.
func (id *chainPublicID) Authorize(context security.Context) (security.PublicID, error) {
- rootCert := id.certificates[0]
- rootKey, err := rootCert.PublicKey.Decode()
- if err != nil {
- // unlikely to hit this case, as chainPublicID would have integrity.
- return nil, err
- }
- // Implicit "caveat": The identity provider should not be mistrusted.
- switch tl := keys.LevelOfTrust(rootKey, rootCert.Name); tl {
- case keys.Unknown, keys.Trusted:
- // No-op
- default:
- return nil, fmt.Errorf("%v public key(%v) for identity provider %q", tl, rootKey, rootCert.Name)
- }
for _, c := range id.certificates {
if err := c.ValidateCaveats(context); err != nil {
return nil, fmt.Errorf("not authorized because %v", err)
@@ -217,6 +218,9 @@
// private key, and a single self-signed certificate specifying the provided
// name and the public key corresponding to the generated private key.
func newChainPrivateID(name string) (security.PrivateID, error) {
+ if err := wire.ValidateBlessingName(name); err != nil {
+ return nil, err
+ }
key, err := ecdsa.GenerateKey(elliptic.P256(), rand.Reader)
if err != nil {
return nil, err
diff --git a/runtimes/google/security/identity_test.go b/runtimes/google/security/identity_test.go
index 5c3a141..18b43bf 100644
--- a/runtimes/google/security/identity_test.go
+++ b/runtimes/google/security/identity_test.go
@@ -16,6 +16,28 @@
type S []string
+func TestNewPrivateID(t *testing.T) {
+ testdata := []struct {
+ name, err string
+ }{
+ {"alice", ""},
+ {"alice#google", ""},
+ {"alice@google", ""},
+ {"bob.smith", ""},
+ {"", "invalid blessing name"},
+ {"/", "invalid blessing name"},
+ {"/alice", "invalid blessing name"},
+ {"alice/", "invalid blessing name"},
+ {"google/alice", "invalid blessing name"},
+ {"google/alice/bob", "invalid blessing name"},
+ }
+ for _, d := range testdata {
+ if _, err := NewPrivateID(d.name); !matchesErrorPattern(err, d.err) {
+ t.Errorf("NewPrivateID(%q): got: %s, want to match: %s", d.name, err, d.err)
+ }
+ }
+}
+
func TestNameAndAuth(t *testing.T) {
var (
cUnknownAlice = newChain("alice").PublicID()
@@ -30,11 +52,10 @@
testdata := []struct {
id security.PublicID
names []string
- err string
}{
{id: cUnknownAlice},
{id: cTrustedAlice, names: S{"veyron/alice"}},
- {id: cMistrustedAlice, err: "Mistrusted"},
+ {id: cMistrustedAlice},
{id: sAlice, names: S{"veyron/alice", "google/alice"}},
{id: sBadAlice},
{id: sGoogleAlice, names: S{"google/alice"}},
@@ -48,9 +69,6 @@
t.Errorf("%q.Authorize returned: (%v, %v), exactly one return value must be nil", d.id, authID, err)
continue
}
- if !matchesErrorPattern(err, d.err) {
- t.Errorf("%q.Authorize returned error: %v, want to match: %q", d.id, err, d.err)
- }
if err := verifyAuthorizedID(d.id, authID, d.names); err != nil {
t.Error(err)
}
