help on TSIGs

[Amitay Isaacs]

On Thu, Oct 16, 2014 at 3:24 AM, Simo <simo at samba.org> wrote:

> On Wed, 2014-10-15 at 16:13 +1100, Amitay Isaacs wrote:
> > Hi Matthieu,
> >
> > On Sun, Oct 12, 2014 at 1:17 PM, Matthieu Patou <mat at samba.org> wrote:
> >         On 10/10/2014 08:20 AM, Simo wrote:
> >                 On Wed, 2014-10-08 at 19:00 -0700, Matthieu Patou
> >                 wrote:
> >                         - if (state->state.sign) { - ret =
> >                         dns_sign_tsig(state->dns, mem_ctx,
> >                         &state->state, + if (state->state->sign) { +
> >                         ret =
> >                         dns_sign_tsig(state->dns, mem_ctx,
> >                         state->state, &state->out_packet,
> >                         0);
> >                 Looks to me a simpler fix would be to pass here
> >                 'state' instead of
> >                 mem_ctx to dns_sign_tsig()
> >         I think it wouldn't be sufficient, you will need to change the
> >         mem_ctx of handle_tkey as well.
> >         Also despite all the variables being called 'state' they have
> >         different type, in the function dns_process_recv (the function
> >         that call dns_sign_tsig) state is a struct dns_process_state
> >         and in dns_server_process_query_send (calling handle_tkey,
> >         where the problem was reported by address sanitizer) it's a
> >         struct dns_server_process_query_state.
> >         It might work but I'm not sure (I haven't checked the life
> >         period of dns_server_process_query_state).
> >
> >         Then I think it's a bad practice to have sub-objects allocated
> >         to an unrelated context, because one day or another it will
> >         bite you because of the different lifetime between the object
> >         and it's sub-objects.
> >
> >         Last but not least, I don't think it should have an impact on
> >         the TSIG stuff, and most probably I'll still have the errors
> >         message in nsupdate.
> >
> >
> >
> >
> > I was able to reproduce this issue even with single NIC.
> >
> >
> > There are two issues:
> >
> >
> > 1. The use-heap-after-free error.
> >
> >
> > A simpler patch is to just fix the memory context for
> > req_state->key_name (attached).
> >
> >
> > 2. tsig verify error
> >
> >
> > Apparently this is a known issue (checked with Andrew Bartlett).  The
> > additional debug information from nsupdate shows that the error is
> > coming from gssapi library.
> >
> >  GSS verify error: GSSAPI error: Major = A token had an invalid
> > Message Integrity Check (MIC), Minor = Success.
> >
> >  tsig key '3061967696.sig-samba-i1.lindom.example.local' (<null>):
> > signature failed to verify(1)
> > ; TSIG error with server: tsig verify failure
>
> Which GSSAPI library was used ? MIT or Heimdal ?
>
> In RHEL/Fedora we backported a couple of patches we sent MIT upstream to
> fix bugs in SPNEGO that affected nsupdate.
>

I tested this on Fedora 20. Samba is built using builtin Heimdal and
nsupdate uses MIT kerberos (krb5-libs-1.11.5-11.fc20.x86_64).

Amitay.