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

Adding more pre-defined servers, as per file TODO #228

Merged
merged 5 commits into from
Apr 8, 2020
Merged

Adding more pre-defined servers, as per file TODO #228

merged 5 commits into from
Apr 8, 2020

Conversation

gcarreno
Copy link
Contributor

Hey there,

I'm including:

  • DALnet
  • EFnet
  • IRCnet
  • Undernet
  • Quakenet

Cheers,
Gus

@SilverRainZ
Copy link
Member

Thank for you contribuation!
Can you please provide the source of the newly added addresses? (Just in comment, not in code).

@SilverRainZ SilverRainZ self-requested a review March 25, 2020 06:05
Copy link
Member

@SilverRainZ SilverRainZ left a comment

Choose a reason for hiding this comment

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

I tested these networks, some of them not works well:

  • DALnet: OK
  • EFnet:
    • TLS ON: Failed to connect to EFnet(irc.efnet.fr:6697): Peer failed to perform TLS handshake: The TLS connection was non-properly terminated.
    • TLS OFF: seems OK
  • IRCnet
    • TLS ON: Failed to connect to IRCnet(irc.ircnet.com:6697): Peer failed to perform TLS handshake: The TLS connection was non-properly terminated.
    • TLS OFF: seems OK
  • Undernet
    • TLS ON: No response
    • TLS OFF: seems OK
  • Quakenet:
    • TLS ON: No response
    • TLS OFF: seems OK

Since srain will choose 6697 as TLS port by default when no port is given, you should provide the correct TLS port or reomve the line tls = true.

@gcarreno
Copy link
Contributor Author

Hey there,

Can you please provide the source of the newly added addresses? (Just in comment, not in code).

I started with these 2:

Then from the mIRC one I went to their official pages, found the Servers page and copied their listing.
From the Wikipedia one I picked the big ones listed above the "Timeline" paragraph.

In terms of TLS, you are right, I need to fix that. I wasn't sure if any of them had it. I just assumed that if they were operating today, they would have the latest security up. Looks like another case of "Assumptions are the mother of all f*ck ups (tm)" :)

I'll remove TLS from all but DALnet then.

Cheers,
Gus

@gcarreno gcarreno requested a review from SilverRainZ March 26, 2020 16:05
@SilverRainZ SilverRainZ added this to the 1.0.2 milestone Mar 26, 2020
Copy link
Member

@SilverRainZ SilverRainZ left a comment

Choose a reason for hiding this comment

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

IMO, An unencrypted TCP connection is dangerous today, I hope that we can provide TLS port as possible.

Beside, I think we does not need so many boackup addresses because the irc.* domain is usually a load balance of others.

@gcarreno
Copy link
Contributor Author

Beside, I think we does not need so many backup addresses because the irc.* domain is usually a load balance of others.

I agree with this and struggled if I would include the full list on each network's site.

I then thought that a list of them with countries would make sense if you want to connect to a server physically near you.

What do you think?

@gcarreno
Copy link
Contributor Author

IMO, An unencrypted TCP connection is dangerous today, I hope that we can provide TLS port as possible.

This makes much sense.

I remember that some of the servers mentioned SSL near them, so I'll revisit the listings and add the 6697 port explicitly.

I was also thinking about adding 6667 to all the others and turning tls=true on that group.

What do you think?

@gcarreno gcarreno requested a review from SilverRainZ March 26, 2020 22:25
@SilverRainZ
Copy link
Member

I then thought that a list of them with countries would make sense if you want to connect to a server physically near you.

Yes, this make sense, so keep these addresses is also fine.

I remember that some of the servers mentioned SSL near them, so I'll revisit the listings and add the 6697 port explicitly.

Thanks, please add them.

I was also thinking about adding 6667 to all the others and turning tls=true on that group.

Why? 6667 is an non-TLS port in IRC protocol, turning on it only cause error.

@gcarreno
Copy link
Contributor Author

gcarreno commented Mar 28, 2020

Why? 6667 is an non-TLS port in IRC protocol, turning on it only cause error.

Ahh, ok got it. I'll not do that then. I'll leave it with no port.

I'll do the 2 things you agreed on your last comment and then ping you again.

…s listed as so under their corresponding web pages.
@gcarreno
Copy link
Contributor Author

All done, ready for your review.

@SilverRainZ
Copy link
Member

So no one of the remaining server provides TLS port?

@gcarreno
Copy link
Contributor Author

gcarreno commented Apr 1, 2020

So no one of the remaining server provides TLS port?

I've gone through all the network's official server pages and these are the ones they mention having SSL/TLS.

So YES :)

Copy link
Member

@SilverRainZ SilverRainZ left a comment

Choose a reason for hiding this comment

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

LGTM

@SilverRainZ SilverRainZ merged commit c732eb3 into SrainApp:master Apr 8, 2020
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