hapijs / glue

Server composer for hapi.js
Other
245 stars 62 forks source link

Issue when passing fs.readFileSync buffer #97

Closed richardzyx closed 7 years ago

richardzyx commented 7 years ago

I was working on getting TLS to work with frame that uses Glue+Manifest+Confidence for configuration management. Having tried both key/cert and pfx pattern and days of debugging, I believe the issue of not having TLS config correctly passed to the underlying node createServer was caused by Glue's composer when converting the manifest file.

Although hapi's documentation stated that the tls setting should be directly passed to node using fs.readFileSync, glue converted these binary file buffer to a strange format that 1. couldn't be read correctly but compiled and didn't throw error or 2. caused failed to create BIO error. The tricky thing is that the buffer will get half read and I could only debug the tls handshake failure using openssl to pinpoint the issue.

The current workaround of encoding the binary buffer to utf8 would only work with key/cert pattern and not with pfx since the converted string would be too large and node will throw a header too long exception.

In addition, this issue was further covered by node's complete conversion to TLSv1+ and wouldn't throw an error when using a SSLv2-3 key/cert. But this only pertains to my own debugging experience.

The apparent solution would be to create a special case for server.connection.tls and pass it without pre-processing. The root cause however I think would be handling raw buffer in general. Similar unanswered question in the past is #46

Please feel free to let me know if I miss something apparent, or you have found evidence that it's caused by Manifest or Confidence. Thanks!

jedireza commented 7 years ago

The apparent solution would be to create a special case for server.connection.tls and pass it without pre-processing.

It doesn't look like glue is doing any processing before passing the object to server.connection(...).

https://github.com/hapijs/glue/blob/8dc31113ee620dea13fc3b94c6cb9bcb3fa81ace/lib/index.js#L85

If you're using confidence, maybe something there could be a problem. Have you tested without confidence in the mix (just a simple JSON object)?

jedireza commented 7 years ago

I'm dropping some related issues, though I see you commented on one already:

jedireza commented 7 years ago

Also maybe relevant:

richardzyx commented 7 years ago

Also maybe relevant: https://stackoverflow.com/questions/24392571/node-js-crypto-js-pfx-header-too-long

Yes I wondered about that as well. This is the main reason why pfx file just won't work either way, since the utf8 workaround will lead to this issue, while the buffer cannot get passed. I will look into the possibility of Confidence being the root cause of this.

richardzyx commented 7 years ago

@jedireza I did some bare-bones testing and can confirm that with pure hapi, tls works when passing binary buffer. But once Confidence is added, I got the same error as before: ERR_SSL_VERSION_OR_CIPHER_MISMATCH

Sorry now it definitely seems to be caused by Confidence. Is there anyway I can directly move this issue to there? Thanks!

jedireza commented 7 years ago

No you can't move issues but you can copy the url and use it in the description of the issue you create (or a comment) and they'll get linked together.

Sounds like this issue can be closed.

WesTyler commented 7 years ago

Closing since the issue was traced to confidence.

lock[bot] commented 4 years ago

This thread has been automatically locked due to inactivity. Please open a new issue for related bugs or questions following the new issue template instructions.