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 handling of multiple SRV records for a single domain #10

Merged
merged 2 commits into from Jul 25, 2020
Merged

Fix handling of multiple SRV records for a single domain #10

merged 2 commits into from Jul 25, 2020

Conversation

ghost
Copy link

@ghost ghost commented Jul 24, 2020

Closes issue #9

echoz.sh Outdated
srv="$( ( dig +short SRV "_xmpp-client._tcp.$domain" | grep . || echo "0 0 5222 $domain" ) | sort -n | sed 's/[[:digit:]]\+[[:space:]][[:digit:]]\+[[:space:]]//')"
host="$(echo "$srv" | sed 's/[[:digit:]]\+[[:space:]]//')"
srv="$( ( dig +short SRV "_xmpp-client._tcp.$domain" | grep . || echo "0 0 5222 $domain" ) | sort -n | sed -n '1s/[[:digit:]]\+[[:space:]][[:digit:]]\+[[:space:]]//p')"
host="$(echo "$srv" | sed 's/[[:digit:]]\+[[:space:]]//' | sed 's/\.$//')"
Copy link
Owner

Choose a reason for hiding this comment

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

Why do you remove the trailing dot?

If there’s a good reason, it should be done in a single sed call anyways.

Copy link
Author

@ghost ghost Jul 24, 2020

Choose a reason for hiding this comment

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

While it certainly works even without removing the trailing dot, I get the following error message (seems benign though) on the first line (resulting from the invocation of openssl s_client):

$ ./echoz.sh [redacted]@xmpp.jp [redacted]
Can't use SSL_get_servername
depth=2 O = Digital Signature Trust Co., CN = DST Root CA X3
verify return:1
depth=1 C = US, O = Let's Encrypt, CN = Let's Encrypt Authority X3
verify return:1
depth=0 CN = xmpp.jp
verify return:1

Removing the trailing dot causes the error on the first line to go away. I can still have the script echo messages regardless of the error message.

Copy link
Author

Choose a reason for hiding this comment

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

@RON42 My best guess is that the SRV record contains the domain name against which the TLS certificates are issued. That trailing dot somehow botches up s_client's checking of that. (Purely a hypothesis; I don't really have anything to back this).

Copy link
Author

Choose a reason for hiding this comment

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

@horazont I would have changed that one sed call for $host to do that, but I haven't played with sed enough to know how to have the REGEX match something, skip over a certain amount upto the last character, and then match the last character, so that sed can discard that away.

Copy link
Owner

Choose a reason for hiding this comment

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

You can just separate the two s commands with a semicolon, like this:

sed 's/[[:digit:]]\+[[:space:]]//;s/\.$//'

Copy link
Author

Choose a reason for hiding this comment

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

@horazont There, commit afa9898 takes care of it.

@horazont
Copy link
Owner

Thanks! Can you please address my question? Then we can get this merged quickly.

@ghost
Copy link
Author

ghost commented Jul 24, 2020

@horazont You're welcome. Actually, thank you, because I was looking for a low level way to read and send messages over XMPP, and now I can inspect the XML files (setting the script to debug mode, which you've very helpfully left in there) to know exactly that.

@horazont horazont self-assigned this Jul 25, 2020
@horazont horazont added the bug Something isn't working label Jul 25, 2020
@horazont horazont merged commit 409c62a into horazont:master Jul 25, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant