[PATCH] Re: resolv_wrapper URI RR support - name compression

[Matt Rogers]

On Fri, 2016-08-26 at 08:23 +0200, Andreas Schneider wrote:
> On Wednesday, 24 August 2016 10:53:17 CEST Matt Rogers wrote:
> > 
> > On Wed, 2016-08-24 at 08:10 +0200, Andreas Schneider wrote:
> > > 
> > > On Tuesday, 23 August 2016 11:41:45 CEST Matt Rogers wrote:
> > > > 
> > > > On Tue, 2016-08-23 at 15:54 +0200, Andreas Schneider wrote:
> > > > > 
> > > > > On Monday, 22 August 2016 17:52:25 CEST Matt Rogers wrote:
> > > > > > 
> > > > > > Hi,
> > > > > > 
> > > > > > I'm using resolv_wrapper from master at 02535da4 to test
> > > > > > krb5
> > > > > > URI
> > > > > > RR
> > > > > > lookups. Similar to the SRV records, rwrap_fake_uri() runs
> > > > > > ns_name_compress() on the result, however URI records
> > > > > > contain a
> > > > > > formatted entry and not a DNS name, so I don't think that
> > > > > > the
> > > > > > call
> > > > > > to
> > > > > > ns_name_compress() is valid here.  The result of running
> > > > > > ns_name_compress() on the URI record (which appears as
> > > > > > "krb5srv:m:kkdcp:https://kdc/path") is that it adds a " "
> > > > > > to
> > > > > > the
> > > > > > beginning that does not get removed with a subsequent
> > > > > > uncompress.
> > > > > > Any
> > > > > > objections to removing the ns_name_compress/uncompress on
> > > > > > URI
> > > > > > RR
> > > > > > results?
> > > > > 
> > > > > This means the provided test is incorrect. We need a test
> > > > > which
> > > > > works
> > > > > in a 
> > > > > real world scenario and also with resolv_wrapper to verify
> > > > > that
> > > > > resolv_wrapper 
> > > > > is doing everything correctly.
> > > > > 
> > > > > Could you use the current test, modify it and make sure it
> > > > > works
> > > > > in
> > > > > the real 
> > > > > world?
> > > > > 
> > > > > Then we can fix resolve_wrapper ...
> > > > 
> > > > I've attached patches for removing the name compression from
> > > > the
> > > > URI
> > > > result as well as the tests. This gives a correct result for
> > > > the
> > > > real
> > > > world test.
> > > 
> > > Hey Matt,
> > > 
> > > thanks for the patches!
> > > 
> > > Please add:
> > > 
> > > size_t uri_len;
> > > 
> > > uri_len = strlen(rr->rrdata.uri_rec.uri) + 1;
> > > 
> > > and then use uri_len as we are using it twice.
> > > 
> > > Also you need to sign your work. See
> > > 
> > > https://www.samba.org/samba/devel/copyright-policy.html
> > 
> > Thanks, I've attached the new patches.
> > 
> > > 
> > > > 
> > > > Another issue is that it can only return a single answer for a
> > > > URI
> > > > query, since it reaches the first matching URI key in the host
> > > > file
> > > > and
> > > > stops. The krb5 URI lookup collects all answers from a query
> > > > and
> > > > parses
> > > > the URIs, sorting the results accordingly, so it would be great
> > > > to
> > > > be
> > > > able to have a fake hosts file with all of the test URIs listed
> > > > and
> > > > not
> > > > have to test them one by one.
> > > 
> > > We are happy to accept patches to allow to return more than one
> > > URI
> > > 
> > > :)
> > 
> > Good, I'll be working on that :)
> 
> git am ~/Downloads/p/resolv_wrapper-uri-comp.patch
> Patch format detection failed.
> 
> Did you create the patches with 'git format-patch -2' or user 'git
> format-
> patch -2 --stdout > rwrap.patch'
> 

Whoops, sorry about that. I've attached the correctly formatted
patches. 

Matt

> 
> Cheers,
> 
> 
> 	-- andreas
> 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-Remove-name-compression-from-URI-RR.patch
Type: text/x-patch
Size: 1645 bytes
Desc: not available
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20160826/1b32f642/0001-Remove-name-compression-from-URI-RR.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0002-Remove-name-decompression-from-URI-RR-tests.patch
Type: text/x-patch
Size: 2889 bytes
Desc: not available
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20160826/1b32f642/0002-Remove-name-decompression-from-URI-RR-tests.bin>