Skip to content

known_host host identity fingerprint duplication #108

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

Closed
legraphista opened this issue Jul 22, 2016 · 3 comments
Closed

known_host host identity fingerprint duplication #108

legraphista opened this issue Jul 22, 2016 · 3 comments

Comments

@legraphista
Copy link
Contributor

Referring to issue #107
I have looked through the code and found that the only way that this error can happen

/app/node_modules/neo4j-driver/lib/v1/internal/ch-node.js:214
      for (var i = 0; i < pending.length; i++) {
                                 ^

TypeError: Cannot read property 'length' of null
    at /app/node_modules/neo4j-driver/lib/v1/internal/ch-node.js:214:34
    ...

is if the trust certificate callback is called twice:

    this._handleConnectionError = this._handleConnectionError.bind(this);

-> this._conn = connect(opts, function () {
      if (!self._open) {
        return;
      }

     <...>
      var pending = self._pending;
      self._pending = null;
      for (var i = 0; i < pending.length; i++) {
        self.write(pending[i]);
      }
    }, this._handleConnectionError);

because it it setting self._pending to be null.

The trust certificate function can be called twice if the loadFingerprint callback is called twice.
After looking at the known_hosts file

localhost:7687 fabacd62dbd94...
localhost:7687 fabacd62dbd94...

We see that there are duplicate entries, causing this to fire more than once:

require('readline').createInterface({
    input: fs.createReadStream(knownHostsPath)
  }).on('line', (line)  => {
    if( line.startsWith( serverId )) {
-->   found = true;
-->   cb( line.split(" ")[1] );
    }
  }).on('close', () => {
    if(!found) {
      cb(null);
    }
  });

But why are there duplicates?
Under the circumstances that the known_hosts file exists but is empty (manually created by someone) and if the app is calling for two (or more) driver sessions in the same process tick

const session = driver.session();
const session2 = driver.session();

The driver doesn't have time to update the known_hosts file and thus creates two (or more) entries of the same thing.
After that, the next session that is created:

setTimeout(() => {
    const session3 = driver.session();
}, 1000);

will throw the error.

@zhenlineo
Copy link
Contributor

Thanks so much for helping us investigating on this issue! So sorry we are running a bit low bandwidth to helping with the related issue #107 now.

We will come back to this and get it fixed next week. You are most welcome to send us a PR if you have got an idea in mind! If it is possible, please include a test to verify the PR have the correct fix and prevent the same problem from happening again.

Thanks again!
Zhen

@legraphista
Copy link
Contributor Author

Sure thing @zhenlineo. I will shortly come up with a PR.

@legraphista
Copy link
Contributor Author

Fixed with #110

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants