Skip to content
This repository has been archived by the owner on Jul 15, 2019. It is now read-only.

Commit

Permalink
lookup.go: check realm name. For a unused case, this is a [security fix]
Browse files Browse the repository at this point in the history
The security fix in this commit affects clients whose trusted verifiers
use the same signing key and monitor colluding malicious keyservers.

The issue does NOT affect clients whose verifiers use different signing
keys for different keyservers. I am not aware of any verifiers who reuse
signing keys, so hopefully nobody is affected.

coname.VerifyLookup used to ignore the realm name in signed epoch heads.
This means that a malicious keyserver may have been able to have
verifiers verify two copies of itself (with different contents and
different realm names) and then present whichever view it chooses.

The client configuration file now includes the canonical realm name for
each name and VerifyLookup considers all SignedEpochHeads that do not
match that name to be invalid.

I changed the protobuf field numbers in config.proto. This should not
matter because this file is only used as a JSON schema; no actual
protobufs adhering to this spec are used anywhere in this codebase.

This change should be ported to the JS implementation of VerifyLookup.
  • Loading branch information
andres-erbsen committed Dec 14, 2015
1 parent d010089 commit 59332ee
Show file tree
Hide file tree
Showing 4 changed files with 92 additions and 33 deletions.
3 changes: 2 additions & 1 deletion keyserver/server_test.go
Expand Up @@ -203,6 +203,7 @@ func setupKeyservers(t *testing.T, nReplicas int) (
replicaIDs := []uint64{}
pol := &proto.AuthorizationPolicy{}
realmConfig := &proto.RealmConfig{
RealmName: testingRealm,
Domains: []string{realmDomain},
VRFPublic: vrfPublic,
VerificationPolicy: pol,
Expand Down Expand Up @@ -408,7 +409,7 @@ func doUpdate(
Candidates: []uint64{},
Subexpressions: []*proto.QuorumExpr{},
},
}},
}},
ProfileCommitment: commitment[:],
},
}
Expand Down
4 changes: 4 additions & 0 deletions lookup.go
Expand Up @@ -99,6 +99,10 @@ func VerifyConsensus(rcg *proto.RealmConfig, ratifications []*proto.SignedEpochH
return nil, fmt.Errorf("VerifyConsensus: epoch heads don't match: %x vs %x", want, got)
}
}
// check that the seh corresponds to the realm in question
if got := ratifications[0].Head.Head.Realm; got != rcg.RealmName {
return nil, fmt.Errorf("VerifyConsensus: SEH does not match realm: %q != %q", got, rcg.RealmName)
}
// check that the seh is not expired
if t := ratifications[0].Head.Head.IssueTime.Time().Add(rcg.EpochTimeToLive.Duration()); now.After(t) {
return nil, fmt.Errorf("VerifyConsensus: epoch expired at %v < %v", t, now)
Expand Down
99 changes: 75 additions & 24 deletions proto/config.pb.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

19 changes: 11 additions & 8 deletions proto/config.proto
Expand Up @@ -24,33 +24,36 @@ message Config {
}

message RealmConfig {
// RealmName is the canonical name of the realm. It is signed by the
// verifiers as a part of the epoch head.
string RealmName = 1;
// Domains specifies a list of domains that belong to this realm.
// Configuring one domain to belong to multiple realms is considered an
// error.
// TODO: support TLS-style wildcards.
repeated string domains = 1;
repeated string domains = 2;
// Addr is the TCP (host:port) address of the keyserver GRPC interface.
string addr = 2;
string addr = 3;
// URL is the location of the secondary, HTTP-based interface to the
// keyserver. It is not necessarily on the same host as addr.
string URL = 3;
string URL = 4;
// VRFPublic is the public key of the verifiable random function used for
// user id privacy. Here it is used to check that the anti-spam obfuscation
// layer is properly used as a one-to-one mapping between real and
// obfuscated usernames.
bytes VRFPublic = 4;
bytes VRFPublic = 5;
// VerificationPolicy specifies the conditions on how a lookup must be
// verified for it to be accepted. Each verifier in VerificationPolicy MUST
// have a NoOlderThan entry.
AuthorizationPolicy verification_policy = 5;
AuthorizationPolicy verification_policy = 6;

// EpochTimeToLive specifies the duration for which an epoch is valid after
// it has been issued. A client that has access to a clock MUST NOT accept
// epoch heads with IssueTime more than EpochTimeToLive in the past.
Duration epoch_time_to_live = 6 [(gogoproto.nullable) = false];
Duration epoch_time_to_live = 7 [(gogoproto.nullable) = false];

// TreeNonce is the global nonce that is hashed into the Merkle tree nodes.
bytes tree_nonce = 7;
bytes tree_nonce = 8;

TLSConfig client_tls = 8 [(gogoproto.customname) = "ClientTLS"];
TLSConfig client_tls = 9 [(gogoproto.customname) = "ClientTLS"];
}

0 comments on commit 59332ee

Please sign in to comment.