Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(NODE-3689): require hello command for connection handshake to use OP_MSG disallowing OP_QUERY #3938

Merged
merged 17 commits into from
Dec 6, 2023
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions src/sdam/topology.ts
Original file line number Diff line number Diff line change
Expand Up @@ -382,6 +382,10 @@ export class Topology extends TypedEventEmitter<TopologyEvents> {
return this.s.options.loadBalanced;
}

get serverApi(): ServerApi | undefined {
return this.s.options.serverApi;
}

get capabilities(): ServerCapabilities {
return new ServerCapabilities(this.lastHello());
}
Expand Down
7 changes: 4 additions & 3 deletions src/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -371,9 +371,10 @@ export function uuidV4(): Buffer {
*/
export function maxWireVersion(topologyOrServer?: Connection | Topology | Server): number {
if (topologyOrServer) {
if (topologyOrServer.loadBalanced) {
// Since we do not have a monitor, we assume the load balanced server is always
// pointed at the latest mongodb version. There is a risk that for on-prem
if (topologyOrServer.loadBalanced || topologyOrServer.serverApi?.version) {
// Since we do not have a monitor, we assume
durran marked this conversation as resolved.
Show resolved Hide resolved
// when a server API version or load-balanced topology are requested
// the server is always pointed at the latest mongodb version. There is a risk that for on-prem
// deployments that don't upgrade immediately that this could alert to the
// application that a feature is available that is actually not.
return MAX_SUPPORTED_WIRE_VERSION;
Expand Down
83 changes: 82 additions & 1 deletion test/integration/mongodb-handshake/mongodb-handshake.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,12 @@ import * as sinon from 'sinon';
import {
Connection,
LEGACY_HELLO_COMMAND,
MessageStream,
MongoServerError,
MongoServerSelectionError
MongoServerSelectionError,
OpMsgRequest,
OpQueryRequest,
ServerApiVersion
} from '../../mongodb';

describe('MongoDB Handshake', () => {
Expand Down Expand Up @@ -57,9 +61,86 @@ describe('MongoDB Handshake', () => {
it('constructs a handshake with the specified compressors', async function () {
client = this.configuration.newClient({ compressors: ['snappy'] });
await client.connect();
// The load-balanced mode doesn’t perform SDAM,
// so `connect` doesn’t do anything unless authentication is enabled.
// Force the driver to send a command to the server in the noauth mode.
await client.db('admin').command({ ping: 1 });
expect(spy.called).to.be.true;
const handshakeDoc = spy.getCall(0).args[1];
expect(handshakeDoc).to.have.property('compression').to.deep.equal(['snappy']);
});
});

context('when load-balanced', function () {
let writeCommandSpy: Sinon.SinonSpy;
beforeEach(() => {
writeCommandSpy = sinon.spy(MessageStream.prototype, 'writeCommand');
});

afterEach(() => sinon.restore());

it('should send the hello command as OP_MSG', {
durran marked this conversation as resolved.
Show resolved Hide resolved
metadata: { requires: { topology: 'load-balanced' } },
test: async function () {
client = this.configuration.newClient({ loadBalanced: true });
await client.connect();
// The load-balanced mode doesn’t perform SDAM,
// so `connect` doesn’t do anything unless authentication is enabled.
// Force the driver to send a command to the server in the noauth mode.
await client.db('admin').command({ ping: 1 });
expect(writeCommandSpy).to.have.been.called;
expect(writeCommandSpy.firstCall.args[0] instanceof OpMsgRequest).to.equal(true);
durran marked this conversation as resolved.
Show resolved Hide resolved
}
});
});

context('when serverApi version is present', function () {
let writeCommandSpy: Sinon.SinonSpy;
beforeEach(() => {
writeCommandSpy = sinon.spy(MessageStream.prototype, 'writeCommand');
nbbeeken marked this conversation as resolved.
Show resolved Hide resolved
});

afterEach(() => sinon.restore());

it('should send the hello command as OP_MSG', {
durran marked this conversation as resolved.
Show resolved Hide resolved
metadata: { requires: { topology: '!load-balanced', mongodb: '>=5.0' } },
test: async function () {
client = this.configuration.newClient({}, { serverApi: { version: ServerApiVersion.v1 } });
await client.connect();
nbbeeken marked this conversation as resolved.
Show resolved Hide resolved
// The load-balanced mode doesn’t perform SDAM,
// so `connect` doesn’t do anything unless authentication is enabled.
// Force the driver to send a command to the server in the noauth mode.
await client.db('admin').command({ ping: 1 });
expect(writeCommandSpy).to.have.been.called;
expect(writeCommandSpy.firstCall.args[0] instanceof OpMsgRequest).to.equal(true);
durran marked this conversation as resolved.
Show resolved Hide resolved
}
});
});

context('when not load-balanced and serverApi version is not present', function () {
let writeCommandSpy: Sinon.SinonSpy;
beforeEach(() => {
writeCommandSpy = sinon.spy(MessageStream.prototype, 'writeCommand');
});

afterEach(() => sinon.restore());

it('should send the hello command as OP_MSG', {
durran marked this conversation as resolved.
Show resolved Hide resolved
metadata: { requires: { topology: '!load-balanced', mongodb: '>=5.0' } },
test: async function () {
client = this.configuration.newClient();
await client.connect();
// The load-balanced mode doesn’t perform SDAM,
// so `connect` doesn’t do anything unless authentication is enabled.
// Force the driver to send a command to the server in the noauth mode.
await client.db('admin').command({ ping: 1 });
expect(writeCommandSpy).to.have.been.called;

const opRequests = writeCommandSpy.getCalls().map(items => items.args[0]);
expect(opRequests[0] instanceof OpQueryRequest).to.equal(true);
nbbeeken marked this conversation as resolved.
Show resolved Hide resolved
const isOpMsgRequestSent = !!opRequests.find(op => op instanceof OpMsgRequest);
expect(isOpMsgRequestSent).to.equal(true);
durran marked this conversation as resolved.
Show resolved Hide resolved
}
});
});
});
2 changes: 1 addition & 1 deletion test/readme.md
Original file line number Diff line number Diff line change
Expand Up @@ -330,7 +330,7 @@ The following steps will walk you through how to start and test a load balancer.
A new file name `lb.env` is automatically created.
1. Source the environment variables using a command like `source lb.env`.
1. Export **each** of the environment variables that were created in `lb.env`. For example: `export SINGLE_MONGOS_LB_URI`.
1. Export the `LOAD_BALANCED` environment variable to 'true': `export LOAD_BALANCED='true'`
1. Export the `LOAD_BALANCER` environment variable to 'true': `export LOAD_BALANCER='true'`
alenakhineika marked this conversation as resolved.
Show resolved Hide resolved
1. Disable auth for tests: `export AUTH='noauth'`
1. Run the test suite as you normally would:
```sh
Expand Down
105 changes: 105 additions & 0 deletions test/unit/cmap/connection.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1135,4 +1135,109 @@ describe('new Connection()', function () {
expect(writeCommandSpy.firstCall.args[0] instanceof OpQueryRequest).to.equal(true);
});
});

describe('when serverApi version is present', () => {
nbbeeken marked this conversation as resolved.
Show resolved Hide resolved
const CONNECT_DEFAULTS = {
id: 1,
tls: false,
generation: 1,
monitorCommands: false,
metadata: {} as ClientMetadata
};
let server;
let connectOptions;
let connection: Connection;
let writeCommandSpy;

beforeEach(async () => {
server = await mock.createServer();
server.setMessageHandler(request => {
request.reply(mock.HELLO);
});
writeCommandSpy = sinon.spy(MessageStream.prototype, 'writeCommand');
});

afterEach(async () => {
connection?.destroy({ force: true });
sinon.restore();
await mock.cleanup();
});

it('sends the first command as OP_MSG', async () => {
try {
connectOptions = {
...CONNECT_DEFAULTS,
hostAddress: server.hostAddress() as HostAddress,
socketTimeoutMS: 100,
serverApi: { version: '1' }
};

connection = await promisify<Connection>(callback =>
//@ts-expect-error: Callbacks do not have mutual exclusion for error/result existence
connect(connectOptions, callback)
)();

await promisify(callback =>
connection.command(ns('admin.$cmd'), { hello: 1 }, {}, callback)
)();
} catch (error) {
/** Connection timeouts, but the handshake message is sent. */
}

expect(writeCommandSpy).to.have.been.called;
expect(writeCommandSpy.firstCall.args[0] instanceof OpMsgRequest).to.equal(true);
durran marked this conversation as resolved.
Show resolved Hide resolved
});
});

describe('when serverApi version is not present', () => {
const CONNECT_DEFAULTS = {
id: 1,
tls: false,
generation: 1,
monitorCommands: false,
metadata: {} as ClientMetadata
};
let server;
let connectOptions;
let connection: Connection;
let writeCommandSpy;

beforeEach(async () => {
server = await mock.createServer();
server.setMessageHandler(request => {
request.reply(mock.HELLO);
});
writeCommandSpy = sinon.spy(MessageStream.prototype, 'writeCommand');
});

afterEach(async () => {
connection?.destroy({ force: true });
sinon.restore();
await mock.cleanup();
});

it('sends the first command as OP_MSG', async () => {
try {
connectOptions = {
...CONNECT_DEFAULTS,
hostAddress: server.hostAddress() as HostAddress,
socketTimeoutMS: 100
};

connection = await promisify<Connection>(callback =>
//@ts-expect-error: Callbacks do not have mutual exclusion for error/result existence
connect(connectOptions, callback)
)();

await promisify(callback =>
connection.command(ns('admin.$cmd'), { hello: 1 }, {}, callback)
)();
} catch (error) {
/** Connection timeouts, but the handshake message is sent. */
}

expect(writeCommandSpy).to.have.been.called;
expect(writeCommandSpy.firstCall.args[0] instanceof OpQueryRequest).to.equal(true);
durran marked this conversation as resolved.
Show resolved Hide resolved
});
});
});