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

SSL support #729

Merged
merged 9 commits into from
Feb 7, 2014
Merged

SSL support #729

merged 9 commits into from
Feb 7, 2014

Conversation

sidorares
Copy link
Member

this is still WIP, but please review
see also #481

I'm able to connect to amazon rds using ssl:

var conn = require('node-mysql').createConnection({
  host     : 'xxxx.xxxxx.ap-southeast-2.rds.amazonaws.com',
  user     : 'xxxx',
  password : 'xxxx',
  port     : '3306',
  database : 'xxx',
  ssl      : 'Amazon RDS'
});

conn.query("show status like 'Ssl_cipher';", console.log);

output:

null [ { Variable_name: 'Ssl_cipher', Value: 'AES256-SHA' } ] [ { catalog: 'def',
    db: 'information_schema',
    table: 'STATUS',
    orgTable: 'STATUS',
    name: 'Variable_name',
    orgName: 'VARIABLE_NAME',
    filler1: <Buffer 0c>,
    charsetNr: 33,
    length: 192,
    type: 253,
    flags: 1,
    decimals: 0,
    filler2: <Buffer 00 00>,
    default: undefined,
    zeroFill: false,
    protocol41: true },
  { catalog: 'def',
    db: 'information_schema',
    table: 'STATUS',
    orgTable: 'STATUS',
    name: 'Value',
    orgName: 'VARIABLE_VALUE',
    filler1: <Buffer 0c>,
    charsetNr: 33,
    length: 3072,
    type: 253,
    flags: 0,
    decimals: 0,
    filler2: <Buffer 00 00>,
    default: undefined,
    zeroFill: false,
    protocol41: true } ]

Things left to do:

  • documentation
  • connect/disconnect streams2 properly using unpipe/pipe ( see TODO comments in the code )

otherwise it looks like ssl is fully functional

Not sure if there is a way to have ssl tested with travis mysql setup. Any suggestions?

@felixge
Copy link
Collaborator

felixge commented Feb 5, 2014

  • API LGTM!
  • Can't comment on the implementation details, haven't hand-rolled SSL/TLS before.
  • Do you see any way to get a local test case going? Maybe by adding SSL to the fake server implementation as well?

@felixge
Copy link
Collaborator

felixge commented Feb 5, 2014

Also: WHOOOOOOOOT. Thank you so much for working on this. 💖

@predominant
Copy link

❤️

@gergelyke
Copy link

Great! But I would add some test cases, just in case :)

@sidorares
Copy link
Member Author

@felixge - nothing simple out of my head. I have local self-signed cert and can start mysql configured to use them. Maybe it is possible to start second mysql server on travis listening on 3308 ( 3306 for initial travis server and 3307 for mysql fake ). I'll try to add ssl to fake server as well but no promise

@sidorares
Copy link
Member Author

@gergelyke I shall stop you not :)

@dresende
Copy link
Collaborator

dresende commented Feb 5, 2014

👍

@tim-kos
Copy link

tim-kos commented Feb 6, 2014

❤️ 👍

@sidorares
Copy link
Member Author

I'll leave tests as a separate issue - #730
Merging this PR, would be great to hear real life feedback before publishing as v0.2.1 - please try master!

sidorares added a commit that referenced this pull request Feb 7, 2014
@sidorares sidorares merged commit bd90d1d into master Feb 7, 2014
@sidorares sidorares deleted the ssl branch February 7, 2014 01:36
@felixge
Copy link
Collaborator

felixge commented Feb 7, 2014

💖 !

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

Successfully merging this pull request may close these issues.

None yet

7 participants