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.

Sign up for GitHub

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

test: enable setuid/setgid test #12403

Closed
wants to merge 1 commit into from
Closed

test: enable setuid/setgid test #12403

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
56 changes: 40 additions & 16 deletions test/disabled/test-setuidgid.js → test/parallel/test-process-setuid-setgid.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,25 +20,49 @@
// USE OR OTHER DEALINGS IN THE SOFTWARE.

'use strict';
// Requires special privileges
const common = require('../common');

const assert = require('assert');

var oldgid = process.getgid();
process.setgid('nobody');
var newgid = process.getgid();
assert.notStrictEqual(newgid, oldgid, 'gids expected to be different');
if (common.isWindows) {
// uid/gid functions are POSIX only
assert.strictEqual(process.getuid, undefined);
assert.strictEqual(process.setuid, undefined);
assert.strictEqual(process.getgid, undefined);
assert.strictEqual(process.setgid, undefined);
return;
}

var olduid = process.getuid();
process.setuid('nobody');
var newuid = process.getuid();
assert.notStrictEqual(newuid, olduid, 'uids expected to be different');

try {
process.setuid('nobody1234');
} catch (e) {
assert.strictEqual(e.message,
'failed to resolve group',
'unexpected error message'
assert.throws(() => {
process.setuid('fhqwhgadshgnsdhjsdbkhsdabkfabkveybvf');
}, /^Error: setuid user id does not exist$/);

// If we're not running as super user...
if (process.getuid() !== 0) {
assert.doesNotThrow(() => {
process.getgid();
process.getuid();
});

assert.throws(
() => { process.setgid('nobody'); },
/^Error: (EPERM, .+|setgid group id does not exist)$/
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am actually getting Error: EPERM, Operation not permitted for both the cases. Also, the capture group doesn't look correct to me.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am actually getting Error: EPERM, Operation not permitted for both the cases.

@thefourtheye Error message depends on your operating system. SmartOS gives Error: EPERM, Not owner here.

The regexp allows any error that starts with Error: EPERM, and also allows Error: setgid group id does not exist for the situation where nobody is not included on the operating system (which happens on ubuntu1404-64 in FIPS mode on our CI).

TL;DR: Test passes if you have group nobody and get EPERM back along with some text from the OS, and it will also pass if group nobody doesn't exist at all.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, the capture group doesn't look correct to me.

@thefourtheye There's room for improvement. For one thing, it could be a non-capturing group. But the regexp seems to work to me. What situation are you concerned about?

> /^Error: (EPERM, .+|setgid group id does not exist)$/.test('Error: setgid group id does not exist')
true
> /^Error: (EPERM, .+|setgid group id does not exist)$/.test('Error: setgid group id does not exist adding some text so it will be false')
false
> /^Error: (EPERM, .+|setgid group id does not exist)$/.test('Error: EPERM, setgid group id does not exist adding some text so it will be false except now it as EPERM at the start so it should be true again')
true
>

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh I thought EPERM is always going to be there. As "setgid group id does not exist" is present in all the cases, can we make EPERM, optional and check for the presence of "setgid group id does not exist"?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The only thing that's there in all cases is Error: .

> /^Error: (EPERM, .+|setgid group id does not exist)$/.test('Error: EPERM, arbitrary text.')
true
>

The regexp can be thought of as: String must start with Error: and then there are two options that will match after that. One is EPERM, followed by one or more characters of anything. The other is the string setgid group id does not exist precisely.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh okay.

);

assert.throws(
() => { process.setuid('nobody'); },
/^Error: EPERM, /
);
return;
}

// If we are running as super user...
const oldgid = process.getgid();
process.setgid('nobody');
const newgid = process.getgid();
assert.notStrictEqual(newgid, oldgid);

const olduid = process.getuid();
process.setuid('nobody');
const newuid = process.getuid();
assert.notStrictEqual(newuid, olduid);