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

Enable server to server TLS #211

Open
wants to merge 65 commits into
base: master
Choose a base branch
from
Open

Enable server to server TLS #211

wants to merge 65 commits into from

Conversation

eaescob
Copy link
Contributor

@eaescob eaescob commented Aug 6, 2022

No description provided.

@lgtm-com
Copy link

lgtm-com bot commented Aug 6, 2022

This pull request introduces 1 alert when merging 5c4cc45 into 7fc039d - view on LGTM.com

new alerts:

  • 1 for Wrong type of arguments to formatting function

@lgtm-com
Copy link

lgtm-com bot commented Aug 6, 2022

This pull request introduces 2 alerts when merging 52b4dcc into 7fc039d - view on LGTM.com

new alerts:

  • 1 for Wrong type of arguments to formatting function
  • 1 for Comparison result is always the same

@lgtm-com
Copy link

lgtm-com bot commented Aug 6, 2022

This pull request introduces 1 alert when merging ed6587c into 7fc039d - view on LGTM.com

new alerts:

  • 1 for Wrong type of arguments to formatting function

@lgtm-com
Copy link

lgtm-com bot commented Aug 6, 2022

This pull request introduces 1 alert when merging 1b19af1 into 7fc039d - view on LGTM.com

new alerts:

  • 1 for Wrong type of arguments to formatting function

@lgtm-com
Copy link

lgtm-com bot commented Aug 6, 2022

This pull request introduces 2 alerts when merging 024722d into 7fc039d - view on LGTM.com

new alerts:

  • 2 for Wrong type of arguments to formatting function

@lgtm-com
Copy link

lgtm-com bot commented Aug 6, 2022

This pull request introduces 2 alerts when merging 37aa0fa into 7fc039d - view on LGTM.com

new alerts:

  • 2 for Wrong type of arguments to formatting function

@lgtm-com
Copy link

lgtm-com bot commented Aug 6, 2022

This pull request introduces 1 alert when merging 730a4ca into 7fc039d - view on LGTM.com

new alerts:

  • 1 for Wrong type of arguments to formatting function

@eaescob eaescob requested a review from crigler July 9, 2023 03:53
src/m_server.c Outdated
Comment on lines 189 to 193
#ifdef USE_SSL
IsSSL(cptr) ? " encrypted" : "",
#else
RC4EncLink(cptr) ? " encrypted" : "",
#endif
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks like it won't show anything for RC4 encrypted links anymore when USE_SSL is defined.

src/s_bsd.c Outdated
Comment on lines 880 to 904
sendto_one(cptr, "CAPAB SSJOIN NOQUIT BURST UNCONNECT DKEY"
sendto_one(cptr, "CAPAB SSJOIN NOQUIT BURTS UNCONNECT DKEY"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"BURTS" looks like a typo.

src/s_bsd.c Outdated

int ret=0;

mydata_index = SSL_get_ex_new_index(0, "mydata index", NULL, NULL, NULL);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

mydata_index should be initialized once in ssl_init instead of being recreated every connect attempt. This may show up as a problem when multiple connects are attempted simultaneously.

src/ssl.c Outdated
Comment on lines 471 to 479
{ /* only compare CN against server cert, not rest of chain */
/*
* for testing, must delete
*/

X509_NAME *subj = X509_get_subject_name(cert);
X509_NAME_ENTRY *e = X509_NAME_get_entry(subj, 5);
ASN1_STRING *d = X509_NAME_ENTRY_get_data(e);
char *cn = ASN1_STRING_data(d);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks like it could crash if the certificate isn't exactly as the code expects. Using X509_NAME_get_index_by_NID(subj, NID_commonName, -1) and using the result after checking for an error might help here.

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

Successfully merging this pull request may close these issues.

2 participants