[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Re: [PATCH] nbd-server: handle auth for v6-mapped IPv4 addresses



Hi Graham,

Sorry, I didn't have time to review this earlier due to other
commitments (I'm a member of the FOSDEM orga team).

On Wed, Feb 17, 2021 at 11:03:43AM +0000, Graham Cobb wrote:
> On 08/02/2021 23:42, Graham R. Cobb wrote:
> > 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 #35 in the github repository.
> > 
> > Signed-off-by: Graham R. Cobb <g.nbd@cobb.uk.net>
> 
> No comments or reviewed-by so far. I am not sure of the protocol for
> this mailing list, but this is a gentle ping :-)
> 
> > ---
> >  nbdsrv.c | 30 ++++++++++++++++++++++++------
> >  1 file changed, 24 insertions(+), 6 deletions(-)
> > 
> > diff --git a/nbdsrv.c b/nbdsrv.c
> > index 168d425..01ba1b2 100644
> > --- a/nbdsrv.c
> > +++ b/nbdsrv.c
> > @@ -35,6 +35,9 @@ bool address_matches(const char* mask, const struct sockaddr* addr, GError** err
> >  	char privmask[strlen(mask)+1];
> >  	int masklen;
> >  	int addrlen = addr->sa_family == AF_INET ? 4 : 16;
> > +#define IPV4_MAP_PREFIX 12
> > +	uint8_t ipv4_mapped[IPV4_MAP_PREFIX+4] = {0, 0, 0, 0, 0, 0, 0, 0, 0, 0,
> > +		255, 255, 0, 0, 0, 0};
> >  
> >  	strcpy(privmask, mask);
> >  
> > @@ -61,12 +64,9 @@ bool address_matches(const char* mask, const struct sockaddr* addr, GError** err
> >  		uint8_t* byte_t;
> >  		uint8_t mask = 0;
> >  		int len_left = masklen;
> > -		if(res->ai_family != addr->sa_family) {
> > -			msg(LOG_DEBUG, "client address does not match %d/%d: address family mismatch (IPv4 vs IPv6?)",
> > -			    (int)res->ai_family, (int)addr->sa_family);
> > -			goto next;
> > -		}
> > -		switch(addr->sa_family) {
> > +		if(res->ai_family == addr->sa_family) {
> > +			/* Both addresses are the same address family so do a simple comparison */
> > +			switch(addr->sa_family) {
> >  			case AF_INET:
> >  				byte_s = (const uint8_t*)(&(((struct sockaddr_in*)addr)->sin_addr));
> >  				byte_t = (uint8_t*)(&(((struct sockaddr_in*)(res->ai_addr))->sin_addr));
> > @@ -75,6 +75,24 @@ bool address_matches(const char* mask, const struct sockaddr* addr, GError** err
> >  				byte_s = (const uint8_t*)(&(((struct sockaddr_in6*)addr)->sin6_addr));
> >  				byte_t = (uint8_t*)(&(((struct sockaddr_in6*)(res->ai_addr))->sin6_addr));
> >  				break;
> > +			}

You confused me a bit here because you drop the "switch" line above, but
then change indentation rules ;-)

But okay, it makes sense, now that I look at it closer.

Applied, thanks.

-- 
To the thief who stole my anti-depressants: I hope you're happy

  -- seen somewhere on the Internet on a photo of a billboard


Reply to: