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

nbd-client does not deal with v6-mapped IPv4 addresses #35

Closed
hyphop opened this issue Mar 15, 2016 · 10 comments
Closed

nbd-client does not deal with v6-mapped IPv4 addresses #35

hyphop opened this issue Mar 15, 2016 · 10 comments

Comments

@hyphop
Copy link

hyphop commented Mar 15, 2016

client authorization never have success when call authorized_client(client)

this block not working correctly for me from nbdsrv.c

        if(res->ai_family != addr->sa_family) {
            goto next;
        }

after comment this block all working fine

//      if(res->ai_family != addr->sa_family) {
//          goto next;
//      }
@yoe
Copy link
Member

yoe commented Mar 16, 2016

On Mon, Mar 14, 2016 at 08:03:19PM -0700, hyphop wrote:

client authorization never have success when call authorized_client(client)

this block not working correctly for me from nbdsrv.c

    if(res->ai_family != addr->sa_family) {
        goto next;
    }

after comment this block all working fine

// if(res->ai_family != addr->sa_family) {
// goto next;
// }

That shouldn't happen. The test is there for a reason; we support
multiple protocol families (IPv6, IPv4, unix domain sockets), and it
makes no sense to compare sockets unless the protocol family is the
same.

Could you check what the values of ai_family and sa_family are at this
point, and let me know?

< ron> I mean, the main practical problem with C++, is there's like a dozen
people in the world who think they really understand all of its rules,
and pretty much all of them are just lying to themselves too.
-- #debian-devel, OFTC, 2016-02-12

@hyphop
Copy link
Author

hyphop commented Mar 20, 2016

hi again

ok i try to help u more

add some string for more details to this block

   if(res->ai_family != addr->sa_family) {
       printf("CHECK!!! %d != %d\n", res->ai_family, addr->sa_family

);
goto next;
}

uname -a

Linux odroid 3.10.80-135 #1 SMP PREEMPT Sat Nov 28 13:10:52 BRST 2015
armv7l armv7l armv7l GNU/Linux

cat /etc/nbd-server/allow

192.168.0.0/16
127.0.0.1
::1

./nbd-server -d

** Message: virtstyle ipliteral
** Message: connect from 192.168.7.217, assigned file is
/dev/disk/by-label/odroid-media
CHECK!!! 2 != 10
CHECK!!! 2 != 10
CHECK!!! 2 != 10
CHECK!!! 2 != 10
CHECK!!! 2 != 10
CHECK!!! 2 != 10
** Message: Client '192.168.7.217' is not authorized to access

netstat -anp|grep nbd

tcp6 0 0 :::10809 :::*
LISTEN 3078/nbd-server

nbd-client -N odroid-media odroid /dev/nbd0

Negotiation: ..Error: Read failed: End of file
Exiting.

If i make force ipv4 listen nbd-server all work fine

./nbd-server 192.168.7.168:1111 -d

........

PS: i think u must do address_matches realization more advanced ))))
becouse it not working for ipv6 listen socket and ipv4 connected client

Best regards Artem

2016-03-16 14:32 GMT+07:00 Wouter Verhelst notifications@github.com:

On Mon, Mar 14, 2016 at 08:03:19PM -0700, hyphop wrote:

client authorization never have success when call
authorized_client(client)

this block not working correctly for me from nbdsrv.c

if(res->ai_family != addr->sa_family) {
goto next;
}

after comment this block all working fine

// if(res->ai_family != addr->sa_family) {
// goto next;
// }

That shouldn't happen. The test is there for a reason; we support
multiple protocol families (IPv6, IPv4, unix domain sockets), and it
makes no sense to compare sockets unless the protocol family is the
same.

Could you check what the values of ai_family and sa_family are at this
point, and let me know?

< ron> I mean, the main practical problem with C++, is there's like a
dozen
people in the world who think they really understand all of its rules,
and pretty much all of them are just lying to themselves too.
-- #debian-devel, OFTC, 2016-02-12


You are receiving this because you authored the thread.
Reply to this email directly or view it on GitHub
#35 (comment)

@gwyn17
Copy link

gwyn17 commented May 25, 2016

I have seen the same issue. The work around is to also add IPv6 addresses to your authfile:

10.30.0.0/24
::ffff:0a1e:0000/24

@jamespharaoh
Copy link

I am also seeing this issue. Essentially, if I add an authfile to an export, then it denies all connections. I am listing IP addresses and am confident that this is a bug in the server. I am using the version shipped with Ubuntu Xenial (16.04).

I will be able to work around this, using iptables to restrict access to NBD, but this seems like a serious issue which needs to be addressed.

yoe added a commit that referenced this issue Oct 31, 2016
The "maxconnections" setting considers connections to all exports,
rather than "just" the one where the "maxconnections" setting is set.
This is unexpected and not useful.

Fix by creating a socketpair over which the child asks the parent
process whether a connection to the given export is allowed, rather than
doing the check in the client process.

Closes github #35

Signed-off-by: Wouter Verhelst <w@uter.be>
@yoe
Copy link
Member

yoe commented Oct 31, 2016

Commit was supposed to say it fixed issue #41, not this one...

@yoe
Copy link
Member

yoe commented Nov 4, 2016

The problem about this issue is that in some cases, the kernel will return an IPv6-mapped IPv4 address. Dealing with this is ugly, but not impossible. However, due to other priorities, this has been on the backburner for a while.

I'll deal with it, hopefully soon, but no guarantees.

In the mean time, adding IPv6-mapped IPv4 addresses to the authfile should be a reasonable workaround.

@gwyn17
Copy link

gwyn17 commented Nov 4, 2016

In the interim, it would be nice if there was some way to easily debug the problem. A log message that showed nbd matching IPv6 to IPv4 and failing. This would give a big clue to the work around.

yoe added a commit that referenced this issue Nov 5, 2016
Currently, when we refuse to match an authorized_file entry for address
family mismatches, we don't say anything and instead just exit. That
works, but makes debugging harder.

Add a log message, so that users can at least figure out why things
fail.

Should help for github issue #35, although it's not a full fix.

Signed-off-by: Wouter Verhelst <w@uter.be>
@yoe yoe changed the title client authorization never have success when call authorized_client(client) nbd-client does not deal with v6-mapped IPv4 addresses Mar 14, 2017
@MarkRose
Copy link

Just ran into this one. At least the manual pages should be updated...

@GrahamCobb
Copy link

By the way, the auth file allows v6-mapped addresses to be expressed in "mixed" notation (not just in the hex notation shown in the earlier comment). For example, the v6-mapped version of 192.168.0.220 can be entered as ::ffff:192.168.0.220

However, if you want to include a mask (e.g. /24) you need to add 96. So 192.168.10.0/24 would become ::ffff:192.168.10.0/120.

I have worked up a patch to finally fix this, so that when the address families are different, the test compares the v6 address with the v6-mapped version of the v4 address, including handling the longer mask (either way round).

I think it says somewhere that you don't accept pull requests so I will send it to the mailing list shortly.

GrahamCobb added a commit to GrahamCobb/nbd that referenced this issue Feb 9, 2021
This patch adds support for using IPv4 addresses in the nbd-server auth file
even if the socket provides v6-mapped addresses. It extends the comparison
code in address_matches to handle the two cases where the auth file and the
socket use different address families.

This fixes issue NetworkBlockDevice#35 in the github repository.
@GrahamCobb
Copy link

Patch sent to mailing list 8th Feb: https://lists.debian.org/nbd/2021/02/msg00018.html

No comments so far. If you would prefer a PR please let me know.

@yoe yoe closed this as completed in aeae56e Feb 24, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants