Skip to content

Commit

Permalink
ncp-web: fix port checking
Browse files Browse the repository at this point in the history 
Signed-off-by: nachoparker <nacho@ownyourbits.com>
  • Loading branch information
@nachoparker
nachoparker committed May 13, 2021
1 parent bd0c23d commit 1a8ac71
Show file tree
Hide file tree
Showing 2 changed files with 13 additions and 7 deletions.
14 changes: 9 additions & 5 deletions bin/ncp-diag
View file
Original file line number Diff line number Diff line change
Expand Up @@ -64,15 +64,19 @@ echo "internet check|$( ping -W 2 -w 1 -q github.com &>/dev/null && echo ok || e

function is_port_open()
{
local PORT=$1
local port=$1
local public_ip
public_ip="$(curl icanhazip.com 2>/dev/null)" || { echo "closed"; return 1; }

local tmp_file=$(mktemp)
local v=$(wget -T2 -t1 -q --keep-session-cookies --save-cookies $tmp_file https://portchecker.co -O - | grep -oP "_csrf\" value=\"\K.*\"")
if [[ "$v" != "" ]]; then
wget -T2 -t1 -q --load-cookies $tmp_file https://portchecker.co --post-data "port=$PORT&_csrf=${v::-1}" -O - \
local token=$(wget -T2 -t1 -qO- --keep-session-cookies --save-cookies $tmp_file https://portchecker.co | grep -oP "_csrf\" value=\"\K.*\"")
rm $tmp_file

if [[ "${token}" != "" ]]; then
wget -T2 -t1 -qO- --load-cookies $tmp_file https://portchecker.co --post-data "target_ip=${public_ip}&port=${port}&_csrf=${token::-1}" \
| grep -q '<span class="green">open</span>' && { echo "open"; return 1; }
fi
echo "closed"
rm $tmp_file
}

echo "port check 80|$( is_port_open 80 )"
Expand Down
6 changes: 4 additions & 2 deletions changelog.md
View file
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@

[v1.36.1](https://github.com/nextcloud/nextcloudpi/commit/8e7579d) (2021-05-09) lamp: allow only TLSv12 and TLSv13
[v1.36.2](https://github.com/nextcloud/nextcloudpi/commit/fccc04c) (2021-05-11) ncp-web: fix port checking

[v1.36.0](https://github.com/nextcloud/nextcloudpi/commit/24b6018) (2020-09-16) Namecheap dynamic DNS client
[v1.36.1](https://github.com/nextcloud/nextcloudpi/commit/c7f2939) (2021-05-09) lamp: allow only TLSv12 and TLSv13

[v1.36.0 ](https://github.com/nextcloud/nextcloudpi/commit/24b6018) (2020-09-16) Namecheap dynamic DNS client

[v1.35.2 ](https://github.com/nextcloud/nextcloudpi/commit/bfab195) (2021-04-29) ncp-web: fix display of big files for 32 bit

Expand Down

7 comments on commit 1a8ac71

@dsmic
Copy link
Contributor

@dsmic dsmic commented on 1a8ac71 May 13, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

Hmm, why should it be ok to remove the tmp file before it is used in the second request. I did the original patch, and it keeps the cookies for the second request….
As this seems to work, possibly the website does not depend anymore on the cookies?!

@nachoparker
Copy link
Member Author

Choose a reason for hiding this comment

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

duh, that is my mistake

@dsmic
Copy link
Contributor

@dsmic dsmic commented on 1a8ac71 May 13, 2021

Choose a reason for hiding this comment

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

but you are right, I forgot to remove tmp_file in case of open port :) This was probably, what you wanted to fix…

@nachoparker
Copy link
Member Author

@nachoparker nachoparker commented on 1a8ac71 May 13, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

no, I just thought it wasn't used anymore because I wasn't careful. I need somebody to test the fix by doing sudo ncp-update devel. I can't test this right now since my ports are not open with my new ISP

thanks for the good catch

@dsmic
Copy link
Contributor

@dsmic dsmic commented on 1a8ac71 May 13, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

It shows closed to me too.

I think you can not strip the "-O -" in the wget (I don't remember what it was, but I added it and I get displayed open again)

and I had to remove: target_ip=${public_ip}& too ???

public_ip does not give my external (on the router) ip address… I gives a local ip6 address

Then of cause public_ip is not needed anymore?!

This shows open for me again:

function is_port_open()
{
  local port=$1
  local public_ip
  public_ip="$(curl icanhazip.com 2>/dev/null)" || { echo "closed"; return 1; }

  local tmp_file=$(mktemp)
  local token=$(wget -T2 -t1 -q --keep-session-cookies --save-cookies $tmp_file https://portchecker.co -O -| grep -oP "_csrf\" value=\"\K.*\"")

  if [[ "${token}" != "" ]]; then
    wget -T2 -t1 -q --load-cookies $tmp_file https://portchecker.co --post-data "port=${port}&_csrf=${token::-1}" -O -\
    | grep -q '<span class="green">open</span>' && { echo "open"; return 1; }
  fi
  rm $tmp_file
  echo "closed"
}

@nachoparker
Copy link
Member Author

Choose a reason for hiding this comment

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

the public IP is needed, in my case (closed ports) it always returns "open" if I don't use that parameter, and that parameter is what the real website expects. I have not however tested IPv6, so that could be a reason. The -O - is totally fine.

So the thing is that it shows open if the query is incomplete, you can try this by doing is_port_open <some_weird_port_that_is_closed>. It just fails to detect something that is closed unless we use this public_ip parameter.

@nachoparker
Copy link
Member Author

Choose a reason for hiding this comment

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

I removed my previous message asking to do some checks because you know what, it will be easier just to fix this myself in a couple days once my ISP opens my ports and I can test all cases.

Thanks!

Please sign in to comment.