This new patchset obsoletes the patch sent earlier today.
The first and third patch are obvious.
The second patch implement a new command line option -N that takes
an on|off argument.
When 'on' is specified the RPC Server name as passed from the kernel
to rpc.gssd is check to see if it really is an actual IP address, if it
is the current code is executed (and reverse resolution happens),
otherwise the name used at the mount option is used directly w/o any
DNS resolution to construct the GSSAPI name.
Avoiding Reverse name resolution helps making the system work when PTR records
cannot be properly set on a network (because the amdin does not control DNS for
example) and also avoids a potential MITM attack (as explained early on in the
original patch thread).
Simo Sorce (3):
Fix segfault when using -R option
Avoid reverse resolution for server name
Document new -N option
utils/gssd/gss_util.h | 2 ++
utils/gssd/gssd.c | 18 ++++++++++++++++--
utils/gssd/gssd.man | 11 ++++++++++-
utils/gssd/gssd_proc.c | 25 +++++++++++++++++++++----
4 files changed, 49 insertions(+), 7 deletions(-)
On 02/04/13 13:49, Simo Sorce wrote:
> The getopt string did not add : after the R option resulting in a sefgault
> whenever -R was used as optarg is NULL and it is dereferenced.
>
> Signed-off-by: Simo Sorce <[email protected]>
committed...
steved.
> ---
> utils/gssd/gssd.c | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/utils/gssd/gssd.c b/utils/gssd/gssd.c
> index 0be251781bacaa562270f773341126bc95ca6b45..07b1e52e6b84e9bcba96e7a63b0505ca7823482a 100644
> --- a/utils/gssd/gssd.c
> +++ b/utils/gssd/gssd.c
> @@ -102,7 +102,7 @@ main(int argc, char *argv[])
> char *progname;
>
> memset(ccachesearch, 0, sizeof(ccachesearch));
> - while ((opt = getopt(argc, argv, "fvrlmnMp:k:d:t:R")) != -1) {
> + while ((opt = getopt(argc, argv, "fvrlmnMp:k:d:t:R:")) != -1) {
> switch (opt) {
> case 'f':
> fg = 1;
>
The getopt string did not add : after the R option resulting in a sefgault
whenever -R was used as optarg is NULL and it is dereferenced.
Signed-off-by: Simo Sorce <[email protected]>
---
utils/gssd/gssd.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/utils/gssd/gssd.c b/utils/gssd/gssd.c
index 0be251781bacaa562270f773341126bc95ca6b45..07b1e52e6b84e9bcba96e7a63b0505ca7823482a 100644
--- a/utils/gssd/gssd.c
+++ b/utils/gssd/gssd.c
@@ -102,7 +102,7 @@ main(int argc, char *argv[])
char *progname;
memset(ccachesearch, 0, sizeof(ccachesearch));
- while ((opt = getopt(argc, argv, "fvrlmnMp:k:d:t:R")) != -1) {
+ while ((opt = getopt(argc, argv, "fvrlmnMp:k:d:t:R:")) != -1) {
switch (opt) {
case 'f':
fg = 1;
--
1.7.1
Options are not in alphabetical order but -z/-Z clearly always come last.
Signed-off-by: Simo Sorce <[email protected]>
---
utils/gssd/gssd.man | 13 ++++++++++++-
1 files changed, 12 insertions(+), 1 deletions(-)
diff --git a/utils/gssd/gssd.man b/utils/gssd/gssd.man
index 79d9bf91ac6b976c57d167e60d07f828a3ff5b1f..7918c2a0ff76c3918449cf3e1420f0a289929ac1 100644
--- a/utils/gssd/gssd.man
+++ b/utils/gssd/gssd.man
@@ -8,7 +8,7 @@
rpc.gssd \- RPCSEC_GSS daemon
.SH SYNOPSIS
.B rpc.gssd
-.RB [ \-fMnlvr ]
+.RB [ \-fMnlvrzZ ]
.RB [ \-k
.IR keytab ]
.RB [ \-p
@@ -266,6 +266,17 @@ new kernel contexts to be negotiated after
seconds, which allows changing Kerberos tickets and identities frequently.
The default is no explicit timeout, which means the kernel context will live
the lifetime of the Kerberos service ticket used in its creation.
+.TP
+.B -z
+This option tries to avoid DNS Reverse (PTR) lookups for determining the
+server name to pass to GSSAPI if the name passed at mount point is not an IP
+address. Currently off by default for compatibility reasons.
+.TP
+.B -Z
+This is the inverse of
+.B -z
+and forces the use of DNS Reverse resolution of the server's IP address to
+retrieve the server name to use in GSAPI authentication.
.SH SEE ALSO
.BR rpc.svcgssd (8),
.BR kerberos (1),
--
1.7.1
On Tue, 2013-04-09 at 13:35 -0400, Steve Dickson wrote:
>
> On 09/04/13 13:25, Simo Sorce wrote:
> > On Tue, 2013-04-09 at 13:15 -0400, Steve Dickson wrote:
> >>
> >> On 08/04/13 10:08, Simo Sorce wrote:
> >>> On Mon, 2013-04-08 at 09:39 -0400, Steve Dickson wrote:
> >>>>
> >>>> On 02/04/13 15:32, Simo Sorce wrote:
> >>>>> A NFS client should be able to work properly even if the DNS Reverse record
> >>>>> for the server is not set. There is no excuse to forcefully prevent that
> >>>>> from working when it can.
> >>>>>
> >>>>> This patch adds a new pair of options (-z/-Z) that allow to turn on/off
> >>>>> DNS reverse resolution for determining the server name to use with GSSAPI.
> >>>> Again, please tell me why we need the -Z flag when that is the default?
> >>>
> >>> The idea is to switch the default in the code at some point, so then -Z
> >>> will be needed to get back to the original behavior.
> >> I'm thinking that's what major version number changes are for... not flags...
> >>
> >>>
> >>> The idea is that by having both flags a distribution may choose to
> >>> decide now what behavior they want and use the relative flag. Then even
> >>> if we change the default their configuration will not "break".
> >> I'll do the work to remove the option and repost the patches..
> >
> > As you wish, I do not have hard preferences, should we take the bait and
> > also by default *not* do PTR lookups ?
> I was thinking no. Leaves the default as is and used the -z to avoid the
> lookup...
>
> I'm struggling with how big of a problem this really is, so why should be break
> existing environments? I'm no DNS expert but I thinking not have PTR is
> a DNS config issue... but again I'm no expert...
Read this:
http://ssimo.org/blog/id_015.html
Simo.
--
Simo Sorce * Red Hat, Inc * New York
> -----Original Message-----
> From: [email protected] [mailto:linux-nfs-
> [email protected]] On Behalf Of Simo Sorce
> Sent: Tuesday, April 02, 2013 1:49 PM
> To: Linux NFS Mailing list
> Subject: [PATCH 2/3] Avoid reverse resolution for server name
>
> A NFS client should be able to work properly even if the DNS Reverse record
> for the server is not set. There is no excuse to forcefully prevent that from
> working when it can.
Note that rpc.statd has the same limitation.
> This patch adds a new -N option that takes an 'on' or 'off' argument and
> controls whether forced PTR resolution is avoided (on) or performed (off)
'-N' already has a very different meaning on rpc.mountd and rpc.nfsd. It might therefore be better to choose a different name to avoid confusion.
Also, why do a single option with an 'on/off' argument instead of just choosing 2 options ?
Trond
On Mon, 2013-04-08 at 09:39 -0400, Steve Dickson wrote:
>
> On 02/04/13 15:32, Simo Sorce wrote:
> > A NFS client should be able to work properly even if the DNS Reverse record
> > for the server is not set. There is no excuse to forcefully prevent that
> > from working when it can.
> >
> > This patch adds a new pair of options (-z/-Z) that allow to turn on/off
> > DNS reverse resolution for determining the server name to use with GSSAPI.
> Again, please tell me why we need the -Z flag when that is the default?
The idea is to switch the default in the code at some point, so then -Z
will be needed to get back to the original behavior.
The idea is that by having both flags a distribution may choose to
decide now what behavior they want and use the relative flag. Then even
if we change the default their configuration will not "break".
Simo.
--
Simo Sorce * Red Hat, Inc * New York
On 09/04/13 15:22, J. Bruce Fields wrote:
> On Tue, Apr 09, 2013 at 03:12:56PM -0400, Steve Dickson wrote:
>>
>>
>> On 09/04/13 14:54, J. Bruce Fields wrote:
>>> Argh, no, one away or another the default needs to be to not do the PTR
>>> lookup.
>> Fine...
>>
>>>
>>> The transition Simo's using was Jeff's suggestion. Let's just stick to
>>> that if we don't have a good reason.
>> Yeah... I would like to avoid adding to flags... I don't think both are
>> needed.
>
> So, no flags.
>
>>> (But I don't have strong opinions about how to do it either. I'd
>>> actually be OK with being harsh and just switching to the new behavior
>>> without any option.)
>> My crutch is I'm not a big DNS guy so I'm not sure how much breakage
>> would occur... So I would rather be on the safe side and give people
>> a way to go back...
>
> So, yes to flags. I'm confused!
Join the club! ;-)
>
> I guess we can be moderately harsh: switch to the new default and
> provide only a flag to restore the old default for whoever wants it, but
> not a flag to specify the new default. Is that what you mean?
Yes... This makes sense to me...
steved.
On Wed, 2013-04-03 at 10:56 -0400, J. Bruce Fields wrote:
> On Wed, Apr 03, 2013 at 02:35:48PM +0000, Myklebust, Trond wrote:
> > On Wed, 2013-04-03 at 10:20 -0400, J. Bruce Fields wrote:
> > > On Tue, Apr 02, 2013 at 03:32:29PM -0400, Simo Sorce wrote:
> > > > Options are not in alphabetical order but -z/-Z clearly always come last.
> > > >
> > > > Signed-off-by: Simo Sorce <[email protected]>
> > > > ---
> > > > utils/gssd/gssd.man | 13 ++++++++++++-
> > > > 1 files changed, 12 insertions(+), 1 deletions(-)
> > > >
> > > > diff --git a/utils/gssd/gssd.man b/utils/gssd/gssd.man
> > > > index 79d9bf91ac6b976c57d167e60d07f828a3ff5b1f..7918c2a0ff76c3918449cf3e1420f0a289929ac1 100644
> > > > --- a/utils/gssd/gssd.man
> > > > +++ b/utils/gssd/gssd.man
> > > > @@ -8,7 +8,7 @@
> > > > rpc.gssd \- RPCSEC_GSS daemon
> > > > .SH SYNOPSIS
> > > > .B rpc.gssd
> > > > -.RB [ \-fMnlvr ]
> > > > +.RB [ \-fMnlvrzZ ]
> > > > .RB [ \-k
> > > > .IR keytab ]
> > > > .RB [ \-p
> > > > @@ -266,6 +266,17 @@ new kernel contexts to be negotiated after
> > > > seconds, which allows changing Kerberos tickets and identities frequently.
> > > > The default is no explicit timeout, which means the kernel context will live
> > > > the lifetime of the Kerberos service ticket used in its creation.
> > > > +.TP
> > > > +.B -z
> > > > +This option tries to avoid DNS Reverse (PTR) lookups for determining the
> > > > +server name to pass to GSSAPI if the name passed at mount point is not an IP
> > > > +address. Currently off by default for compatibility reasons.
> > > > +.TP
> > > > +.B -Z
> > > > +This is the inverse of
> > > > +.B -z
> > > > +and forces the use of DNS Reverse resolution of the server's IP address to
> > > > +retrieve the server name to use in GSAPI authentication.
> > >
> > > By the way I think with the "new" upcall, gssd ignores whatever it got
> > > out of the info file if the "target=" parameter is provided in the
> > > upcall.
> >
> > Correct.
> >
> > > (But looking at the code I think that's only used in the nfsv4.0
> > > callback case, and isn't worth mentioning here.)
> >
> > Wrong. It is also used for NFSv4 and NFSv4.1 state management.
> > IOW: SETCLIENTID/RENEW and for NFSv4.1 EXCHANGE_ID/SEQUENCE; anything
> > that uses clp->cl_machine_cred.
>
> I was talk about "target=", but I believe you're talking about
> "service=".
>
> The former is a server name (myserver.example.com), the latter a service
> name (nfs).
Right, but gssd_refresh_krb5_machine_credential combines both in order
to create the keytab entry.
--
Trond Myklebust
Linux NFS client maintainer
NetApp
[email protected]
http://www.netapp.com
On Tue, Apr 09, 2013 at 03:12:56PM -0400, Steve Dickson wrote:
>
>
> On 09/04/13 14:54, J. Bruce Fields wrote:
> > Argh, no, one away or another the default needs to be to not do the PTR
> > lookup.
> Fine...
>
> >
> > The transition Simo's using was Jeff's suggestion. Let's just stick to
> > that if we don't have a good reason.
> Yeah... I would like to avoid adding to flags... I don't think both are
> needed.
So, no flags.
> > (But I don't have strong opinions about how to do it either. I'd
> > actually be OK with being harsh and just switching to the new behavior
> > without any option.)
> My crutch is I'm not a big DNS guy so I'm not sure how much breakage
> would occur... So I would rather be on the safe side and give people
> a way to go back...
So, yes to flags. I'm confused!
I guess we can be moderately harsh: switch to the new default and
provide only a flag to restore the old default for whoever wants it, but
not a flag to specify the new default. Is that what you mean?
--b.
On Wed, Apr 03, 2013 at 02:35:48PM +0000, Myklebust, Trond wrote:
> On Wed, 2013-04-03 at 10:20 -0400, J. Bruce Fields wrote:
> > On Tue, Apr 02, 2013 at 03:32:29PM -0400, Simo Sorce wrote:
> > > Options are not in alphabetical order but -z/-Z clearly always come last.
> > >
> > > Signed-off-by: Simo Sorce <[email protected]>
> > > ---
> > > utils/gssd/gssd.man | 13 ++++++++++++-
> > > 1 files changed, 12 insertions(+), 1 deletions(-)
> > >
> > > diff --git a/utils/gssd/gssd.man b/utils/gssd/gssd.man
> > > index 79d9bf91ac6b976c57d167e60d07f828a3ff5b1f..7918c2a0ff76c3918449cf3e1420f0a289929ac1 100644
> > > --- a/utils/gssd/gssd.man
> > > +++ b/utils/gssd/gssd.man
> > > @@ -8,7 +8,7 @@
> > > rpc.gssd \- RPCSEC_GSS daemon
> > > .SH SYNOPSIS
> > > .B rpc.gssd
> > > -.RB [ \-fMnlvr ]
> > > +.RB [ \-fMnlvrzZ ]
> > > .RB [ \-k
> > > .IR keytab ]
> > > .RB [ \-p
> > > @@ -266,6 +266,17 @@ new kernel contexts to be negotiated after
> > > seconds, which allows changing Kerberos tickets and identities frequently.
> > > The default is no explicit timeout, which means the kernel context will live
> > > the lifetime of the Kerberos service ticket used in its creation.
> > > +.TP
> > > +.B -z
> > > +This option tries to avoid DNS Reverse (PTR) lookups for determining the
> > > +server name to pass to GSSAPI if the name passed at mount point is not an IP
> > > +address. Currently off by default for compatibility reasons.
> > > +.TP
> > > +.B -Z
> > > +This is the inverse of
> > > +.B -z
> > > +and forces the use of DNS Reverse resolution of the server's IP address to
> > > +retrieve the server name to use in GSAPI authentication.
> >
> > By the way I think with the "new" upcall, gssd ignores whatever it got
> > out of the info file if the "target=" parameter is provided in the
> > upcall.
>
> Correct.
>
> > (But looking at the code I think that's only used in the nfsv4.0
> > callback case, and isn't worth mentioning here.)
>
> Wrong. It is also used for NFSv4 and NFSv4.1 state management.
> IOW: SETCLIENTID/RENEW and for NFSv4.1 EXCHANGE_ID/SEQUENCE; anything
> that uses clp->cl_machine_cred.
I was talk about "target=", but I believe you're talking about
"service=".
The former is a server name (myserver.example.com), the latter a service
name (nfs).
--b.
On 08/04/13 10:08, Simo Sorce wrote:
> On Mon, 2013-04-08 at 09:39 -0400, Steve Dickson wrote:
>>
>> On 02/04/13 15:32, Simo Sorce wrote:
>>> A NFS client should be able to work properly even if the DNS Reverse record
>>> for the server is not set. There is no excuse to forcefully prevent that
>>> from working when it can.
>>>
>>> This patch adds a new pair of options (-z/-Z) that allow to turn on/off
>>> DNS reverse resolution for determining the server name to use with GSSAPI.
>> Again, please tell me why we need the -Z flag when that is the default?
>
> The idea is to switch the default in the code at some point, so then -Z
> will be needed to get back to the original behavior.
I'm thinking that's what major version number changes are for... not flags...
>
> The idea is that by having both flags a distribution may choose to
> decide now what behavior they want and use the relative flag. Then even
> if we change the default their configuration will not "break".
I'll do the work to remove the option and repost the patches..
steved.
On Tue, Apr 02, 2013 at 03:32:29PM -0400, Simo Sorce wrote:
> Options are not in alphabetical order but -z/-Z clearly always come last.
>
> Signed-off-by: Simo Sorce <[email protected]>
> ---
> utils/gssd/gssd.man | 13 ++++++++++++-
> 1 files changed, 12 insertions(+), 1 deletions(-)
>
> diff --git a/utils/gssd/gssd.man b/utils/gssd/gssd.man
> index 79d9bf91ac6b976c57d167e60d07f828a3ff5b1f..7918c2a0ff76c3918449cf3e1420f0a289929ac1 100644
> --- a/utils/gssd/gssd.man
> +++ b/utils/gssd/gssd.man
> @@ -8,7 +8,7 @@
> rpc.gssd \- RPCSEC_GSS daemon
> .SH SYNOPSIS
> .B rpc.gssd
> -.RB [ \-fMnlvr ]
> +.RB [ \-fMnlvrzZ ]
> .RB [ \-k
> .IR keytab ]
> .RB [ \-p
> @@ -266,6 +266,17 @@ new kernel contexts to be negotiated after
> seconds, which allows changing Kerberos tickets and identities frequently.
> The default is no explicit timeout, which means the kernel context will live
> the lifetime of the Kerberos service ticket used in its creation.
> +.TP
> +.B -z
> +This option tries to avoid DNS Reverse (PTR) lookups for determining the
> +server name to pass to GSSAPI if the name passed at mount point is not an IP
> +address. Currently off by default for compatibility reasons.
> +.TP
> +.B -Z
> +This is the inverse of
> +.B -z
> +and forces the use of DNS Reverse resolution of the server's IP address to
> +retrieve the server name to use in GSAPI authentication.
By the way I think with the "new" upcall, gssd ignores whatever it got
out of the info file if the "target=" parameter is provided in the
upcall.
(But looking at the code I think that's only used in the nfsv4.0
callback case, and isn't worth mentioning here.)
--b.
> .SH SEE ALSO
> .BR rpc.svcgssd (8),
> .BR kerberos (1),
> --
> 1.7.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, 02 Apr 2013 14:08:56 -0400
Simo Sorce <[email protected]> wrote:
> On Tue, 2013-04-02 at 17:58 +0000, Myklebust, Trond wrote:
> > > -----Original Message-----
> > > From: [email protected] [mailto:linux-nfs-
> > > [email protected]] On Behalf Of Simo Sorce
> > > Sent: Tuesday, April 02, 2013 1:49 PM
> > > To: Linux NFS Mailing list
> > > Subject: [PATCH 2/3] Avoid reverse resolution for server name
> > >
> > > A NFS client should be able to work properly even if the DNS Reverse record
> > > for the server is not set. There is no excuse to forcefully prevent that from
> > > working when it can.
> >
> > Note that rpc.statd has the same limitation.
> >
> > > This patch adds a new -N option that takes an 'on' or 'off' argument and
> > > controls whether forced PTR resolution is avoided (on) or performed (off)
> >
> > '-N' already has a very different meaning on rpc.mountd and rpc.nfsd. It might therefore be better to choose a different name to avoid confusion.
> > Also, why do a single option with an 'on/off' argument instead of just choosing 2 options ?
> >
> Well Jeff seemed to suggest that way, I do not have any preference, can
> you please give me 2 letters to use ?
>
> Meanwhile I'll check rpc.statd as well.
>
> Simo.
>
FWIW, doesn't much matter to me...
You just want some clear way to designate what behavior you want, and
some way to tell whether it was specified at all (so you can disable
the warning if it was).
--
Jeff Layton <[email protected]>
On Tue, 2013-04-09 at 13:15 -0400, Steve Dickson wrote:
>
> On 08/04/13 10:08, Simo Sorce wrote:
> > On Mon, 2013-04-08 at 09:39 -0400, Steve Dickson wrote:
> >>
> >> On 02/04/13 15:32, Simo Sorce wrote:
> >>> A NFS client should be able to work properly even if the DNS Reverse record
> >>> for the server is not set. There is no excuse to forcefully prevent that
> >>> from working when it can.
> >>>
> >>> This patch adds a new pair of options (-z/-Z) that allow to turn on/off
> >>> DNS reverse resolution for determining the server name to use with GSSAPI.
> >> Again, please tell me why we need the -Z flag when that is the default?
> >
> > The idea is to switch the default in the code at some point, so then -Z
> > will be needed to get back to the original behavior.
> I'm thinking that's what major version number changes are for... not flags...
>
> >
> > The idea is that by having both flags a distribution may choose to
> > decide now what behavior they want and use the relative flag. Then even
> > if we change the default their configuration will not "break".
> I'll do the work to remove the option and repost the patches..
As you wish, I do not have hard preferences, should we take the bait and
also by default *not* do PTR lookups ?
Simo.
--
Simo Sorce * Red Hat, Inc * New York
A NFS client should be able to work properly even if the DNS Reverse record
for the server is not set. There is no excuse to forcefully prevent that
from working when it can.
This patch adds a new pair of options (-z/-Z) that allow to turn on/off
DNS reverse resolution for determining the server name to use with GSSAPI.
To avoid breaking current behavior the option defaults to off by default,
ideally we will turn this on by default after a transition period.
Signed-off-by: Simo Sorce <[email protected]>
---
utils/gssd/gss_util.h | 2 ++
utils/gssd/gssd.c | 10 ++++++++--
utils/gssd/gssd_proc.c | 25 +++++++++++++++++++++----
3 files changed, 31 insertions(+), 6 deletions(-)
diff --git a/utils/gssd/gss_util.h b/utils/gssd/gss_util.h
index aa9f77806075f9ab67a7763a75a010369ba2d1b9..663fb0998bede6144118f890b9311ee8687176e3 100644
--- a/utils/gssd/gss_util.h
+++ b/utils/gssd/gss_util.h
@@ -52,4 +52,6 @@ int gssd_check_mechs(void);
gss_krb5_set_allowable_enctypes(min, cred, num, types)
#endif
+extern int avoid_ptr;
+
#endif /* _GSS_UTIL_H_ */
diff --git a/utils/gssd/gssd.c b/utils/gssd/gssd.c
index 07b1e52e6b84e9bcba96e7a63b0505ca7823482a..1f0ac0c47667c42ed03e271cb18b6124165e5d5f 100644
--- a/utils/gssd/gssd.c
+++ b/utils/gssd/gssd.c
@@ -85,7 +85,7 @@ sig_hup(int signal)
static void
usage(char *progname)
{
- fprintf(stderr, "usage: %s [-f] [-l] [-M] [-n] [-v] [-r] [-p pipefsdir] [-k keytab] [-d ccachedir] [-t timeout] [-R preferred realm]\n",
+ fprintf(stderr, "usage: %s [-f] [-l] [-M] [-n] [-v] [-r] [-p pipefsdir] [-k keytab] [-d ccachedir] [-t timeout] [-R preferred realm] [-z] [-Z]\n",
progname);
exit(1);
}
@@ -102,7 +102,7 @@ main(int argc, char *argv[])
char *progname;
memset(ccachesearch, 0, sizeof(ccachesearch));
- while ((opt = getopt(argc, argv, "fvrlmnMp:k:d:t:R:")) != -1) {
+ while ((opt = getopt(argc, argv, "fvrlmnMp:k:d:t:R:zZ")) != -1) {
switch (opt) {
case 'f':
fg = 1;
@@ -150,6 +150,12 @@ main(int argc, char *argv[])
errx(1, "Encryption type limits not supported by Kerberos libraries.");
#endif
break;
+ case 'z':
+ avoid_ptr = 1;
+ break;
+ case 'Z':
+ avoid_ptr = 0;
+ break;
default:
usage(argv[0]);
break;
diff --git a/utils/gssd/gssd_proc.c b/utils/gssd/gssd_proc.c
index ea01e92e4565670b97dea1a936d2f0dbdc7c4610..21d4e1d78eb54d177626cb0a19b9de4e93e0a20d 100644
--- a/utils/gssd/gssd_proc.c
+++ b/utils/gssd/gssd_proc.c
@@ -67,6 +67,7 @@
#include <errno.h>
#include <gssapi/gssapi.h>
#include <netdb.h>
+#include <ctype.h>
#include "gssd.h"
#include "err_util.h"
@@ -107,6 +108,8 @@ struct pollfd * pollarray;
unsigned long pollsize; /* the size of pollaray (in pollfd's) */
+int avoid_ptr = 0;
+
/*
* convert a presentation address string to a sockaddr_storage struct. Returns
* true on success or false on failure.
@@ -165,12 +168,26 @@ addrstr_to_sockaddr(struct sockaddr *sa, const char *node, const char *port)
* convert a sockaddr to a hostname
*/
static char *
-sockaddr_to_hostname(const struct sockaddr *sa, const char *addr)
+get_servername(const char *name, const struct sockaddr *sa, const char *addr)
{
socklen_t addrlen;
int err;
char *hostname;
char hbuf[NI_MAXHOST];
+ unsigned char buf[sizeof(struct in6_addr)];
+ int do_ptr_lookup = 0;
+
+ if (avoid_ptr) {
+ /* try to determine if this is a name, or an IP address.
+ * If it is an IP fallback to a PTR lookup */
+ if (strchr(name, '.') && inet_pton(AF_INET, name, buf) == 1)
+ do_ptr_lookup = 1; /* IPv4 */
+ else if (strchr(name, ':') && inet_pton(AF_INET6, name, buf) == 1)
+ do_ptr_lookup = 1; /* or IPv6 */
+ if (!do_ptr_lookup) {
+ return strdup(name);
+ }
+ }
switch (sa->sa_family) {
case AF_INET:
@@ -208,7 +225,7 @@ read_service_info(char *info_file_name, char **servicename, char **servername,
struct sockaddr *addr) {
#define INFOBUFLEN 256
char buf[INFOBUFLEN + 1];
- static char dummy[128];
+ static char server[128];
int nbytes;
static char service[128];
static char address[128];
@@ -236,7 +253,7 @@ read_service_info(char *info_file_name, char **servicename, char **servername,
"service: %127s %15s version %15s\n"
"address: %127s\n"
"protocol: %15s\n",
- dummy,
+ server,
service, program, version,
address,
protoname);
@@ -258,7 +275,7 @@ read_service_info(char *info_file_name, char **servicename, char **servername,
if (!addrstr_to_sockaddr(addr, address, port))
goto fail;
- *servername = sockaddr_to_hostname(addr, address);
+ *servername = get_servername(server, addr, address);
if (*servername == NULL)
goto fail;
--
1.7.1
On Tue, Apr 09, 2013 at 01:35:06PM -0400, Steve Dickson wrote:
>
>
> On 09/04/13 13:25, Simo Sorce wrote:
> > On Tue, 2013-04-09 at 13:15 -0400, Steve Dickson wrote:
> >>
> >> On 08/04/13 10:08, Simo Sorce wrote:
> >>> On Mon, 2013-04-08 at 09:39 -0400, Steve Dickson wrote:
> >>>>
> >>>> On 02/04/13 15:32, Simo Sorce wrote:
> >>>>> A NFS client should be able to work properly even if the DNS Reverse record
> >>>>> for the server is not set. There is no excuse to forcefully prevent that
> >>>>> from working when it can.
> >>>>>
> >>>>> This patch adds a new pair of options (-z/-Z) that allow to turn on/off
> >>>>> DNS reverse resolution for determining the server name to use with GSSAPI.
> >>>> Again, please tell me why we need the -Z flag when that is the default?
> >>>
> >>> The idea is to switch the default in the code at some point, so then -Z
> >>> will be needed to get back to the original behavior.
> >> I'm thinking that's what major version number changes are for... not flags...
> >>
> >>>
> >>> The idea is that by having both flags a distribution may choose to
> >>> decide now what behavior they want and use the relative flag. Then even
> >>> if we change the default their configuration will not "break".
> >> I'll do the work to remove the option and repost the patches..
> >
> > As you wish, I do not have hard preferences, should we take the bait and
> > also by default *not* do PTR lookups ?
> I was thinking no. Leaves the default as is and used the -z to avoid the
> lookup...
>
> I'm struggling with how big of a problem this really is, so why should be break
> existing environments? I'm no DNS expert but I thinking not have PTR is
> a DNS config issue... but again I'm no expert...
Argh, no, one away or another the default needs to be to not do the PTR
lookup.
The transition Simo's using was Jeff's suggestion. Let's just stick to
that if we don't have a good reason.
(But I don't have strong opinions about how to do it either. I'd
actually be OK with being harsh and just switching to the new behavior
without any option.)
--b.
On 02/04/13 13:58, Myklebust, Trond wrote:
>> -----Original Message-----
>> From: [email protected] [mailto:linux-nfs-
>> [email protected]] On Behalf Of Simo Sorce
>> Sent: Tuesday, April 02, 2013 1:49 PM
>> To: Linux NFS Mailing list
>> Subject: [PATCH 2/3] Avoid reverse resolution for server name
>>
>> A NFS client should be able to work properly even if the DNS Reverse record
>> for the server is not set. There is no excuse to forcefully prevent that from
>> working when it can.
>
> Note that rpc.statd has the same limitation.
>
>> This patch adds a new -N option that takes an 'on' or 'off' argument and
>> controls whether forced PTR resolution is avoided (on) or performed (off)
>
> '-N' already has a very different meaning on rpc.mountd and rpc.nfsd. It might therefore be better to choose a different name to avoid confusion.
> Also, why do a single option with an 'on/off' argument instead of just choosing 2 options ?
Do we really what two options? why not just have something like -Z that will restore today default?
steved.
>
> Trond
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
On Tue, 2013-04-02 at 17:58 +0000, Myklebust, Trond wrote:
> > -----Original Message-----
> > From: [email protected] [mailto:linux-nfs-
> > [email protected]] On Behalf Of Simo Sorce
> > Sent: Tuesday, April 02, 2013 1:49 PM
> > To: Linux NFS Mailing list
> > Subject: [PATCH 2/3] Avoid reverse resolution for server name
> >
> > A NFS client should be able to work properly even if the DNS Reverse record
> > for the server is not set. There is no excuse to forcefully prevent that from
> > working when it can.
>
> Note that rpc.statd has the same limitation.
>
> > This patch adds a new -N option that takes an 'on' or 'off' argument and
> > controls whether forced PTR resolution is avoided (on) or performed (off)
>
> '-N' already has a very different meaning on rpc.mountd and rpc.nfsd. It might therefore be better to choose a different name to avoid confusion.
> Also, why do a single option with an 'on/off' argument instead of just choosing 2 options ?
>
Well Jeff seemed to suggest that way, I do not have any preference, can
you please give me 2 letters to use ?
Meanwhile I'll check rpc.statd as well.
Simo.
--
Simo Sorce * Red Hat, Inc * New York
On Tue, 9 Apr 2013 15:22:59 -0400
"J. Bruce Fields" <[email protected]> wrote:
> On Tue, Apr 09, 2013 at 03:12:56PM -0400, Steve Dickson wrote:
> >
> >
> > On 09/04/13 14:54, J. Bruce Fields wrote:
> > > Argh, no, one away or another the default needs to be to not do the PTR
> > > lookup.
> > Fine...
> >
> > >
> > > The transition Simo's using was Jeff's suggestion. Let's just stick to
> > > that if we don't have a good reason.
> > Yeah... I would like to avoid adding to flags... I don't think both are
> > needed.
>
> So, no flags.
>
> > > (But I don't have strong opinions about how to do it either. I'd
> > > actually be OK with being harsh and just switching to the new behavior
> > > without any option.)
> > My crutch is I'm not a big DNS guy so I'm not sure how much breakage
> > would occur... So I would rather be on the safe side and give people
> > a way to go back...
>
> So, yes to flags. I'm confused!
>
> I guess we can be moderately harsh: switch to the new default and
> provide only a flag to restore the old default for whoever wants it, but
> not a flag to specify the new default. Is that what you mean?
>
I think the above is the best course of action at this point. My
original thinking was "let's transition to the new behavior gracefully"
-- start with the default as-is, and then after suitably warning
everyone switch the default to the new behavior.
Now there's a CVE in play though, so I think our hands are tied here.
We have to change the default to the new behavior now without any sort
of graceful transition. That's likely to break in some environments at
least, so I think we need some mechanism to allow people to switch gssd
to the old behavior.
Note too that the problems are not likely to be "lack of a PTR record",
but rather that they have multiple A records pointing at the server. In
that situation, the ai_canonname field in the addrinfo struct may not
match what the PTR record points to, depending on which server name you
use.
--
Jeff Layton <[email protected]>
A NFS client should be able to work properly even if the DNS Reverse record
for the server is not set. There is no excuse to forcefully prevent that
from working when it can.
This patch adds a new -N option that takes an 'on' or 'off' argument and
controls whether forced PTR resolution is avoided (on) or performed (off)
To avoid breaking current behavior the option defaults to off by default,
ideally we will turn this on by default after a transition period.
Signed-off-by: Simo Sorce <[email protected]>
---
utils/gssd/gss_util.h | 2 ++
utils/gssd/gssd.c | 18 ++++++++++++++++--
utils/gssd/gssd_proc.c | 25 +++++++++++++++++++++----
3 files changed, 39 insertions(+), 6 deletions(-)
diff --git a/utils/gssd/gss_util.h b/utils/gssd/gss_util.h
index aa9f77806075f9ab67a7763a75a010369ba2d1b9..663fb0998bede6144118f890b9311ee8687176e3 100644
--- a/utils/gssd/gss_util.h
+++ b/utils/gssd/gss_util.h
@@ -52,4 +52,6 @@ int gssd_check_mechs(void);
gss_krb5_set_allowable_enctypes(min, cred, num, types)
#endif
+extern int avoid_ptr;
+
#endif /* _GSS_UTIL_H_ */
diff --git a/utils/gssd/gssd.c b/utils/gssd/gssd.c
index 07b1e52e6b84e9bcba96e7a63b0505ca7823482a..cc326dc2a729c1191c9f72ffd32203a58452b793 100644
--- a/utils/gssd/gssd.c
+++ b/utils/gssd/gssd.c
@@ -82,10 +82,19 @@ sig_hup(int signal)
return;
}
+static int get_bool_arg(const char *arg)
+{
+ if (strcmp(arg, "on") == 0)
+ return 1;
+ if (strcmp(arg, "off") == 0)
+ return 0;
+ return -1;
+}
+
static void
usage(char *progname)
{
- fprintf(stderr, "usage: %s [-f] [-l] [-M] [-n] [-v] [-r] [-p pipefsdir] [-k keytab] [-d ccachedir] [-t timeout] [-R preferred realm]\n",
+ fprintf(stderr, "usage: %s [-f] [-l] [-M] [-N on|off] [-n] [-v] [-r] [-p pipefsdir] [-k keytab] [-d ccachedir] [-t timeout] [-R preferred realm]\n",
progname);
exit(1);
}
@@ -102,7 +111,7 @@ main(int argc, char *argv[])
char *progname;
memset(ccachesearch, 0, sizeof(ccachesearch));
- while ((opt = getopt(argc, argv, "fvrlmnMp:k:d:t:R:")) != -1) {
+ while ((opt = getopt(argc, argv, "fvrlmnMp:k:d:t:R:N:")) != -1) {
switch (opt) {
case 'f':
fg = 1;
@@ -150,6 +159,11 @@ main(int argc, char *argv[])
errx(1, "Encryption type limits not supported by Kerberos libraries.");
#endif
break;
+ case 'N':
+ avoid_ptr = get_bool_arg(optarg);
+ if (avoid_ptr == -1)
+ errx(1, "Invalid argument for -N option");
+ break;
default:
usage(argv[0]);
break;
diff --git a/utils/gssd/gssd_proc.c b/utils/gssd/gssd_proc.c
index ea01e92e4565670b97dea1a936d2f0dbdc7c4610..21d4e1d78eb54d177626cb0a19b9de4e93e0a20d 100644
--- a/utils/gssd/gssd_proc.c
+++ b/utils/gssd/gssd_proc.c
@@ -67,6 +67,7 @@
#include <errno.h>
#include <gssapi/gssapi.h>
#include <netdb.h>
+#include <ctype.h>
#include "gssd.h"
#include "err_util.h"
@@ -107,6 +108,8 @@ struct pollfd * pollarray;
unsigned long pollsize; /* the size of pollaray (in pollfd's) */
+int avoid_ptr = 0;
+
/*
* convert a presentation address string to a sockaddr_storage struct. Returns
* true on success or false on failure.
@@ -165,12 +168,26 @@ addrstr_to_sockaddr(struct sockaddr *sa, const char *node, const char *port)
* convert a sockaddr to a hostname
*/
static char *
-sockaddr_to_hostname(const struct sockaddr *sa, const char *addr)
+get_servername(const char *name, const struct sockaddr *sa, const char *addr)
{
socklen_t addrlen;
int err;
char *hostname;
char hbuf[NI_MAXHOST];
+ unsigned char buf[sizeof(struct in6_addr)];
+ int do_ptr_lookup = 0;
+
+ if (avoid_ptr) {
+ /* try to determine if this is a name, or an IP address.
+ * If it is an IP fallback to a PTR lookup */
+ if (strchr(name, '.') && inet_pton(AF_INET, name, buf) == 1)
+ do_ptr_lookup = 1; /* IPv4 */
+ else if (strchr(name, ':') && inet_pton(AF_INET6, name, buf) == 1)
+ do_ptr_lookup = 1; /* or IPv6 */
+ if (!do_ptr_lookup) {
+ return strdup(name);
+ }
+ }
switch (sa->sa_family) {
case AF_INET:
@@ -208,7 +225,7 @@ read_service_info(char *info_file_name, char **servicename, char **servername,
struct sockaddr *addr) {
#define INFOBUFLEN 256
char buf[INFOBUFLEN + 1];
- static char dummy[128];
+ static char server[128];
int nbytes;
static char service[128];
static char address[128];
@@ -236,7 +253,7 @@ read_service_info(char *info_file_name, char **servicename, char **servername,
"service: %127s %15s version %15s\n"
"address: %127s\n"
"protocol: %15s\n",
- dummy,
+ server,
service, program, version,
address,
protoname);
@@ -258,7 +275,7 @@ read_service_info(char *info_file_name, char **servicename, char **servername,
if (!addrstr_to_sockaddr(addr, address, port))
goto fail;
- *servername = sockaddr_to_hostname(addr, address);
+ *servername = get_servername(server, addr, address);
if (*servername == NULL)
goto fail;
--
1.7.1
On 09/04/13 14:54, J. Bruce Fields wrote:
> On Tue, Apr 09, 2013 at 01:35:06PM -0400, Steve Dickson wrote:
>>
>>
>> On 09/04/13 13:25, Simo Sorce wrote:
>>> On Tue, 2013-04-09 at 13:15 -0400, Steve Dickson wrote:
>>>>
>>>> On 08/04/13 10:08, Simo Sorce wrote:
>>>>> On Mon, 2013-04-08 at 09:39 -0400, Steve Dickson wrote:
>>>>>>
>>>>>> On 02/04/13 15:32, Simo Sorce wrote:
>>>>>>> A NFS client should be able to work properly even if the DNS Reverse record
>>>>>>> for the server is not set. There is no excuse to forcefully prevent that
>>>>>>> from working when it can.
>>>>>>>
>>>>>>> This patch adds a new pair of options (-z/-Z) that allow to turn on/off
>>>>>>> DNS reverse resolution for determining the server name to use with GSSAPI.
>>>>>> Again, please tell me why we need the -Z flag when that is the default?
>>>>>
>>>>> The idea is to switch the default in the code at some point, so then -Z
>>>>> will be needed to get back to the original behavior.
>>>> I'm thinking that's what major version number changes are for... not flags...
>>>>
>>>>>
>>>>> The idea is that by having both flags a distribution may choose to
>>>>> decide now what behavior they want and use the relative flag. Then even
>>>>> if we change the default their configuration will not "break".
>>>> I'll do the work to remove the option and repost the patches..
>>>
>>> As you wish, I do not have hard preferences, should we take the bait and
>>> also by default *not* do PTR lookups ?
>> I was thinking no. Leaves the default as is and used the -z to avoid the
>> lookup...
>>
>> I'm struggling with how big of a problem this really is, so why should be break
>> existing environments? I'm no DNS expert but I thinking not have PTR is
>> a DNS config issue... but again I'm no expert...
>
> Argh, no, one away or another the default needs to be to not do the PTR
> lookup.
Fine...
>
> The transition Simo's using was Jeff's suggestion. Let's just stick to
> that if we don't have a good reason.
Yeah... I would like to avoid adding to flags... I don't think both are
needed.
>
> (But I don't have strong opinions about how to do it either. I'd
> actually be OK with being harsh and just switching to the new behavior
> without any option.)
My crutch is I'm not a big DNS guy so I'm not sure how much breakage
would occur... So I would rather be on the safe side and give people
a way to go back...
steved.
This one does the same thing as the previous but uses -z/-Z options.
Pick the one you like most, but this one is less code, so I guess it is
porefereable.
Simo Sorce (2):
Avoid reverse resolution for server name
Document new -z/-Z options
utils/gssd/gss_util.h | 2 ++
utils/gssd/gssd.c | 10 ++++++++--
utils/gssd/gssd.man | 13 ++++++++++++-
utils/gssd/gssd_proc.c | 25 +++++++++++++++++++++----
4 files changed, 43 insertions(+), 7 deletions(-)
On Wed, 2013-04-03 at 10:20 -0400, J. Bruce Fields wrote:
> On Tue, Apr 02, 2013 at 03:32:29PM -0400, Simo Sorce wrote:
> > Options are not in alphabetical order but -z/-Z clearly always come last.
> >
> > Signed-off-by: Simo Sorce <[email protected]>
> > ---
> > utils/gssd/gssd.man | 13 ++++++++++++-
> > 1 files changed, 12 insertions(+), 1 deletions(-)
> >
> > diff --git a/utils/gssd/gssd.man b/utils/gssd/gssd.man
> > index 79d9bf91ac6b976c57d167e60d07f828a3ff5b1f..7918c2a0ff76c3918449cf3e1420f0a289929ac1 100644
> > --- a/utils/gssd/gssd.man
> > +++ b/utils/gssd/gssd.man
> > @@ -8,7 +8,7 @@
> > rpc.gssd \- RPCSEC_GSS daemon
> > .SH SYNOPSIS
> > .B rpc.gssd
> > -.RB [ \-fMnlvr ]
> > +.RB [ \-fMnlvrzZ ]
> > .RB [ \-k
> > .IR keytab ]
> > .RB [ \-p
> > @@ -266,6 +266,17 @@ new kernel contexts to be negotiated after
> > seconds, which allows changing Kerberos tickets and identities frequently.
> > The default is no explicit timeout, which means the kernel context will live
> > the lifetime of the Kerberos service ticket used in its creation.
> > +.TP
> > +.B -z
> > +This option tries to avoid DNS Reverse (PTR) lookups for determining the
> > +server name to pass to GSSAPI if the name passed at mount point is not an IP
> > +address. Currently off by default for compatibility reasons.
> > +.TP
> > +.B -Z
> > +This is the inverse of
> > +.B -z
> > +and forces the use of DNS Reverse resolution of the server's IP address to
> > +retrieve the server name to use in GSAPI authentication.
>
> By the way I think with the "new" upcall, gssd ignores whatever it got
> out of the info file if the "target=" parameter is provided in the
> upcall.
Correct.
> (But looking at the code I think that's only used in the nfsv4.0
> callback case, and isn't worth mentioning here.)
Wrong. It is also used for NFSv4 and NFSv4.1 state management.
IOW: SETCLIENTID/RENEW and for NFSv4.1 EXCHANGE_ID/SEQUENCE; anything
that uses clp->cl_machine_cred.
--
Trond Myklebust
Linux NFS client maintainer
NetApp
[email protected]
http://www.netapp.com
On Wed, 2013-04-03 at 11:10 -0400, Trond Myklebust wrote:
> On Wed, 2013-04-03 at 10:56 -0400, J. Bruce Fields wrote:
> > On Wed, Apr 03, 2013 at 02:35:48PM +0000, Myklebust, Trond wrote:
> > > On Wed, 2013-04-03 at 10:20 -0400, J. Bruce Fields wrote:
> > > > On Tue, Apr 02, 2013 at 03:32:29PM -0400, Simo Sorce wrote:
> > > > > Options are not in alphabetical order but -z/-Z clearly always come last.
> > > > >
> > > > > Signed-off-by: Simo Sorce <[email protected]>
> > > > > ---
> > > > > utils/gssd/gssd.man | 13 ++++++++++++-
> > > > > 1 files changed, 12 insertions(+), 1 deletions(-)
> > > > >
> > > > > diff --git a/utils/gssd/gssd.man b/utils/gssd/gssd.man
> > > > > index 79d9bf91ac6b976c57d167e60d07f828a3ff5b1f..7918c2a0ff76c3918449cf3e1420f0a289929ac1 100644
> > > > > --- a/utils/gssd/gssd.man
> > > > > +++ b/utils/gssd/gssd.man
> > > > > @@ -8,7 +8,7 @@
> > > > > rpc.gssd \- RPCSEC_GSS daemon
> > > > > .SH SYNOPSIS
> > > > > .B rpc.gssd
> > > > > -.RB [ \-fMnlvr ]
> > > > > +.RB [ \-fMnlvrzZ ]
> > > > > .RB [ \-k
> > > > > .IR keytab ]
> > > > > .RB [ \-p
> > > > > @@ -266,6 +266,17 @@ new kernel contexts to be negotiated after
> > > > > seconds, which allows changing Kerberos tickets and identities frequently.
> > > > > The default is no explicit timeout, which means the kernel context will live
> > > > > the lifetime of the Kerberos service ticket used in its creation.
> > > > > +.TP
> > > > > +.B -z
> > > > > +This option tries to avoid DNS Reverse (PTR) lookups for determining the
> > > > > +server name to pass to GSSAPI if the name passed at mount point is not an IP
> > > > > +address. Currently off by default for compatibility reasons.
> > > > > +.TP
> > > > > +.B -Z
> > > > > +This is the inverse of
> > > > > +.B -z
> > > > > +and forces the use of DNS Reverse resolution of the server's IP address to
> > > > > +retrieve the server name to use in GSAPI authentication.
> > > >
> > > > By the way I think with the "new" upcall, gssd ignores whatever it got
> > > > out of the info file if the "target=" parameter is provided in the
> > > > upcall.
> > >
> > > Correct.
> > >
> > > > (But looking at the code I think that's only used in the nfsv4.0
> > > > callback case, and isn't worth mentioning here.)
> > >
> > > Wrong. It is also used for NFSv4 and NFSv4.1 state management.
> > > IOW: SETCLIENTID/RENEW and for NFSv4.1 EXCHANGE_ID/SEQUENCE; anything
> > > that uses clp->cl_machine_cred.
> >
> > I was talk about "target=", but I believe you're talking about
> > "service=".
> >
> > The former is a server name (myserver.example.com), the latter a service
> > name (nfs).
>
> Right, but gssd_refresh_krb5_machine_credential combines both in order
> to create the keytab entry.
>
Never mind. I see what you mean: we only set the
rpc_client->cl_principal for the case of NFSv4 callbacks to the client.
--
Trond Myklebust
Linux NFS client maintainer
NetApp
[email protected]
http://www.netapp.com
On 02/04/13 14:21, Simo Sorce wrote:
> On Tue, 2013-04-02 at 17:58 +0000, Myklebust, Trond wrote:
>>> -----Original Message-----
>>> From: [email protected] [mailto:linux-nfs-
>>> [email protected]] On Behalf Of Simo Sorce
>>> Sent: Tuesday, April 02, 2013 1:49 PM
>>> To: Linux NFS Mailing list
>>> Subject: [PATCH 2/3] Avoid reverse resolution for server name
>>>
>>> A NFS client should be able to work properly even if the DNS Reverse record
>>> for the server is not set. There is no excuse to forcefully prevent that from
>>> working when it can.
>>
>> Note that rpc.statd has the same limitation.
>
> I checked rpc.statd with a quick "git grep getnameinfo", and as far as I
> can see PTR lookups are done only if the 'name' is actually an IP
> address, but skipped if not. So I think rpc.statd is ok.
>
> I see rpc.statd uses getaddrinfo with hints set to AI_NUMERICHOST, if
> that's preferred over using inet_pton I can change my patches to do the
> same.
Please do.. let try to keep it consistent.... thanks!
steved.
Options are not in alphabetical order but I put the new one after the -M,
let's see this slow bubble sort algorithm to go on in the next decade :)
Signed-off-by: Simo Sorce <[email protected]>
---
utils/gssd/gssd.man | 11 ++++++++++-
1 files changed, 10 insertions(+), 1 deletions(-)
diff --git a/utils/gssd/gssd.man b/utils/gssd/gssd.man
index 79d9bf91ac6b976c57d167e60d07f828a3ff5b1f..d1851eb0c39d8fda3da6020b35566d391fe2daba 100644
--- a/utils/gssd/gssd.man
+++ b/utils/gssd/gssd.man
@@ -8,7 +8,7 @@
rpc.gssd \- RPCSEC_GSS daemon
.SH SYNOPSIS
.B rpc.gssd
-.RB [ \-fMnlvr ]
+.RB [ \-fMnlvrN ]
.RB [ \-k
.IR keytab ]
.RB [ \-p
@@ -245,6 +245,15 @@ is set,
.B rpc.gssd
stores machine credentials in memory instead.
.TP
+.B -N [on|off]
+When -N is set to
+.BR on
+the program tries to avoid DNS Reverse (PTR) lookups for resolving the server
+name if the name passed at mount point is not an IP address.
+Currently defaults to
+.BR off
+for compatibility reasons.
+.TP
.B -v
Increases the verbosity of the output (can be specified multiple times).
.TP
--
1.7.1
On Tue, 2013-04-02 at 14:25 -0400, Steve Dickson wrote:
>
> On 02/04/13 14:21, Simo Sorce wrote:
> > On Tue, 2013-04-02 at 17:58 +0000, Myklebust, Trond wrote:
> >>> -----Original Message-----
> >>> From: [email protected] [mailto:linux-nfs-
> >>> [email protected]] On Behalf Of Simo Sorce
> >>> Sent: Tuesday, April 02, 2013 1:49 PM
> >>> To: Linux NFS Mailing list
> >>> Subject: [PATCH 2/3] Avoid reverse resolution for server name
> >>>
> >>> A NFS client should be able to work properly even if the DNS Reverse record
> >>> for the server is not set. There is no excuse to forcefully prevent that from
> >>> working when it can.
> >>
> >> Note that rpc.statd has the same limitation.
> >
> > I checked rpc.statd with a quick "git grep getnameinfo", and as far as I
> > can see PTR lookups are done only if the 'name' is actually an IP
> > address, but skipped if not. So I think rpc.statd is ok.
> >
> > I see rpc.statd uses getaddrinfo with hints set to AI_NUMERICHOST, if
> > that's preferred over using inet_pton I can change my patches to do the
> > same.
> Please do.. let try to keep it consistent.... thanks!
Ok I looked at doing it, and looked at a helper function to reuse to
avoid duplication, however the functions in statd/hostname.c are static,
then I looked ad host_pton in support/export/hostname.c and realized 2
things:
- the comments there say inet_pton is actually better for what I need to
do, in fact inet_pton is used as a filter before calling getaddrinfo.
- the function can't be used in gssd_proc.c as the file is not linked
there.
So given the above I would prefer to not change the use of inet_pton in
the patches and leave them as is.
Simo.
--
Simo Sorce * Red Hat, Inc * New York
On 09/04/13 13:25, Simo Sorce wrote:
> On Tue, 2013-04-09 at 13:15 -0400, Steve Dickson wrote:
>>
>> On 08/04/13 10:08, Simo Sorce wrote:
>>> On Mon, 2013-04-08 at 09:39 -0400, Steve Dickson wrote:
>>>>
>>>> On 02/04/13 15:32, Simo Sorce wrote:
>>>>> A NFS client should be able to work properly even if the DNS Reverse record
>>>>> for the server is not set. There is no excuse to forcefully prevent that
>>>>> from working when it can.
>>>>>
>>>>> This patch adds a new pair of options (-z/-Z) that allow to turn on/off
>>>>> DNS reverse resolution for determining the server name to use with GSSAPI.
>>>> Again, please tell me why we need the -Z flag when that is the default?
>>>
>>> The idea is to switch the default in the code at some point, so then -Z
>>> will be needed to get back to the original behavior.
>> I'm thinking that's what major version number changes are for... not flags...
>>
>>>
>>> The idea is that by having both flags a distribution may choose to
>>> decide now what behavior they want and use the relative flag. Then even
>>> if we change the default their configuration will not "break".
>> I'll do the work to remove the option and repost the patches..
>
> As you wish, I do not have hard preferences, should we take the bait and
> also by default *not* do PTR lookups ?
I was thinking no. Leaves the default as is and used the -z to avoid the
lookup...
I'm struggling with how big of a problem this really is, so why should be break
existing environments? I'm no DNS expert but I thinking not have PTR is
a DNS config issue... but again I'm no expert...
steved.
On 02/04/13 15:32, Simo Sorce wrote:
> A NFS client should be able to work properly even if the DNS Reverse record
> for the server is not set. There is no excuse to forcefully prevent that
> from working when it can.
>
> This patch adds a new pair of options (-z/-Z) that allow to turn on/off
> DNS reverse resolution for determining the server name to use with GSSAPI.
Again, please tell me why we need the -Z flag when that is the default?
steved.
>
> To avoid breaking current behavior the option defaults to off by default,
> ideally we will turn this on by default after a transition period.
>
> Signed-off-by: Simo Sorce <[email protected]>
> ---
> utils/gssd/gss_util.h | 2 ++
> utils/gssd/gssd.c | 10 ++++++++--
> utils/gssd/gssd_proc.c | 25 +++++++++++++++++++++----
> 3 files changed, 31 insertions(+), 6 deletions(-)
>
> diff --git a/utils/gssd/gss_util.h b/utils/gssd/gss_util.h
> index aa9f77806075f9ab67a7763a75a010369ba2d1b9..663fb0998bede6144118f890b9311ee8687176e3 100644
> --- a/utils/gssd/gss_util.h
> +++ b/utils/gssd/gss_util.h
> @@ -52,4 +52,6 @@ int gssd_check_mechs(void);
> gss_krb5_set_allowable_enctypes(min, cred, num, types)
> #endif
>
> +extern int avoid_ptr;
> +
> #endif /* _GSS_UTIL_H_ */
> diff --git a/utils/gssd/gssd.c b/utils/gssd/gssd.c
> index 07b1e52e6b84e9bcba96e7a63b0505ca7823482a..1f0ac0c47667c42ed03e271cb18b6124165e5d5f 100644
> --- a/utils/gssd/gssd.c
> +++ b/utils/gssd/gssd.c
> @@ -85,7 +85,7 @@ sig_hup(int signal)
> static void
> usage(char *progname)
> {
> - fprintf(stderr, "usage: %s [-f] [-l] [-M] [-n] [-v] [-r] [-p pipefsdir] [-k keytab] [-d ccachedir] [-t timeout] [-R preferred realm]\n",
> + fprintf(stderr, "usage: %s [-f] [-l] [-M] [-n] [-v] [-r] [-p pipefsdir] [-k keytab] [-d ccachedir] [-t timeout] [-R preferred realm] [-z] [-Z]\n",
> progname);
> exit(1);
> }
> @@ -102,7 +102,7 @@ main(int argc, char *argv[])
> char *progname;
>
> memset(ccachesearch, 0, sizeof(ccachesearch));
> - while ((opt = getopt(argc, argv, "fvrlmnMp:k:d:t:R:")) != -1) {
> + while ((opt = getopt(argc, argv, "fvrlmnMp:k:d:t:R:zZ")) != -1) {
> switch (opt) {
> case 'f':
> fg = 1;
> @@ -150,6 +150,12 @@ main(int argc, char *argv[])
> errx(1, "Encryption type limits not supported by Kerberos libraries.");
> #endif
> break;
> + case 'z':
> + avoid_ptr = 1;
> + break;
> + case 'Z':
> + avoid_ptr = 0;
> + break;
> default:
> usage(argv[0]);
> break;
> diff --git a/utils/gssd/gssd_proc.c b/utils/gssd/gssd_proc.c
> index ea01e92e4565670b97dea1a936d2f0dbdc7c4610..21d4e1d78eb54d177626cb0a19b9de4e93e0a20d 100644
> --- a/utils/gssd/gssd_proc.c
> +++ b/utils/gssd/gssd_proc.c
> @@ -67,6 +67,7 @@
> #include <errno.h>
> #include <gssapi/gssapi.h>
> #include <netdb.h>
> +#include <ctype.h>
>
> #include "gssd.h"
> #include "err_util.h"
> @@ -107,6 +108,8 @@ struct pollfd * pollarray;
>
> unsigned long pollsize; /* the size of pollaray (in pollfd's) */
>
> +int avoid_ptr = 0;
> +
> /*
> * convert a presentation address string to a sockaddr_storage struct. Returns
> * true on success or false on failure.
> @@ -165,12 +168,26 @@ addrstr_to_sockaddr(struct sockaddr *sa, const char *node, const char *port)
> * convert a sockaddr to a hostname
> */
> static char *
> -sockaddr_to_hostname(const struct sockaddr *sa, const char *addr)
> +get_servername(const char *name, const struct sockaddr *sa, const char *addr)
> {
> socklen_t addrlen;
> int err;
> char *hostname;
> char hbuf[NI_MAXHOST];
> + unsigned char buf[sizeof(struct in6_addr)];
> + int do_ptr_lookup = 0;
> +
> + if (avoid_ptr) {
> + /* try to determine if this is a name, or an IP address.
> + * If it is an IP fallback to a PTR lookup */
> + if (strchr(name, '.') && inet_pton(AF_INET, name, buf) == 1)
> + do_ptr_lookup = 1; /* IPv4 */
> + else if (strchr(name, ':') && inet_pton(AF_INET6, name, buf) == 1)
> + do_ptr_lookup = 1; /* or IPv6 */
> + if (!do_ptr_lookup) {
> + return strdup(name);
> + }
> + }
>
> switch (sa->sa_family) {
> case AF_INET:
> @@ -208,7 +225,7 @@ read_service_info(char *info_file_name, char **servicename, char **servername,
> struct sockaddr *addr) {
> #define INFOBUFLEN 256
> char buf[INFOBUFLEN + 1];
> - static char dummy[128];
> + static char server[128];
> int nbytes;
> static char service[128];
> static char address[128];
> @@ -236,7 +253,7 @@ read_service_info(char *info_file_name, char **servicename, char **servername,
> "service: %127s %15s version %15s\n"
> "address: %127s\n"
> "protocol: %15s\n",
> - dummy,
> + server,
> service, program, version,
> address,
> protoname);
> @@ -258,7 +275,7 @@ read_service_info(char *info_file_name, char **servicename, char **servername,
> if (!addrstr_to_sockaddr(addr, address, port))
> goto fail;
>
> - *servername = sockaddr_to_hostname(addr, address);
> + *servername = get_servername(server, addr, address);
> if (*servername == NULL)
> goto fail;
>
>
On Tue, 2013-04-02 at 17:58 +0000, Myklebust, Trond wrote:
> > -----Original Message-----
> > From: [email protected] [mailto:linux-nfs-
> > [email protected]] On Behalf Of Simo Sorce
> > Sent: Tuesday, April 02, 2013 1:49 PM
> > To: Linux NFS Mailing list
> > Subject: [PATCH 2/3] Avoid reverse resolution for server name
> >
> > A NFS client should be able to work properly even if the DNS Reverse record
> > for the server is not set. There is no excuse to forcefully prevent that from
> > working when it can.
>
> Note that rpc.statd has the same limitation.
I checked rpc.statd with a quick "git grep getnameinfo", and as far as I
can see PTR lookups are done only if the 'name' is actually an IP
address, but skipped if not. So I think rpc.statd is ok.
I see rpc.statd uses getaddrinfo with hints set to AI_NUMERICHOST, if
that's preferred over using inet_pton I can change my patches to do the
same.
Simo.
--
Simo Sorce * Red Hat, Inc * New York