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

allow clients to validate server without providing their own credentials #163

Closed
wants to merge 1 commit into from
Closed

allow clients to validate server without providing their own credentials #163

wants to merge 1 commit into from

Conversation

mrose17
Copy link
Contributor

@mrose17 mrose17 commented Feb 25, 2014

No description provided.

@noppermann
Copy link

+1

@mcollina mcollina mentioned this pull request May 10, 2014
8 tasks
@mcollina
Copy link
Member

Can you please add a test for this? Thanks!

@mrose17
Copy link
Contributor Author

mrose17 commented May 10, 2014

this is kind of embarrassing to admit… i've never really figured out how to do the node test harness stuff… all i can tell you is that i've been using the code 24x7 for the last two months (-; sorry!

@mcollina
Copy link
Member

Can you please try #183 and check if everything works as planned? I did some refactoring.

@mrose17
Copy link
Contributor Author

mrose17 commented May 13, 2014

sorry, it's still broken. the issue is that opts.keyPath/opts.certPath are logically separate from opts.ca -- keyPath and certPath deal with the client authenticating itself, whilst ca deals with the certification authorities to be used when the server authenticates itself. so the logic needs to look like this:

    if (opts.keyPath && opts.certPath) {
      tls_opts.key = fs.readFileSync(opts.keyPath);
      tls_opts.cert = fs.readFileSync(opts.certPath);
    }
    tls_opts.ca = [];
    if (opts.ca) {
        for (var i = 0;i<opts.ca.length;i++) {
            tls_opts.ca[i] = fs.readFileSync(opts.ca[i]);
        }
    }
    tls_opts.rejectUnauthorized = opts.rejectUnauthorized || false;

@mcollina
Copy link
Member

It is exactly like that in #183: https://github.com/adamvr/MQTT.js/blob/0.3.9-dev/lib/mqtt.js#L128-L137.

Can you git pull that branch and test it? In case it's not working, can you please provide a script that show the issue?

@mrose17
Copy link
Contributor Author

mrose17 commented May 13, 2014

oops, my bad! ok, i'm running with it now and don't see any issues. i'll keep running with it locally. if there's an issue, i'll report back… otherwise, let's call this one closed. thanks!

@mrose17 mrose17 closed this May 13, 2014
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

Successfully merging this pull request may close these issues.

3 participants