jschoubben
3195634441
The builder was documented as holding its own broker credential and nothing else, and nothing issued one — so in practice it used whatever it was handed, which was the broker's administrative account. A program documented as holding its own credential and given somebody else's is worse than one with no story at all. `builder issue <name>` creates an account that may read the build queue and write to the mesh exchange. Not a node account: a build machine is not a node, and a node's queue carries its declarations. Two faults found by running it, both about the answer path: - the reply queue was left for the broker to name, and the account was scoped to `amq.gen-*` — one broker's convention. The builder built, could not answer, and the connection closed. Reply queues are named here now, deterministically. - the answer then went via the DEFAULT exchange, where permission is granted per exchange rather than per queue. A builder allowed to use it could publish into any node's queue, which is the privilege a build machine most obviously should not have. Answers go through the mesh exchange, which it already may use, and an asker binds its reply queue to the same key and filters by correlation. Verified against a real broker: a builder cannot consume a node's queue and cannot publish to the default exchange. That check nearly reported the opposite — an unconfirmed publish is asynchronous, so the refusal arrives as a channel close afterwards and a naive test sees success. With publisher confirms it is immediate. A negative security assertion made against an asynchronous call is not an assertion. Redelivery was observed working while fixing this: builders that died before answering left their work on the queue, and the next builder did all of it. Also: the queue and exchange names exist in both `broker` and `link`, because `link` imports `broker`. A test in an external package keeps them agreeing — a builder scoped to a queue nothing publishes to takes no work and says nothing about why.
68 lines
2.9 KiB
Go
68 lines
2.9 KiB
Go
package broker_test
|
|
|
|
import (
|
|
"regexp"
|
|
"testing"
|
|
|
|
"github.com/novox/mesh-control/internal/broker"
|
|
"github.com/novox/mesh-control/internal/link"
|
|
)
|
|
|
|
// The names are written twice, so a test keeps them agreeing.
|
|
//
|
|
// `link` imports `broker`, so `broker` cannot import `link` — the queue and exchange names
|
|
// therefore exist in both. A scoped account naming a queue nothing publishes to produces a
|
|
// builder that takes no work and says nothing about why, which is the worst kind of silence.
|
|
//
|
|
// An external test package, because it may import both without either importing the other.
|
|
func TestTheNamesTheBrokerScopesAreTheNamesTheLinkUses(t *testing.T) {
|
|
for _, agreed := range []struct {
|
|
what string
|
|
scoped string
|
|
actually string
|
|
}{
|
|
{"the build queue", broker.BuildQueueName, link.BuildQueue},
|
|
{"the exchange", broker.ExchangeName, link.Exchange},
|
|
{"a node's queue", broker.QueueFor("somewhere"), link.QueueFor("somewhere")},
|
|
} {
|
|
if agreed.scoped != agreed.actually {
|
|
t.Errorf("%s: the broker scopes %q and the link uses %q",
|
|
agreed.what, agreed.scoped, agreed.actually)
|
|
}
|
|
}
|
|
}
|
|
|
|
func TestABuilderMayWriteToAReplyQueueAndReadNoNodesDeclarations(t *testing.T) {
|
|
// The scoping, checked as patterns rather than by connecting: what it may write must include
|
|
// the queue an asker actually waits on, and what it may read must not include any node's.
|
|
//
|
|
// This exists because the first version scoped writes to `amq.gen-*` — the name one broker
|
|
// happens to generate — and the builder built, could not answer, and the connection simply
|
|
// closed saying only "not allowed to publish to exchange \'\'". Answering through the
|
|
// exchange is what removed the need for any of that.
|
|
write := regexp.MustCompile("^" + regexp.QuoteMeta(broker.ExchangeName) + "$")
|
|
if !write.MatchString(broker.ExchangeName) {
|
|
t.Error("a builder may not write to the exchange, so it can take work and never answer")
|
|
}
|
|
// And not the default exchange, where permission is per exchange rather than per queue — a
|
|
// builder allowed to use it could publish into any node's queue.
|
|
//
|
|
// Confirmed against a real broker as well, and worth recording how that nearly went wrong:
|
|
// an unconfirmed publish is asynchronous, so a refusal arrives as a channel close afterwards
|
|
// and a naive check reports success. With publisher confirms the broker's refusal is
|
|
// immediate. **A negative security assertion made against an asynchronous call is not an
|
|
// assertion.**
|
|
if write.MatchString("") {
|
|
t.Error("a builder may publish to the default exchange, and so into any node's queue")
|
|
}
|
|
|
|
read := regexp.MustCompile("^" + regexp.QuoteMeta(broker.BuildQueueName) + "$")
|
|
if !read.MatchString(broker.BuildQueueName) {
|
|
t.Error("a builder may not read the build queue")
|
|
}
|
|
if read.MatchString(link.QueueFor("someone-else")) {
|
|
// A build machine is not a node, and a node's queue carries its declarations.
|
|
t.Error("a builder may read another machine's declarations")
|
|
}
|
|
}
|