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.

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

Bug: TypeError: connection._server.options.SNICallback is not a function #635

Closed
titanism opened this issue Feb 23, 2024 · 7 comments · Fixed by #625
Closed

Bug: TypeError: connection._server.options.SNICallback is not a function #635

titanism opened this issue Feb 23, 2024 · 7 comments · Fixed by #625
Labels

Comments

@titanism
Copy link
Contributor

Despite setting a default SNICallback function, it is being binded and/or used on the wrong object (typo perhaps).

TypeError: connection._server.options.SNICallback is not a function
    at TLSSocket.SNICallback [as _SNICallback] (/usr/src/app/node_modules/.pnpm/wildduck@1.41.0/node_modules/wildduck/imap-core/lib/commands/starttls.js:42:40)
    at TLSWrap.loadSNI [as oncertcb] (node:_tls_wrap:220:9) 

connection._server.options.SNICallback(servername, (err, context) => {

The function is being invoked from connection._server.options.SNICallback however the default SNICallback function is being binded to this.options.SNICallback, not this.server.options as per this:

// ensure SNICallback method
if (typeof this.options.SNICallback !== 'function') {
// create default SNI handler
this.options.SNICallback = (servername, cb) => {
cb(null, this.secureContext.get(servername));
};
}

When you start in insecure mode, e.g. port 113, and if you use a client like Apple Mail and accidentally enable STARTTLS, an uncaught exception / error will be thrown as per above.

This most likely needs fixed for both IMAP and POP3 implementations.

Thanks to @shaunwarman for the find 🙏

@andris9
Copy link
Member

andris9 commented Feb 25, 2024

This bug seems only to be triggered if you create a custom IMAP/POP3 server, not when running WildDuck. The reason for the bug is that the default SNICallback is only defined when the server is using TLS, should be the reverse. WildDuck always defines this method and as such the default is never called and the bug is not triggered.

@andris9 andris9 added the bug label Feb 25, 2024
titanism added a commit to titanism/wildduck that referenced this issue Feb 26, 2024
@titanism
Copy link
Contributor Author

@andris9 PR submitted at #637

@titanism
Copy link
Contributor Author

@andris9 I think this fix actually broke stuff.

3|imap-993  | TypeError: this.options.SNICallback is not a function
3|imap-993  |     at TLSSocket.SNICallback [as _SNICallback] (/var/www/production/source/node_modules/.pnpm/wildduck@1.42.5/node_modules/wildduck/imap-core/lib/imap-server.js:291:30)
3|imap-993  |     at TLSWrap.loadSNI [as oncertcb] (node:_tls_wrap:217:9)
3|imap-993  | TypeError: this.options.SNICallback is not a function
3|imap-993  |     at TLSSocket.SNICallback [as _SNICallback] (/var/www/production/source/node_modules/.pnpm/wildduck@1.42.5/node_modules/wildduck/imap-core/lib/imap-server.js:291:30)
3|imap-993  |     at TLSWrap.loadSNI [as oncertcb] (node:_tls_wrap:217:9) { hide_meta: true }

This is in production after version bumping to latest WildDuck release with this patch.

@titanism
Copy link
Contributor Author

Downgrading to v1.42.1 fixes the issue in production environments with Forward Email

titanism added a commit to forwardemail/forwardemail.net that referenced this issue Apr 20, 2024
@andris9
Copy link
Member

andris9 commented Apr 21, 2024

WildDuck seems to work fine. Additionally, there are some changes planned around SNI in the near future, as using SNI will become more important. So no plans to address your issue for now as it does not actually concern the usage of WildDuck.

@titanism
Copy link
Contributor Author

I think that 4b19dee should be rolled back in the interim and released to npm

@andris9
Copy link
Member

andris9 commented Apr 22, 2024

The server assumes that SNICallback is set. This is an internal API, and WildDuck uses SNICallback. In future releases, the SNI will become the default anyway, so it is OK to assume that SNICallback is always set.

titanism added a commit to forwardemail/forwardemail.net that referenced this issue Jun 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants