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

fix: handshake SSL error with AWS RDS #2857

Merged
merged 4 commits into from
Jul 15, 2024
Merged

Conversation

wellwelwel
Copy link
Collaborator

@wellwelwel wellwelwel commented Jul 15, 2024

Closes #2581.


This PR deprecates the ssl: 'Amazon RDS' and uses the certificate bundle from aws-ssl-profiles dependency.
The documentation has been updated to suggest installing aws-ssl-profiles for AWS RDS certificates.

@MarioRomanDono, thanks again for the proxy bundle 🙋🏻‍♂️


'use strict';

const awsCaBundle = require('aws-ssl-profiles');

/**
 * @deprecated
 * Please, use [**aws-ssl-profiles**](https://github.com/mysqljs/aws-ssl-profiles).
 */
exports['Amazon RDS'] = {
  ca: awsCaBundle.ca,
};

Note

☔️ The coverage percentage was affected by removing the amount of previous lines from string certificates (~2,877 lines removed).

Copy link

codecov bot commented Jul 15, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 88.18%. Comparing base (2e61c94) to head (4118413).

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2857      +/-   ##
==========================================
- Coverage   90.34%   88.18%   -2.16%     
==========================================
  Files          71       71              
  Lines       15749    12874    -2875     
  Branches     1350     1351       +1     
==========================================
- Hits        14228    11353    -2875     
  Misses       1521     1521              
Flag Coverage Δ
compression-0 88.18% <ø> (-2.16%) ⬇️
compression-1 88.18% <ø> (-2.16%) ⬇️
tls-0 87.60% <ø> (-2.27%) ⬇️
tls-1 87.94% <ø> (-2.21%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@wellwelwel wellwelwel marked this pull request as ready for review July 15, 2024 07:34
@sidorares
Copy link
Owner

LGTM, thanks @wellwelwel !

@wellwelwel wellwelwel merged commit de071bb into sidorares:master Jul 15, 2024
69 checks passed
@wellwelwel wellwelwel deleted the aws branch July 15, 2024 22:37
@mrajasekar-godaddy
Copy link

Hi @sidorares Has this fix been published yet ? Currently I'm using promise-mysql which is a wrapper function of mysqljs

Currently when I use ssl: awsCaBundle to pull in the new cert then I am getting the following error:

"error": {
"message": "ER_BAD_FIELD_ERROR: Unknown column 'PJ.tier' in 'field list'",
"code": "ER_BAD_FIELD_ERROR",
"detail": null,
"stack": "Error: ER_BAD_FIELD_ERROR: Unknown column 'PJ.tier' in 'field list'\n
"}"

And this is not related to the codebase. Is there something which I am doing here is wrong

Our config return { host: process.env.DBHOST, port: process.env.DBPORT, user: process.env.DBUSER, password: process.env.DBPASSWORD, connectTimeout: 3000, timezone: 'Z', ssl: awsCaBundle }

@wellwelwel
Copy link
Collaborator Author

wellwelwel commented Jul 19, 2024

Hi @mrajasekar-godaddy, can you perform a basic test?

Instead of:

{
  ssl: awsCaBundle,
}

Try:

{
  ssl: {
    ca: awsCaBundle.ca,
  }
}

I'm a bit uncomfortable with the import approach, for example:

import awsCaBundle from 'aws-ssl-profiles';

console.log(Object.getOwnPropertyNames(awsCaBundle));
// -> [ 'ca', 'proxyBundle' ]

By using it as { ssl: awsCaBundle }, we aren't only passing on ca, but also all the modules available in the import (not sure if this is related, btw).

@mrajasekar-godaddy
Copy link

Hi @wellwelwel, Thank you for the quick response. Sure will do test it

Got it. Agree with your thought on using the import approach, that wouldn't be ideal to use. Will test out the suggestion and let you know.

{ ssl: { ca: awsCaBundle.ca, } }

@mrajasekar-godaddy
Copy link

Hi @wellwelwel

I did try using this { ssl: { ca: awsCaBundle.ca, } } but it still does't work and I am seeing the same error. Is there any other possible ways I can try it. It seems to not refer to the ca cert.

Thanks in Advance

@wellwelwel
Copy link
Collaborator Author

I did try using this { ssl: { ca: awsCaBundle.ca, } } but it still does't work and I am seeing the same error. Is there any other possible ways I can try it. It seems to not refer to the ca cert.

Hey @mrajasekar-godaddy, in that case, I recommend you to open an issue.

If possible, share a minimal reproduction 🙋🏻‍♂️

@mrajasekar-godaddy
Copy link

Thank you for the quick response. Submitted an issue here [https://github.com//issues/2869]

@mrajasekar-godaddy
Copy link

mrajasekar-godaddy commented Jul 21, 2024

@wellwelwel Also, Added steps for reproducing the error in the issue. It would be great, if we could even schedule a call if that works.

My observations ( if this would help by any chance ):

  1. ssl: expects a string Amazon RDS , but when we supply ssl: awsCaBunlde it doesn't accpet it, as awsCaBunle is an object

  2. Also, when we supply as { ssl: { ca: awsCaBunlde.ca } } it won't work either. as ca option does expect string.

Thanks again

@mrajasekar-godaddy
Copy link

@wellwelwel Hi, I was able to connect to SSL finally. Thank you for the support and the help

Huge thanks to all the folks who contributed to creating this library.

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.

HANDSHAKE_SSL_ERROR with RDS and 3.9.3+
3 participants