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

Already on GitHub? Sign in to your account

hash.update fails on second call (https://github.com/joyent/node/issues/749 Seen) #1415

Closed
Shahor opened this Issue Jul 28, 2011 · 12 comments

Comments

Projects
None yet
8 participants

Shahor commented Jul 28, 2011

Hi !
After a research I've seen this (joyent#749) happened to another person and should be fixed but I am facing the same problem.

Maybe it is just me doing wrong so here is the code :

var io = require('socket.io').listen(80);
var hasher = require('crypto').createHash('sha1');
var actualGames = [];

io.sockets.on('connection', function (socket) {
    var userHash = hasher.update(socket.id).digest('base64');
    socket.emit('greeting');

    socket.on('init', function(data) {
        var hash = data['hash'] || false;

        if (hash === false || (typeof actualGames[data['hash']] === 'undefined'))
        {
            console.log("CASE A");
            /* Just arrived on the game */
            actualGames[userHash] = {
                'board' : [[0,  0, 0,], [0,  0, 0,], [0,  0, 0,]],
                'players' : {
                    'p1' : socket,
                    'p2' : false
                }
            }
            socket.emit('init_ok', { 'hash' : userHash });
        }
        else /* Joining existing game */
        {
            console.log("CASE B");
        }
    })
});

And here is the server log :

$> sudo node server.js 
   info  - socket.io started
   debug - served static /socket.io.js
   debug - client authorized
   info  - handshake authorized 1607484018445727749
   debug - setting request GET /socket.io/1/websocket/1607484018445727749
   debug - set heartbeat interval for client 1607484018445727749
   debug - client authorized for 
   debug - websocket writing 1::
   debug - websocket writing 5:::{"name":"greeting"}
   debug - websocket received data packet 5:::{"name":"init","args":[{}]}
   debug - websocket writing 5:::{"name":"init_ok","args":[{"hash":"V245+PPSxBE/LEPEmEpUq6wLoYo="}]}
   info  - transport end
   debug - set close timeout for client 1607484018445727749
   debug - cleared close timeout for client 1607484018445727749
   debug - cleared heartbeat interval for client 1607484018445727749
   debug - discarding transport
   debug - served static /socket.io.js
   debug - client authorized
   info  - handshake authorized 78672861331388078
   debug - setting request GET /socket.io/1/websocket/78672861331388078
   debug - set heartbeat interval for client 78672861331388078
   debug - client authorized for 
   debug - websocket writing 1::

/Users/shahor/Documents/work/github/TicTacNode/server/server.js:6
    var userHash = hasher.update(socket.id).digest('base64');
                       ^
TypeError: HashUpdate fail
    at SocketNamespace.<anonymous> (/Users/shahor/Documents/work/github/TicTacNode/server/server.js:6:24)
    at SocketNamespace.$emit (events.js:64:17)
    at connect (/Users/shahor/Documents/work/github/TicTacNode/server/node_modules/socket.io/lib/namespace.js:290:10)
    at /Users/shahor/Documents/work/github/TicTacNode/server/node_modules/socket.io/lib/namespace.js:307:13
    at SocketNamespace.authorize (/Users/shahor/Documents/work/github/TicTacNode/server/node_modules/socket.io/lib/namespace.js:251:5)
    at SocketNamespace.handlePacket (/Users/shahor/Documents/work/github/TicTacNode/server/node_modules/socket.io/lib/namespace.js:301:14)
    at Manager.handleClient (/Users/shahor/Documents/work/github/TicTacNode/server/node_modules/socket.io/lib/manager.js:608:30)
    at Manager.handleUpgrade (/Users/shahor/Documents/work/github/TicTacNode/server/node_modules/socket.io/lib/manager.js:544:8)

I don't get it right now.

japj commented Jul 28, 2011

@Shahor what version of node did you see this on?

Shahor commented Jul 28, 2011

$> node -v
v0.4.10

japj commented Jul 28, 2011

@bnoordhuis does this sound familiar to you?

Owner

bnoordhuis commented Jul 28, 2011

@japj: variation on #749. That was about double calls to digest(), this about double calls to update(). I'll add extra checks to node_crypto.cc.

@koichik: I think the documentation should clarify that crypto objects are not reusable.

@Shahor: you cannot reuse hash objects. Your example works when you change this snippet:

var userHash = hasher.update(socket.id).digest('base64');

to this:

var userHash = require('crypto').createHash('sha1').update(socket.id).digest('base64');

Shahor commented Jul 28, 2011

Ok I see.

Indeed, the documentation didn't specify much about this so I thought it was ok.

Thank you for your help, I am closing this issue.

@Shahor Shahor closed this Jul 28, 2011

@koichik koichik added a commit that referenced this issue Jul 29, 2011

@koichik koichik Doc improvements
related to #1391, #1415.
c72223e
Owner

bnoordhuis commented Jul 30, 2011

@pquerna: peer review b31539a pretty please?

dekz commented Sep 9, 2011

Sorry to post on this issue again but here is some review.

@bnoordhuis It may actually be nicer to reinitialise the Digest object on a call to update after a digest. This way a new object doesn't have to be allocated every time and the same hash object can be reused. Updating docs to reflect this change would be needed, just declaring that a digest should be the called as a final method and no previous state should be expected once called.

Owner

bnoordhuis commented Sep 9, 2011

Completely forgotten about this, reopening.

Reinitializing the digest: I'll consider it. The other crypto classes (okay, probably just crypto.DiffieHellman) aren't reusable either.

@bnoordhuis bnoordhuis reopened this Sep 9, 2011

I certainly was not surprised by this behaviour and quickly came to the conclusion the crypto instances are not reusable. It's indeed explicitly stated in the documentation for both 0.4.x and 0.5.x...

Note: hash object can not be used after digest() method been called.

If it's not a security issue I suggest taking the advice of @dekz and making them reusable.

I'm not sure about cleaning up the crypto instance on a digest as this would add overhead to all existing crypto code. Here's two proposals...

  1. Reset on an update() after a digest()
  2. Expose a reset() or reinitialize() method

The latter of these options also better preserves backwards compatibility. That is, any existing code which...

  • presumes an update() after a digest() will fail
  • relies on some kind of final state of the crypto instance

will continue to work since the new reset() or reinitialize() method is not being called.

sh1mmer commented Nov 3, 2011

I'm marking this as a feature request because it's a non-bug issue.

Owner

indutny commented Mar 18, 2014

Crypto instances are not reusable, that's it.

@indutny indutny closed this Mar 18, 2014

@nihgwu nihgwu referenced this issue in rockq-org/austack-core Jul 30, 2015

Closed

get error when try to resendVerifyCode #175

@haocenxu haocenxu added a commit to haocenxu/node-cmpp-3 that referenced this issue Sep 19, 2016

@haocenxu haocenxu Avoid problem caused by multiple call to createHash 364237e

@haocenxu haocenxu referenced this issue in flyingfisher/node-cmpp-3 Sep 19, 2016

Closed

Avoid problem caused by multiple call to createHash #2

@tu1ly tu1ly added a commit to apiaryio/ivy that referenced this issue May 15, 2017

@tu1ly tu1ly fix: revert of 571c8a0 5236513
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment