tedious driver allows for incorrect domain value

[kevindashgit]

Expected: I get back a connection failure as user BADDOMAIN\USERNAME is invalid. Actual: I can connect without issue npm info:

└─┬ tedious@3.0.1
  ├── big-number@1.0.0
  ├─┬ bl@2.1.2
  │ ├─┬ readable-stream@2.3.6
  │ │ ├── core-util-is@1.0.2
  │ │ ├── isarray@1.0.0
  │ │ └── process-nextick-args@2.0.0
  │ └── safe-buffer@5.1.2
  ├── depd@1.1.2
  ├─┬ iconv-lite@0.4.24
  │ └── safer-buffer@2.1.2
  ├── native-duplexpair@1.0.0
  ├── punycode@2.1.1
  ├─┬ readable-stream@3.0.6
  │ ├── inherits@2.0.3
  │ ├── string_decoder@1.1.1
  │ └── util-deprecate@1.0.2
  └── sprintf-js@1.1.1

Example:

let Connection = require('tedious').Connection;

let master = {
  "server": "10.134.134.194",
  "port": 1433,
  "domain": "somestringvalue",  // actual domain is "thor"
  "userName": "batman",
  "password": "superman",
  "database": "master",
  "options": {
    "encrypt": true,
    "instanceName": "spiderman"
  }
};

let connection = new Connection(master);

var Request = require('tedious').Request;

function executeStatement() {
  request = new Request('select 1', function(err, rowCount) {
    if (err) {
      console.log(err);
    } else {
      console.log(rowCount + ' rows');
    }
  });

  request.on('row', function(columns) {
    columns.forEach(function(column) {
      console.log(column.value);
    });
  });

  request.on('done', ()=> {
    connection.close();
  });

  connection.execSql(request);
}

connection.on('connect', function(err) {
  if (err) {
    console.error('Error:', err);
    connection.close();
  }
  // If no error, then good to go...
  executeStatement();
}
);

Another thing to note is that if I delete the domain property from the configuration, I get an error as expected. If I were to provide an empty string, also get an error. Any non-empty value works though.

Maybe I'm just missing something and this is expected behavior?

[MichaelSun90]

Hi @kevindashgit, I have tried to reproduce this issue with tedious 3.0.1. However, I have not been able to reproduce the behavior that conducts a connection with any domain values. One thing that I can suggest is, could you maybe try the lastest Tedious build, see if this issue still exists? If you are not restricted to this specific tedious version. When I try to reproduce the issue, if I pass a wrong domain value : I get an error says： "ConnectionError: Login failed. The login is from an untrusted domain and cannot be used with Integrated authentication." which I think it recognizes the wrong domain and reporting an error against it. If I remove the domain from the config or give an empty string as its value: I get an error says： "ConnectionError: Login failed for user "

Are you receiving similar errors when you remove the domain from the config or give an empty string as its value?

[kevindashgit]

Hi @MichaelSun90 so I think what needs to happen is, if the username is associated with a domain, then if you provide a string value in the domain field (any string value) - it will be accepted. If there is no domain user, then I would expect a failure.

so, for user BAR:

FOO\BAR  // works
BAR      // fails
ZOO\BAR  // works

I will try the latest version of Tedious - although I am using the version packaged with node-mssql module.

[kevindashgit]

Confirmed same behavior with v5.0.3

Note: My configuration also included encrypt: true and instanceName: someInstanceName so not sure if that factors in...

[kevindashgit]

@MichaelSun90

If I remove the domain from the config or give an empty string as its value:

Please try not removing the domain from the config, but instead adding a dubious one.

[MichaelSun90]

@kevindashgit, I have two findings that I want to share with you: First, I think if you want to keep using tedious 5.0.3, then the domain, username, and password are passed into the connection configure in a different structure: authentication.options.userName User name to use for authentication. authentication.options.password Password to use for authentication. authentication.options.domain Once you set domain for ntlm authentication type, driver will connect to SQL Server using domain login. In order to make the connection working properly, this structure is necessary.

Second, under current Tedious logic, domain name is only supported for ntlm authentication type. If you did not indicate any authentication type, then Tedious will use a default one, which will not pick up the domain name passed in. This explains why an invalid domain will also work for the connection. authentication.type Type of the authentication method, valid types are default, ntlm, azure-active-directory- password

For detailed documentation for these fields, you can check this link: http://tediousjs.github.io/tedious/api-connection.html.

I hope this helps.

[kevindashgit]

Hey @MichaelSun90 appreciate your getting back to me. So I upgraded to tedious@latest (6.1.1) and used this config:

  var config = {
   server: '123.456.78.9',
   authentication: {
     type: 'ntlm',
     options: {
       userName: 'user',
       password: 'pass',
       domain: 'notcorrectdomain',
     }
   },
   options: {
     instanceName: 'someinstance',
     encrypt: true,
     database: 'master'
   }
 };

Am I doing something wrong? I'm still able to get away with using any string value for the domain property and have the connection succeed.

Interesting though, if I leave the domain property out entirely, I get this error: TypeError: The "config.authentication.options.domain" property must be of type string.

So...it wants a domain value present, but doesn't use it.

[MichaelSun90]

Hi @kevindashgit, If the authentication type is set to 'ntlm', then a domain value is necessary. So, if the domain does not exist from the config, then getting an error is the expected behavior.

On my side, if I pass in an incorrect domain name, I will get an error 'Login failed. The login is from an untrusted domain and cannot be used with Integrated authentication.' Also, your config looks exactly the same as my setup. Therefore, I am just wondering if that possible that the domain controller is disabled for your SQL server or some other domain setting is not set?

[IanChokS]

Hi @kevindashgit, are you still experiencing this issue with the latest Tedious version?

[Grravey]

I'm experiencing the same issue and am inclined to believe that this is a Windows configuration problem, rather than an issue with the tedious driver itself (I don't see how the driver would be allowing us to bypass an essential security feature).

I've just posted on stackoverflow to probe for possible leads, but am open to suggestions if you guys are interested.