[Samba] Bug in dhcp-dyndns.sh script, A_REC always singleton array

[Kasper Brandt]

Hello
I was directed to discuss this issue here. As I understand the issue with using the unquoted variable is that it expand globs unless noglob is set. E.g.

root at dy3:/# test="b*"
root at dy3:/# a=($test)
root at dy3:/# echo ${a[0]}
bin

It does seem a bit hypothetical that the output of sambatool dns query ... for an A record should contain a glob, but for the sake of robustness it might still be a good idea to find a better solution.

For a single match the output looks like:
root at dy3:/# /bin/samba-tool dns query "dy3" "poizan.dk" "PoiKodi" A "--use-kerberos=required"
  Name=, Records=1, Children=0
    A: 10.13.37.10 (flags=f0, serial=1392054, ttl=900)

For multiple A records the output looks like:
root at dy3:/# /bin/samba-tool dns query "dy3" "poizan.dk" "test" A "--use-kerberos=required"
  Name=, Records=2, Children=0
    A: 10.13.37.100 (flags=f0, serial=1392062, ttl=3600)
    A: 10.13.37.101 (flags=f0, serial=1392063, ttl=3600)

With the "| grep 'A:' | awk '{print $2}' " filtering these becomes respectively

10.13.37.10

and

10.13.37.100
10.13.37.101

So just splitting by lines should work.

Unfortunately just using mapfile with A_REC variable does not quite work as here strings always gets a newline at the end.

root at dy3:/# mapfile -t foo <<< ""
root at dy3:/# echo ${#foo[@]}
1

I would then suggest instead redirecting the query command right away with process substitution

`mapfile -t `A_REC `< <`($SAMBATOOL dns query "${Server}" "${domain}" "${name}" A "$KTYPE" 2>/dev/null | grep 'A:' | awk '{print $2}')
`
`This seems to make it work as intended:

root at dy3:/# mapfile -t A_REC < <(/bin/samba-tool dns query "dy3" "poizan.dk" "doesnotexists" A "--use-kerberos=required" 2>/dev/null | grep 'A:' | awk '{print $2}')
root at dy3:/# echo ${#A_REC[@]}
0
root at dy3:/# mapfile -t A_REC < <(/bin/samba-tool dns query "dy3" "poizan.dk" "PoiKodi" A "--use-kerberos=required" 2>/dev/null | grep 'A:' | awk '{print $2}')
root at dy3:/# echo ${#A_REC[@]}
1
root at dy3:/# echo ${A_REC[0]}
10.13.37.10
root at dy3:/# mapfile -t A_REC < <(/bin/samba-tool dns query "dy3" "poizan.dk" "test" A "--use-kerberos=required" 2>/dev/null | grep 'A:' | awk '{print $2}')
root at dy3:/# echo ${#A_REC[@]}
2
root at dy3:/# echo ${A_REC[0]}
10.13.37.100
root at dy3:/# echo ${A_REC[1]}
10.13.37.101

- Kasper Brandt

On Fri, Aug 11, 2023, at 1:14 PM, Rowland Penny wrote:
> On Fri, 11 Aug 2023 02:52:59 +0200
> Kasper Brandt via samba-team <samba-team at lists.samba.org> wrote:
> 
> > Hello
> > I encountered a bug in the dhcp-dyndns.sh script embedded in the site
> > at
> > https://wiki.samba.org/index.php/Configure_DHCP_to_update_DNS_records
> > 
> > Version 0.9.5 (introduced in this revision:
> > https://wiki.samba.org/index.php?title=Configure_DHCP_to_update_DNS_records&oldid=18255)
> > introduced the following
> > 
> > > # turn A_REC into an array
> > > A_REC=("$A_REC")
> > 
> > (a bash array wasn't used in the previous version)
> > This is however incorrect as this will always create an array of one
> > element due to the quotes. The branch immediately after that should
> > only be taken if the A record didn't exist is never taken. Instead
> > the branch that should handle 1 existing record gets taken and
> > something like
> > > /bin/samba-tool dns delete "dy3" "poizan.dk" "PoiKodi" A ""
> > > "--use-kerberos=required"
> > gets executed (note the empty data field), which fails with an
> > invalid parameter error.
> > 
> > The line should probably just have been
> > > A_REC=($A_REC)
> > which splits the string into array elements by whitespace.
> > 
> > Someone more familiar with the script might want to verify my fix, or
> > I can just make the change myself if I get wiki access if that is
> > acceptable? At least I am currently using the fix without issue.
> > 
> > - Kasper Brandt
> 
> Problem is, whilst it may be a bug, the suggested fix doesn't appear to
> be the fix. If you do as Kasper suggests and remove the quotation marks
> and then run shellcheck against the script, you get this:
> 
> In dhcp-dyndns.sh.0.9.5 line 233:
> A_REC=($A_REC)
>                        ^-- SC2206: Quote to prevent word splitting, or split robustly with mapfile or read -a.
> 
> Which is why I probably took the easy option and added the quotation
> marks, looks like using a mapfile or read needs to be investigated.
> 
> Can I also point out that the last lines on that wikipage are these:
> 
> Any questions or problems, ask on the Samba mailing list.
> 
> Do not log a bug report on Samba bugzilla for any problems you have
> with this set up, ask on the samba mailing list.
> 
> Rowland
> 
> 
>