Skip to content

Added support for SASL EXTERNAL #363

Merged
osa1 merged 12 commits into
osa1:masterfrom
trevarj:sasl-external
Dec 31, 2022
Merged

Added support for SASL EXTERNAL #363
osa1 merged 12 commits into
osa1:masterfrom
trevarj:sasl-external

Conversation

@trevarj

@trevarj trevarj commented Nov 14, 2021

Copy link
Copy Markdown
Contributor

A user can now generate a x509 certificate, register it with a server,
and provide the PEM file to tiny for use over TLS.

Closes #196

Overview:

  1. Bumped rustls crates
  2. Added configuration for SASL EXTERNAL, where a user specifies a path to a PEM file with a certificate inside (generated with instructions from server).
  3. Use user provided certificate in TLS connector config for client authorization (crates/libtiny_client/src/stream.rs)

@trevarj

trevarj commented Nov 14, 2021 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Native TLS using openssl is waiting on rust-native-tls/rust-native-tls#209, which will make it much more convenient to add the user's certificate.

trevarj
trevarj commented Nov 14, 2021
View reviewed changes
Comment thread crates/tiny/src/main.rs Outdated
osa1
osa1 reviewed Nov 15, 2021
View reviewed changes

@osa1 osa1 left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Thanks @trevarj! Reviewed the front-end parts for now. I'll review the client parts tomorrow.

Comment thread crates/tiny/src/main.rs Outdated
Comment thread crates/tiny/src/main.rs Outdated
Comment thread crates/tiny/src/main.rs Outdated
Comment thread crates/tiny/src/main.rs Outdated
Comment thread crates/tiny/src/main.rs Outdated
Comment thread crates/tiny/src/main.rs Outdated
Comment thread crates/tiny/config.yml Outdated
osa1
osa1 approved these changes Nov 15, 2021
View reviewed changes

@osa1 osa1 left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

This looks great, thanks again @trevarj.

Are you currently using this? I didn't test it myself.

Comment thread crates/libtiny_client/src/lib.rs Outdated
Comment thread crates/libtiny_client/src/state.rs
Comment thread crates/tiny/config.yml Outdated
Comment thread crates/tiny/src/main.rs Outdated
@trevarj

trevarj commented Nov 15, 2021

Copy link
Copy Markdown
Contributor Author

Are you currently using this? I didn't test it myself.

Yes, I think I will continue once merged too.

osa1
osa1 reviewed Nov 16, 2021
View reviewed changes
Comment thread crates/libtiny_client/src/stream.rs Outdated
@trevarj
Added support for SASL EXTERNAL
38a994c 
A user can now generate a x509 certificate, register it with a server,
and provide the PEM file to tiny for use over TLS.

Closes osa1#196
@trevarj
Small cleanup and comments
0bc3a87
@trevarj
Fix comment error
8f6f82c
@trevarj
PR feedback
7ce42d4 
- Showing SASL errors in the respective server tab
- Error message changes
@trevarj
Config backwards compatibility
c68ba07 
Made the SASL config backwards compatible.
@trevarj
More PR Feedback
1555836
@trevarj
SASL External support for native TLS
db39a09
@trevarj
Fixed clippies
3b10e68
@trevarj
Fixed merge issues with pass cmd feature
934cc02
@trevarj

trevarj commented Dec 18, 2022

Copy link
Copy Markdown
Contributor Author

Fixed conflicts after pass cmd feature.

Needs testing still.

@trevarj trevarj mentioned this pull request Dec 18, 2022
Closed
@trevarj

trevarj commented Dec 19, 2022

Copy link
Copy Markdown
Contributor Author

@osa1 Tested using new password commands and SASL EXTERNAL. Both working as expected!
If you still want to merge this, it should be a fairly quick code review.

osa1
osa1 reviewed Dec 19, 2022
View reviewed changes

@osa1 osa1 left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Thanks @trevarj. Code looks good. I will have to read a bit about SASL EXTERNAL before merging this. Hopefully this week..

Comment thread crates/tiny/src/main.rs Outdated
Comment thread crates/tiny/src/main.rs Outdated
Comment thread crates/tiny/config.yml Outdated
# Providing a path to a PEM file will configure SASL EXTERNAL
# For SASL EXTERNAL certificate and fingerprint generation, see server documentation.
# You will need to register the cert's fingerprint with NickServ
# ex. https://www.oftc.net/NickServ/CertFP/

@osa1 osa1 Dec 19, 2022

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Remove this paragraph please. The config is getting quite large. We have an example below on how to use SASL EXTERNAL. We should add this info to the README if necessary.

@trevarj trevarj Dec 19, 2022

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I will add to the Wiki and link to it from the README? (Since the README is getting large too...)

@osa1 osa1 Dec 19, 2022

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

👍 moving this to wiki makes sense to me.

@trevarj trevarj Dec 19, 2022

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

  • Add to wiki once merged

@trevarj

trevarj commented Dec 19, 2022

Copy link
Copy Markdown
Contributor Author

I will have to read a bit about SASL EXTERNAL

Links that I also put in code comments:
https://ircv3.net/docs/sasl-mechs
https://www.alphachat.net/sasl.xhtml

osa1
osa1 approved these changes Dec 31, 2022
View reviewed changes

@osa1 osa1 left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Thanks @trevarj. Before merging this, could you update CHANGELOG adding your name to "Thanks to ..." part and also a line to describe these changes? I would do it myself but I don't know what this is about and I don't have time to study it now. If it works I'm happy to merge.

@trevarj

trevarj commented Dec 31, 2022

Copy link
Copy Markdown
Contributor Author

@osa1 Updated CHANGELOG. I created a stub page on the wiki to add configuration details later.

@osa1
osa1 merged commit 2544793 into osa1:master Dec 31, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@osa1 osa1 osa1 approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

SASL External

2 participants

@trevarj @osa1