2013-11-09 22:46:09

by Steve Dickson

[permalink] [raw]
Subject: [PATCH] Adding the nfs4_secure_mounts bool

The nfs4_secure_mounts controls whether security
flavors will be tried during the establishing of
NFSv4 state and the pseudoroot lookups.

This allows security daemon like rpc.gssd to
tell the kernel that secure mounts should be tried.

To enable secure mounts:
echo "on" > /proc/fs/nfsfs/secure

To disable secure mounts:
echo "off" > /proc/fs/nfsfs/secure

Signed-off-by: Steve Dickson <[email protected]>
---
fs/nfs/client.c | 66 +++++++++++++++++++++++++++++++++++++++++++++++++++
fs/nfs/nfs4_fs.h | 1 +
fs/nfs/nfs4client.c | 7 ++++-
fs/nfs/nfs4proc.c | 4 ++-
fs/nfs/super.c | 2 +
5 files changed, 77 insertions(+), 3 deletions(-)

diff --git a/fs/nfs/client.c b/fs/nfs/client.c
index 1d09289..0ad38e4 100644
--- a/fs/nfs/client.c
+++ b/fs/nfs/client.c
@@ -1214,6 +1214,9 @@ static void *nfs_volume_list_start(struct seq_file *p, loff_t *pos);
static void *nfs_volume_list_next(struct seq_file *p, void *v, loff_t *pos);
static void nfs_volume_list_stop(struct seq_file *p, void *v);
static int nfs_volume_list_show(struct seq_file *m, void *v);
+static ssize_t nfs_secure_write(struct file *file, const char __user *buf, size_t size, loff_t *pos);
+static ssize_t nfs_secure_read(struct file *file, char __user *buf, size_t size, loff_t *pos);
+

static const struct seq_operations nfs_volume_list_ops = {
.start = nfs_volume_list_start,
@@ -1229,6 +1232,13 @@ static const struct file_operations nfs_volume_list_fops = {
.release = seq_release,
.owner = THIS_MODULE,
};
+static const struct file_operations nfs_secure_fops = {
+ .write = nfs_secure_write,
+ .read = nfs_secure_read,
+ .release = simple_transaction_release,
+ .llseek = default_llseek,
+ .owner = THIS_MODULE,
+};

/*
* open "/proc/fs/nfsfs/servers" which provides a summary of servers with which
@@ -1407,6 +1417,55 @@ static int nfs_volume_list_show(struct seq_file *m, void *v)
return 0;
}

+static ssize_t nfs_secure_data(struct file *file, char *buf, size_t size)
+{
+ int remaining = SIMPLE_TRANSACTION_LIMIT;
+
+ if (size > 0) {
+ if (buf[size-1] != '\n')
+ return -EINVAL;
+ buf[size-1] = 0;
+ if (strncmp(buf, "on", strlen("on")) == 0)
+ nfs4_secure_mounts = true;
+ else if (strncmp(buf, "on", strlen("on")) == 0)
+ nfs4_secure_mounts = false;
+ }
+ size = snprintf(buf, remaining, "nfs4_secure_mounts = %s\n",
+ nfs4_secure_mounts ? "on" : "off");
+
+ return size;
+}
+static ssize_t nfs_secure_write(struct file *file, const char __user *buf, size_t size, loff_t *pos)
+{
+ char *data;
+ ssize_t rv = 0;
+
+ data = simple_transaction_get(file, buf, size);
+ if (IS_ERR(data))
+ return PTR_ERR(data);
+
+ rv = nfs_secure_data(file, data, size);
+ if (rv >= 0) {
+ simple_transaction_set(file, rv);
+ rv = size;
+ }
+ return rv;
+}
+static ssize_t nfs_secure_read(struct file *file, char __user *buf, size_t size, loff_t *pos)
+{
+
+ if (! file->private_data) {
+ /* An attempt to read a transaction file without writing
+ * causes a 0-byte write so that the file can return
+ * state information
+ */
+ ssize_t rv = nfs_secure_write(file, buf, 0, pos);
+ if (rv < 0)
+ return rv;
+ }
+ return simple_transaction_read(file, buf, size, pos);
+}
+
/*
* initialise the /proc/fs/nfsfs/ directory
*/
@@ -1429,8 +1488,14 @@ int __init nfs_fs_proc_init(void)
proc_fs_nfs, &nfs_volume_list_fops);
if (!p)
goto error_2;
+ p = proc_create("secure", S_IFREG|S_IRUSR|S_IWUSR,
+ proc_fs_nfs, &nfs_secure_fops);
+ if (!p)
+ goto error_3;
return 0;

+error_3:
+ remove_proc_entry("secure", proc_fs_nfs);
error_2:
remove_proc_entry("servers", proc_fs_nfs);
error_1:
@@ -1444,6 +1509,7 @@ error_0:
*/
void nfs_fs_proc_exit(void)
{
+ remove_proc_entry("secure", proc_fs_nfs);
remove_proc_entry("volumes", proc_fs_nfs);
remove_proc_entry("servers", proc_fs_nfs);
remove_proc_entry("fs/nfsfs", NULL);
diff --git a/fs/nfs/nfs4_fs.h b/fs/nfs/nfs4_fs.h
index 3ce79b0..cb4faa2 100644
--- a/fs/nfs/nfs4_fs.h
+++ b/fs/nfs/nfs4_fs.h
@@ -453,6 +453,7 @@ extern bool nfs4_disable_idmapping;
extern unsigned short max_session_slots;
extern unsigned short send_implementation_id;
extern bool recover_lost_locks;
+extern bool nfs4_secure_mounts;

#define NFS4_CLIENT_ID_UNIQ_LEN (64)
extern char nfs4_client_id_uniquifier[NFS4_CLIENT_ID_UNIQ_LEN];
diff --git a/fs/nfs/nfs4client.c b/fs/nfs/nfs4client.c
index b4a160a..d92cf9d 100644
--- a/fs/nfs/nfs4client.c
+++ b/fs/nfs/nfs4client.c
@@ -356,6 +356,7 @@ struct nfs_client *nfs4_init_client(struct nfs_client *clp,
char buf[INET6_ADDRSTRLEN + 1];
struct nfs_client *old;
int error;
+ rpc_authflavor_t flavor = RPC_AUTH_UNIX;

if (clp->cl_cons_state == NFS_CS_READY) {
/* the client is initialised already */
@@ -370,8 +371,10 @@ struct nfs_client *nfs4_init_client(struct nfs_client *clp,
__set_bit(NFS_CS_INFINITE_SLOTS, &clp->cl_flags);
__set_bit(NFS_CS_DISCRTRY, &clp->cl_flags);
__set_bit(NFS_CS_NO_RETRANS_TIMEOUT, &clp->cl_flags);
- error = nfs_create_rpc_client(clp, timeparms, RPC_AUTH_GSS_KRB5I);
- if (error == -EINVAL)
+ if (nfs4_secure_mounts)
+ flavor = RPC_AUTH_GSS_KRB5I;
+ error = nfs_create_rpc_client(clp, timeparms, flavor);
+ if (error == -EINVAL && flavor != RPC_AUTH_UNIX)
error = nfs_create_rpc_client(clp, timeparms, RPC_AUTH_UNIX);
if (error < 0)
goto error;
diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index 5ab33c0..8d292e7 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -2896,7 +2896,9 @@ static int nfs4_find_root_sec(struct nfs_server *server, struct nfs_fh *fhandle,
}
} else {
/* no flavors specified by user, try default list */
- for (i = 0; i < ARRAY_SIZE(flav_array); i++) {
+ if (nfs4_secure_mounts == 0)
+ status = nfs4_lookup_root_sec(server, fhandle, info, RPC_AUTH_UNIX);
+ else for (i = 0; i < ARRAY_SIZE(flav_array); i++) {
status = nfs4_lookup_root_sec(server, fhandle, info,
flav_array[i]);
if (status == -NFS4ERR_WRONGSEC || status == -EACCES)
diff --git a/fs/nfs/super.c b/fs/nfs/super.c
index 317d6fc..1e845c3 100644
--- a/fs/nfs/super.c
+++ b/fs/nfs/super.c
@@ -2841,6 +2841,7 @@ unsigned short max_session_slots = NFS4_DEF_SLOT_TABLE_SIZE;
unsigned short send_implementation_id = 1;
char nfs4_client_id_uniquifier[NFS4_CLIENT_ID_UNIQ_LEN] = "";
bool recover_lost_locks = false;
+bool nfs4_secure_mounts = false;

EXPORT_SYMBOL_GPL(nfs_callback_set_tcpport);
EXPORT_SYMBOL_GPL(nfs_callback_tcpport);
@@ -2850,6 +2851,7 @@ EXPORT_SYMBOL_GPL(max_session_slots);
EXPORT_SYMBOL_GPL(send_implementation_id);
EXPORT_SYMBOL_GPL(nfs4_client_id_uniquifier);
EXPORT_SYMBOL_GPL(recover_lost_locks);
+EXPORT_SYMBOL_GPL(nfs4_secure_mounts);

#define NFS_CALLBACK_MAXPORTNR (65535U)

--
1.7.1



2013-11-11 19:04:08

by Steve Dickson

[permalink] [raw]
Subject: Re: [PATCH] Adding the nfs4_secure_mounts bool



On 11/11/13 13:53, Myklebust, Trond wrote:
>
> On Nov 11, 2013, at 13:43, Steve Dickson <[email protected]> wrote:
>
>>
>>
>> On 11/11/13 13:25, Myklebust, Trond wrote:
>>>
>>> On Nov 11, 2013, at 13:06, Steve Dickson <[email protected]> wrote:
>>>
>>>>
>>>>
>>>> On 09/11/13 18:12, Myklebust, Trond wrote:
>>>>> One alternative to the above scheme, which I believe that I?ve
>>>>> suggested before, is to have a permanent entry in rpc_pipefs
>>>>> that rpc.gssd can open and that the kernel can use to detect
>>>>> that it is running. If we make it /var/lib/nfs/rpc_pipefs/gssd/clnt00/gssd,
>>>>> then AFAICS we don?t need to change nfs-utils at all, since all newer
>>>>> versions of rpc.gssd will try to open for read anything of the form
>>>>> /var/lib/nfs/rpc_pipefs/*/clntXX/gssd...
>>>>
>>>> After further review I am going going have to disagree with you on this.
>>>> Since all the context is cached on the initial mount the kernel
>>>
>>> What context?
>> The krb5 blob that the kernel is call up to rpc.gssd to get.. Maybe
>> I'm using the wrong terminology???
>
> That?s only the machine cred. User credentials get allocated and freed all the time.
>
> When the server reboots, then all GSS contexts need to be re-established,
> which can be a lot of call_usermodehelper() upcalls; that?s one of the
> reasons why we decided in favour of a gssd daemon in the first place.
Just curious... Why is the call_usermodehelper() upcalls more expensive
than the rpc_pipefs upcalls?

steved.


2013-11-11 21:47:12

by Chuck Lever

[permalink] [raw]
Subject: Re: [PATCH] Adding the nfs4_secure_mounts bool


On Nov 11, 2013, at 4:13 PM, Steve Dickson <[email protected]> wrote:

>
>
> On 11/11/13 15:33, Chuck Lever wrote:
>>>>
>>>> We have workarounds already that work on every kernel since 3.8.
>>>>
>>> The one that logs 5 to 20 lines (depending on thins are setup or not)
>>> per mount? That does work in some environments but no all. ;-)
>>
>> When does running rpc.gssd not work?
> Define "work"... ;-) Logging 20 error messages on every mount
> when the keytab does not exist is not working or not very well.... IMHO..

At first it was 5 warnings. Then it was "10-15 errors". Now it's 20 on every mount!

How many "-v" options do you specify on your rpc.gssd command line? I normally run rpc.gssd with "-vv" or even "-vvv". Leaving those options off makes it much quieter, and it's the default. Only crazy people crank up the verbosity of rpc.gssd.

Is a mount failing in that case? Too many error messages is a nit compared to "no longer mounts." If gssd error messages are your only beef, then it seems like the solution is pretty obvious.

>>
>>> Or am I missing one? Please tell me I am!!! :-)
>>
>> OK: You're missing one.
> Thank you...
>
>>
>> Client configurations that have auth_rpcgss.ko loaded but do not run rpc.gssd are, quite simply, broken. The gss upcall will not work in that configuration. There is no reason to load auth_rpcgss.ko if user space does not have rpc.gssd enabled and running.
>>
>> This has been a problem for a very long time, but use cases where the 15 second upcall timeout is encountered have until recently been infrequent, and only occur in situations where the mount options don't work anyway, so no one has bothered to address it.
>>
>> This broken configuration is due to incorrect system initialization scripts.
> Which initialization scripts are you referring to?

The NFS initialization scripts. What else deals with auth_rpcgss.ko and rpc.gssd?

>
>>
>> If an administrator chooses not to run rpc.gssd, then auth_rpcgss.ko should not be loaded. The kernel deals quite correctly when auth_rpcgss.ko is not loaded -- GSS flavors are simply not supported, and the kernel uses AUTH_SYS by default for everything. (Right there is your magical administrative interface for controlling what security flavors are used and supported).
>>
>> But the current init scripts load auth_rpcgss.ko unconditionally. Thus any gss upcall will fail until rpc.gssd is started.
>>
>> With upstream kernel commit 4edaa308 (and following) I added a use case that exposes the upcall timeout much more often. It's a latent bug, but we now hit it during the first mount after every client reboot, so it is noticeable to anyone who leaves nfs-secure.service disabled.
>>
>> Therefore the scenario we want to avoid is where auth_rpcgss.ko is loaded, but rpc.gssd is not running. There are two obvious ways to avoid this scenario:
>>
>> A. If auth_rpcgss.ko is loaded unconditionally, make sure rpc.gssd is running unconditionally (and cull the warnings)
>>
>> B. If you do not want to run rpc.gssd, make sure auth_rpcgss.ko is not loaded
> Isn't the case that nfsd will also loads auth_rpcgss. So if server was started we
> would be right back to the 15 sec delay?

Only if NFSD wants to make GSS upcalls and rpc.gssd isn't running. Is that ever the case?

The point is we currently have a very direct method of telling the kernel NFS components not to use GSS or make upcalls. We should use it.


>
> steved.
>
>>
>> Both of these workarounds are configuration changes, not code changes. B. is accomplished simply by blacklisting auth_rpcgss.ko.
>>
>> We could go a step further and remove the module alias added by upstream kernel commit 71afa85e to prevent the kernel from loading auth_rpcgss.ko automatically, and then make sure the nfs-secure.service script (and the server-side equivalent) loads auth_rpcgss.ko before starting rpc.gssd.
>>
>> Since you are opposed to running rpc.gssd, blacklisting auth_rpcgss.ko is the easy choice. Adding auth_rpcgss.ko to the module blacklist is no more difficult than setting a kernel module parameter or toggling nfs-secure.service.
>>
>> However, IMO, in the long run Linux should be installed with rpc.gssd running unconditionally, or dynamically start-able by mount.nfs (like statd works today). We cannot guess a priori whether a user or administrator will want NFS support for GSS flavors, just as we cannot guess a priori whether the user wants support for NFSv3.
>>
>> Yet we always have NFSv3 support available. Likewise, support for GSS should always be available, and there really is no good reason any more not to leave it running all the time.
>>
>> As Trond points out, NFSv4 implementations are required to implement GSS security.
>>
> --
> 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

--
Chuck Lever
chuck[dot]lever[at]oracle[dot]com





2013-11-12 16:16:38

by J. Bruce Fields

[permalink] [raw]
Subject: Re: [PATCH] Adding the nfs4_secure_mounts bool

On Tue, Nov 12, 2013 at 05:29:46AM +0000, Myklebust, Trond wrote:
>
> On Nov 12, 2013, at 0:11, NeilBrown <[email protected]> wrote:
>
> > On Mon, 11 Nov 2013 15:33:14 -0500 Chuck Lever <[email protected]> wrote:
> >
> >>
> >> On Nov 11, 2013, at 1:59 PM, Steve Dickson <[email protected]> wrote:
> >>
> >>> On 11/11/13 13:30, Chuck Lever wrote:
> >>>>
> >>>> On Nov 11, 2013, at 1:06 PM, Steve Dickson <[email protected]> wrote:
> >>>>
> >>>>>
> >>>>>
> >>>>> On 09/11/13 18:12, Myklebust, Trond wrote:
> >>>>>> One alternative to the above scheme, which I believe that I’ve
> >>>>>> suggested before, is to have a permanent entry in rpc_pipefs
> >>>>>> that rpc.gssd can open and that the kernel can use to detect
> >>>>>> that it is running. If we make it /var/lib/nfs/rpc_pipefs/gssd/clnt00/gssd,
> >>>>>> then AFAICS we don’t need to change nfs-utils at all, since all newer
> >>>>>> versions of rpc.gssd will try to open for read anything of the form
> >>>>>> /var/lib/nfs/rpc_pipefs/*/clntXX/gssd...
> >>>>>
> >>>>> After further review I am going going have to disagree with you on this.
> >>>>> Since all the context is cached on the initial mount the kernel
> >>>>> should be using the call_usermodehelper() to call up to rpc.gssd
> >>>>> to get the context, which means we could put this upcall noise
> >>>>> to bed... forever! :-)
> >>>>
> >>>> Ask Al Viro for his comments on whether the kernel should start
> >>>> gssd (either a daemon or a script). Hint: wear your kevlar underpants.
> >>> I was thinking gssd would become a the gssd-cmd command... Al does not
> >>> like the call_usermodehelper() interface?
> >>
> >> He doesn't have a problem with call_usermodehelper() in general. However, the kernel cannot guarantee security if it has to run a fixed command line. Go ask him to explain.
> >>
> >>
> >>>
> >>>>
> >>>> Have you tried Trond's approach yet?
> >>> Looking into it... But nothing is trivial in that code...
> >>>
> >>>>
> >>>>> I realize this is not going happen overnight, so I would still
> >>>>> like to propose my nfs4_secure_mounts bool patch as bridge
> >>>>> to the new call_usermodehelper() since its the cleanest
> >>>>> solution so far...
> >>>>>
> >>>>> Thoughts?
> >>>>
> >>>> We have workarounds already that work on every kernel since 3.8.
> >>>>
> >>> The one that logs 5 to 20 lines (depending on thins are setup or not)
> >>> per mount? That does work in some environments but no all. ;-)
> >>
> >> When does running rpc.gssd not work?
> >
> > Oohh ooh.. Pick me. Pick me!! I can answer that one.
> >
> > Running rpc.gssd does not work if you are mounting a filesystem using the IP
> > address of the server and that IP address doesn't have a matching hostname
> > anywhere that can be found:
> >
> > In a newly creating minimal kvm install without rpc.gssd running,
> > mount 10.0.2.2:/home /mnt
> >
> > sleeps for 15 seconds then succeeds.
> > If I start rpc.gssd, then the same command takes forever.
> >
> > strace of rpc.gssd shows that it complains about not being able to resolve
> > the host name and "ERROR: failed to read service info". Then it keeps the
> > pipes open but never sends any message on them, so the kernel just keeps on
> > waiting.
> >
> > If I change "fail_keep_client" to "fail_destroy_client", then it closes the
> > pipe and we get the 15 second timeout back.
> > If I change NI_NAMEREQD to 0, then the mount completes instantly. (of course
> > that make serious compromise security so it was just for testing).
> > (Adding an entry to /etc/hosts also gives instant success).
> >
> > I'm hoping that someone who understands this code will suggest something
> > clever so I don't have to dig through all of it ;-)
>
> rpc.gssd is supposed to do a downcall with a zero-length window and an error message in any situation where it cannot establish a GSS context. Normally, I’d expect an EACCES for the above scenario.
>
> IOW: that’s a blatant rpc.gssd bug. One that will also affect you when you're doing NFSv3 and add ‘sec=krb5’ to the mount options.

Also why is gssd trying to do a DNS lookup in this case? This sounds
similar to what f9f5450f8f94 "Avoid DNS reverse resolution for server
names (take 3)" was trying to fix?

--b.

2013-11-11 19:21:32

by Myklebust, Trond

[permalink] [raw]
Subject: Re: [PATCH] Adding the nfs4_secure_mounts bool


On Nov 11, 2013, at 14:05, Steve Dickson <[email protected]> wrote:

>
>
> On 11/11/13 13:53, Myklebust, Trond wrote:
>>
>> On Nov 11, 2013, at 13:43, Steve Dickson <[email protected]> wrote:
>>
>>>
>>>
>>> On 11/11/13 13:25, Myklebust, Trond wrote:
>>>>
>>>> On Nov 11, 2013, at 13:06, Steve Dickson <[email protected]> wrote:
>>>>
>>>>>
>>>>>
>>>>> On 09/11/13 18:12, Myklebust, Trond wrote:
>>>>>> One alternative to the above scheme, which I believe that I?ve
>>>>>> suggested before, is to have a permanent entry in rpc_pipefs
>>>>>> that rpc.gssd can open and that the kernel can use to detect
>>>>>> that it is running. If we make it /var/lib/nfs/rpc_pipefs/gssd/clnt00/gssd,
>>>>>> then AFAICS we don?t need to change nfs-utils at all, since all newer
>>>>>> versions of rpc.gssd will try to open for read anything of the form
>>>>>> /var/lib/nfs/rpc_pipefs/*/clntXX/gssd...
>>>>>
>>>>> After further review I am going going have to disagree with you on this.
>>>>> Since all the context is cached on the initial mount the kernel
>>>>
>>>> What context?
>>> The krb5 blob that the kernel is call up to rpc.gssd to get.. Maybe
>>> I'm using the wrong terminology???
>>
>> That?s only the machine cred. User credentials get allocated and freed all the time.
>>
>> When the server reboots, then all GSS contexts need to be re-established,
>> which can be a lot of call_usermodehelper() upcalls; that?s one of the
>> reasons why we decided in favour of a gssd daemon in the first place.
> Just curious... Why is the call_usermodehelper() upcalls more expensive
> than the rpc_pipefs upcalls?

Each upcall requires you to allocate a complete new process context and run another instance of the gssd executable. If you have enough users, then a reboot situation can quickly escalate into chewing up significantly greater amounts of memory than the single daemon does.


2013-11-11 18:05:52

by Steve Dickson

[permalink] [raw]
Subject: Re: [PATCH] Adding the nfs4_secure_mounts bool



On 09/11/13 18:12, Myklebust, Trond wrote:
> One alternative to the above scheme, which I believe that I?ve
> suggested before, is to have a permanent entry in rpc_pipefs
> that rpc.gssd can open and that the kernel can use to detect
> that it is running. If we make it /var/lib/nfs/rpc_pipefs/gssd/clnt00/gssd,
> then AFAICS we don?t need to change nfs-utils at all, since all newer
> versions of rpc.gssd will try to open for read anything of the form
> /var/lib/nfs/rpc_pipefs/*/clntXX/gssd...

After further review I am going going have to disagree with you on this.
Since all the context is cached on the initial mount the kernel
should be using the call_usermodehelper() to call up to rpc.gssd
to get the context, which means we could put this upcall noise
to bed... forever! :-)

I realize this is not going happen overnight, so I would still
like to propose my nfs4_secure_mounts bool patch as bridge
to the new call_usermodehelper() since its the cleanest
solution so far...

Thoughts?

steved.

2013-11-12 16:47:02

by Chuck Lever

[permalink] [raw]
Subject: Re: [PATCH] Adding the nfs4_secure_mounts bool


On Nov 12, 2013, at 11:24 AM, Steve Dickson <[email protected]> wrote:

>
>
> On 12/11/13 11:09, Chuck Lever wrote:
>>> In the past, if admins want rpc.gssd in the mount path they had to configure it.
>>>> Now we are silently adding, yet another, daemon to the mount path and if
>>>> rpc.gssd starts falling on its face, I think it will be difficult to debug,
>>>> since the daemon is not expected to be there...
>> Our only real choice here is to fix gssd. Anything else is punting the problem down the road.
>>
> No. The last there was a daemon was involved in all NFS client mounts
> (at least that I can remember) was when lockd was a user level daemon.
> The main reason it was ported to the kernel was to get ride of the
> bottle neck it caused... Now we adding similar bottle neck back??
>
> Architecturally, put a daemon in the direct NFS mount path just does
> not make sense... IMHO...

Don't be ridiculous. rpc.gssd is ALREADY in the direct mount path for all Kerberos mounts, and has been for years.

Forget lease management security for a moment, and consider this: There is no possibility of moving forward with a secure NFS solution on Linux if we can't depend on rpc.gssd. Therefore, our only real choice if we want Kerberos to be a first class NFS feature on Linux is to make sure rpc.gssd works reliably.

Last I checked, we are making a robust effort to harden Kerberos support for NFS. So I don't see any contradiction here.

Now, specifically regarding when rpc.gssd is invoked for lease management security: it is invoked the first time each new server is contacted. If you mount the same server many times, there should be just one upcall.

And, if auth_rpcgss.ko is not loaded, there will be no upcall. Ever.

--
Chuck Lever
chuck[dot]lever[at]oracle[dot]com





2013-11-13 00:30:54

by Myklebust, Trond

[permalink] [raw]
Subject: Re: [PATCH] Adding the nfs4_secure_mounts bool

On Wed, 2013-11-13 at 11:23 +-1100, NeilBrown wrote:
+AD4- But back to my problem: Following Trond's suggestion I've come up with the
+AD4- following patch. Does it look right?
+AD4-
+AD4- The +ACI-fd +AD0- -1+ACI- is just to stop us trying to close a non-open fd in an error
+AD4- path.
+AD4-
+AD4- The change from testing -+AD4-servicename to -+AD4-prog stops us from repeating the
+AD4- failed DNS lookup on every request, not that the failure isn't fatal.
+AD4-
+AD4- The last stanza makes sure we always reply to an upcall, with EINVAL if
+AD4- nothing else seems appropriate.

Wouldn't EACCES be more appropriate as a default?

--
Trond Myklebust
Linux NFS client maintainer

NetApp
Trond.Myklebust+AEA-netapp.com
http://www.netapp.com

2013-11-12 05:29:48

by Myklebust, Trond

[permalink] [raw]
Subject: Re: [PATCH] Adding the nfs4_secure_mounts bool


On Nov 12, 2013, at 0:11, NeilBrown <[email protected]> wrote:

> On Mon, 11 Nov 2013 15:33:14 -0500 Chuck Lever <[email protected]> wrote:
>
>>
>> On Nov 11, 2013, at 1:59 PM, Steve Dickson <[email protected]> wrote:
>>
>>> On 11/11/13 13:30, Chuck Lever wrote:
>>>>
>>>> On Nov 11, 2013, at 1:06 PM, Steve Dickson <[email protected]> wrote:
>>>>
>>>>>
>>>>>
>>>>> On 09/11/13 18:12, Myklebust, Trond wrote:
>>>>>> One alternative to the above scheme, which I believe that I?ve
>>>>>> suggested before, is to have a permanent entry in rpc_pipefs
>>>>>> that rpc.gssd can open and that the kernel can use to detect
>>>>>> that it is running. If we make it /var/lib/nfs/rpc_pipefs/gssd/clnt00/gssd,
>>>>>> then AFAICS we don?t need to change nfs-utils at all, since all newer
>>>>>> versions of rpc.gssd will try to open for read anything of the form
>>>>>> /var/lib/nfs/rpc_pipefs/*/clntXX/gssd...
>>>>>
>>>>> After further review I am going going have to disagree with you on this.
>>>>> Since all the context is cached on the initial mount the kernel
>>>>> should be using the call_usermodehelper() to call up to rpc.gssd
>>>>> to get the context, which means we could put this upcall noise
>>>>> to bed... forever! :-)
>>>>
>>>> Ask Al Viro for his comments on whether the kernel should start
>>>> gssd (either a daemon or a script). Hint: wear your kevlar underpants.
>>> I was thinking gssd would become a the gssd-cmd command... Al does not
>>> like the call_usermodehelper() interface?
>>
>> He doesn't have a problem with call_usermodehelper() in general. However, the kernel cannot guarantee security if it has to run a fixed command line. Go ask him to explain.
>>
>>
>>>
>>>>
>>>> Have you tried Trond's approach yet?
>>> Looking into it... But nothing is trivial in that code...
>>>
>>>>
>>>>> I realize this is not going happen overnight, so I would still
>>>>> like to propose my nfs4_secure_mounts bool patch as bridge
>>>>> to the new call_usermodehelper() since its the cleanest
>>>>> solution so far...
>>>>>
>>>>> Thoughts?
>>>>
>>>> We have workarounds already that work on every kernel since 3.8.
>>>>
>>> The one that logs 5 to 20 lines (depending on thins are setup or not)
>>> per mount? That does work in some environments but no all. ;-)
>>
>> When does running rpc.gssd not work?
>
> Oohh ooh.. Pick me. Pick me!! I can answer that one.
>
> Running rpc.gssd does not work if you are mounting a filesystem using the IP
> address of the server and that IP address doesn't have a matching hostname
> anywhere that can be found:
>
> In a newly creating minimal kvm install without rpc.gssd running,
> mount 10.0.2.2:/home /mnt
>
> sleeps for 15 seconds then succeeds.
> If I start rpc.gssd, then the same command takes forever.
>
> strace of rpc.gssd shows that it complains about not being able to resolve
> the host name and "ERROR: failed to read service info". Then it keeps the
> pipes open but never sends any message on them, so the kernel just keeps on
> waiting.
>
> If I change "fail_keep_client" to "fail_destroy_client", then it closes the
> pipe and we get the 15 second timeout back.
> If I change NI_NAMEREQD to 0, then the mount completes instantly. (of course
> that make serious compromise security so it was just for testing).
> (Adding an entry to /etc/hosts also gives instant success).
>
> I'm hoping that someone who understands this code will suggest something
> clever so I don't have to dig through all of it ;-)

rpc.gssd is supposed to do a downcall with a zero-length window and an error message in any situation where it cannot establish a GSS context. Normally, I?d expect an EACCES for the above scenario.

IOW: that?s a blatant rpc.gssd bug. One that will also affect you when you're doing NFSv3 and add ?sec=krb5? to the mount options.

Cheers
Trond

2013-11-11 18:42:06

by Steve Dickson

[permalink] [raw]
Subject: Re: [PATCH] Adding the nfs4_secure_mounts bool



On 11/11/13 13:25, Myklebust, Trond wrote:
>
> On Nov 11, 2013, at 13:06, Steve Dickson <[email protected]> wrote:
>
>>
>>
>> On 09/11/13 18:12, Myklebust, Trond wrote:
>>> One alternative to the above scheme, which I believe that I?ve
>>> suggested before, is to have a permanent entry in rpc_pipefs
>>> that rpc.gssd can open and that the kernel can use to detect
>>> that it is running. If we make it /var/lib/nfs/rpc_pipefs/gssd/clnt00/gssd,
>>> then AFAICS we don?t need to change nfs-utils at all, since all newer
>>> versions of rpc.gssd will try to open for read anything of the form
>>> /var/lib/nfs/rpc_pipefs/*/clntXX/gssd...
>>
>> After further review I am going going have to disagree with you on this.
>> Since all the context is cached on the initial mount the kernel
>
> What context?
The krb5 blob that the kernel is call up to rpc.gssd to get.. Maybe
I'm using the wrong terminology???

>
>> should be using the call_usermodehelper() to call up to rpc.gssd
>> to get the context, which means we could put this upcall noise
>> to bed... forever! :-)
>
> No. We?re not adding yet another up call. We?ve already got two...
>
I was thinking the call_usermodehelper() would replace the rpc_pipfs()
upcall... but just remembered call_usermodehelper() does not have
a clean way to pass data back into the kernel... darn!

steved.

2013-11-11 21:12:28

by Steve Dickson

[permalink] [raw]
Subject: Re: [PATCH] Adding the nfs4_secure_mounts bool



On 11/11/13 15:33, Chuck Lever wrote:
>>>
>>> We have workarounds already that work on every kernel since 3.8.
>>>
>> The one that logs 5 to 20 lines (depending on thins are setup or not)
>> per mount? That does work in some environments but no all. ;-)
>
> When does running rpc.gssd not work?
Define "work"... ;-) Logging 20 error messages on every mount
when the keytab does not exist is not working or not very well.... IMHO..
>
>> Or am I missing one? Please tell me I am!!! :-)
>
> OK: You're missing one.
Thank you...

>
> Client configurations that have auth_rpcgss.ko loaded but do not run rpc.gssd are, quite simply, broken. The gss upcall will not work in that configuration. There is no reason to load auth_rpcgss.ko if user space does not have rpc.gssd enabled and running.
>
> This has been a problem for a very long time, but use cases where the 15 second upcall timeout is encountered have until recently been infrequent, and only occur in situations where the mount options don't work anyway, so no one has bothered to address it.
>
> This broken configuration is due to incorrect system initialization scripts.
Which initialization scripts are you referring to?

>
> If an administrator chooses not to run rpc.gssd, then auth_rpcgss.ko should not be loaded. The kernel deals quite correctly when auth_rpcgss.ko is not loaded -- GSS flavors are simply not supported, and the kernel uses AUTH_SYS by default for everything. (Right there is your magical administrative interface for controlling what security flavors are used and supported).
>
> But the current init scripts load auth_rpcgss.ko unconditionally. Thus any gss upcall will fail until rpc.gssd is started.
>
> With upstream kernel commit 4edaa308 (and following) I added a use case that exposes the upcall timeout much more often. It's a latent bug, but we now hit it during the first mount after every client reboot, so it is noticeable to anyone who leaves nfs-secure.service disabled.
>
> Therefore the scenario we want to avoid is where auth_rpcgss.ko is loaded, but rpc.gssd is not running. There are two obvious ways to avoid this scenario:
>
> A. If auth_rpcgss.ko is loaded unconditionally, make sure rpc.gssd is running unconditionally (and cull the warnings)
>
> B. If you do not want to run rpc.gssd, make sure auth_rpcgss.ko is not loaded
Isn't the case that nfsd will also loads auth_rpcgss. So if server was started we
would be right back to the 15 sec delay?

steved.

>
> Both of these workarounds are configuration changes, not code changes. B. is accomplished simply by blacklisting auth_rpcgss.ko.
>
> We could go a step further and remove the module alias added by upstream kernel commit 71afa85e to prevent the kernel from loading auth_rpcgss.ko automatically, and then make sure the nfs-secure.service script (and the server-side equivalent) loads auth_rpcgss.ko before starting rpc.gssd.
>
> Since you are opposed to running rpc.gssd, blacklisting auth_rpcgss.ko is the easy choice. Adding auth_rpcgss.ko to the module blacklist is no more difficult than setting a kernel module parameter or toggling nfs-secure.service.
>
> However, IMO, in the long run Linux should be installed with rpc.gssd running unconditionally, or dynamically start-able by mount.nfs (like statd works today). We cannot guess a priori whether a user or administrator will want NFS support for GSS flavors, just as we cannot guess a priori whether the user wants support for NFSv3.
>
> Yet we always have NFSv3 support available. Likewise, support for GSS should always be available, and there really is no good reason any more not to leave it running all the time.
>
> As Trond points out, NFSv4 implementations are required to implement GSS security.
>

2013-11-11 18:27:56

by Myklebust, Trond

[permalink] [raw]
Subject: Re: [PATCH] Adding the nfs4_secure_mounts bool


On Nov 11, 2013, at 13:06, Steve Dickson <[email protected]> wrote:

>
>
> On 09/11/13 18:12, Myklebust, Trond wrote:
>> One alternative to the above scheme, which I believe that I?ve
>> suggested before, is to have a permanent entry in rpc_pipefs
>> that rpc.gssd can open and that the kernel can use to detect
>> that it is running. If we make it /var/lib/nfs/rpc_pipefs/gssd/clnt00/gssd,
>> then AFAICS we don?t need to change nfs-utils at all, since all newer
>> versions of rpc.gssd will try to open for read anything of the form
>> /var/lib/nfs/rpc_pipefs/*/clntXX/gssd...
>
> After further review I am going going have to disagree with you on this.
> Since all the context is cached on the initial mount the kernel

What context?

> should be using the call_usermodehelper() to call up to rpc.gssd
> to get the context, which means we could put this upcall noise
> to bed... forever! :-)

No. We?re not adding yet another up call. We?ve already got two...


2013-11-12 05:11:49

by NeilBrown

[permalink] [raw]
Subject: Re: [PATCH] Adding the nfs4_secure_mounts bool

On Mon, 11 Nov 2013 15:33:14 -0500 Chuck Lever <[email protected]> wrote:

>
> On Nov 11, 2013, at 1:59 PM, Steve Dickson <[email protected]> wrote:
>
> > On 11/11/13 13:30, Chuck Lever wrote:
> >>
> >> On Nov 11, 2013, at 1:06 PM, Steve Dickson <[email protected]> wrote:
> >>
> >>>
> >>>
> >>> On 09/11/13 18:12, Myklebust, Trond wrote:
> >>>> One alternative to the above scheme, which I believe that I’ve
> >>>> suggested before, is to have a permanent entry in rpc_pipefs
> >>>> that rpc.gssd can open and that the kernel can use to detect
> >>>> that it is running. If we make it /var/lib/nfs/rpc_pipefs/gssd/clnt00/gssd,
> >>>> then AFAICS we don’t need to change nfs-utils at all, since all newer
> >>>> versions of rpc.gssd will try to open for read anything of the form
> >>>> /var/lib/nfs/rpc_pipefs/*/clntXX/gssd...
> >>>
> >>> After further review I am going going have to disagree with you on this.
> >>> Since all the context is cached on the initial mount the kernel
> >>> should be using the call_usermodehelper() to call up to rpc.gssd
> >>> to get the context, which means we could put this upcall noise
> >>> to bed... forever! :-)
> >>
> >> Ask Al Viro for his comments on whether the kernel should start
> >> gssd (either a daemon or a script). Hint: wear your kevlar underpants.
> > I was thinking gssd would become a the gssd-cmd command... Al does not
> > like the call_usermodehelper() interface?
>
> He doesn't have a problem with call_usermodehelper() in general. However, the kernel cannot guarantee security if it has to run a fixed command line. Go ask him to explain.
>
>
> >
> >>
> >> Have you tried Trond's approach yet?
> > Looking into it... But nothing is trivial in that code...
> >
> >>
> >>> I realize this is not going happen overnight, so I would still
> >>> like to propose my nfs4_secure_mounts bool patch as bridge
> >>> to the new call_usermodehelper() since its the cleanest
> >>> solution so far...
> >>>
> >>> Thoughts?
> >>
> >> We have workarounds already that work on every kernel since 3.8.
> >>
> > The one that logs 5 to 20 lines (depending on thins are setup or not)
> > per mount? That does work in some environments but no all. ;-)
>
> When does running rpc.gssd not work?

Oohh ooh.. Pick me. Pick me!! I can answer that one.

Running rpc.gssd does not work if you are mounting a filesystem using the IP
address of the server and that IP address doesn't have a matching hostname
anywhere that can be found:

In a newly creating minimal kvm install without rpc.gssd running,
mount 10.0.2.2:/home /mnt

sleeps for 15 seconds then succeeds.
If I start rpc.gssd, then the same command takes forever.

strace of rpc.gssd shows that it complains about not being able to resolve
the host name and "ERROR: failed to read service info". Then it keeps the
pipes open but never sends any message on them, so the kernel just keeps on
waiting.

If I change "fail_keep_client" to "fail_destroy_client", then it closes the
pipe and we get the 15 second timeout back.
If I change NI_NAMEREQD to 0, then the mount completes instantly. (of course
that make serious compromise security so it was just for testing).
(Adding an entry to /etc/hosts also gives instant success).

I'm hoping that someone who understands this code will suggest something
clever so I don't have to dig through all of it ;-)

NeilBrown


Attachments:
signature.asc (828.00 B)

2013-11-11 22:59:28

by Steve Dickson

[permalink] [raw]
Subject: Re: [PATCH] Adding the nfs4_secure_mounts bool

Hey,

On 11/11/13 16:47, Chuck Lever wrote:
>
> On Nov 11, 2013, at 4:13 PM, Steve Dickson <[email protected]> wrote:
>
>>
>>
>> On 11/11/13 15:33, Chuck Lever wrote:
>>>>>
>>>>> We have workarounds already that work on every kernel since 3.8.
>>>>>
>>>> The one that logs 5 to 20 lines (depending on thins are setup or not)
>>>> per mount? That does work in some environments but no all. ;-)
>>>
>>> When does running rpc.gssd not work?
>> Define "work"... ;-) Logging 20 error messages on every mount
>> when the keytab does not exist is not working or not very well.... IMHO..
>
> At first it was 5 warnings. Then it was "10-15 errors". Now it's 20 on every mount!
Yeah... that happen there is no keytab...

>
> How many "-v" options do you specify on your rpc.gssd command line? I normally run
> rpc.gssd with "-vv" or even "-vvv". Leaving those options off makes it much
> quieter, and it's the default.
None... Also you if you have a keytab and the server is not known by the KDC, you
get 5 "ERROR: No credentials found for connection to server XXXXX"
I'm not sure why there is 5... I have not investigated it... but I'm
really hoping the kernel is not doing 5 upcalls as it appears...


> Only crazy people crank up the verbosity of rpc.gssd.
And developers... Does that make us crazy? 8-)

>
> Is a mount failing in that case?
No. The mount success after 15 secs.

> Too many error messages is a nit compared to "no longer mounts." If gssd error
> messages are your only beef, then it seems like the solution is pretty obvious.
Actually its putting rpc.gssd in the mount path of every NFS mount... The
daemon is not as harden as a mountd. I'm concern about the stability in
a lager deployment....

In the past, if admins want rpc.gssd in the mount path they had to configure it.
Now we are silently adding, yet another, daemon to the mount path and if
rpc.gssd starts falling on its face, I think it will be difficult to debug,
since the daemon is not expected to be there...

>
>>>
>>>> Or am I missing one? Please tell me I am!!! :-)
>>>
>>> OK: You're missing one.
>> Thank you...
>>
>>>
>>> Client configurations that have auth_rpcgss.ko loaded but do not run rpc.gssd are, quite simply, broken. The gss upcall will not work in that configuration. There is no reason to load auth_rpcgss.ko if user space does not have rpc.gssd enabled and running.
>>>
>>> This has been a problem for a very long time, but use cases where the 15 second upcall timeout is encountered have until recently been infrequent, and only occur in situations where the mount options don't work anyway, so no one has bothered to address it.
>>>
>>> This broken configuration is due to incorrect system initialization scripts.
>> Which initialization scripts are you referring to?
>
> The NFS initialization scripts. What else deals with auth_rpcgss.ko and rpc.gssd?
I just looked through all the systemd scripts and don't see any explicit
modprobs... Back in the day yes, but today no... What am I missing or
not understanding?

>
>>
>>>
>>> If an administrator chooses not to run rpc.gssd, then auth_rpcgss.ko should not be loaded. The kernel deals quite correctly when auth_rpcgss.ko is not loaded -- GSS flavors are simply not supported, and the kernel uses AUTH_SYS by default for everything. (Right there is your magical administrative interface for controlling what security flavors are used and supported).
>>>
>>> But the current init scripts load auth_rpcgss.ko unconditionally. Thus any gss upcall will fail until rpc.gssd is started.
>>>
>>> With upstream kernel commit 4edaa308 (and following) I added a use case that exposes the upcall timeout much more often. It's a latent bug, but we now hit it during the first mount after every client reboot, so it is noticeable to anyone who leaves nfs-secure.service disabled.
>>>
>>> Therefore the scenario we want to avoid is where auth_rpcgss.ko is loaded, but rpc.gssd is not running. There are two obvious ways to avoid this scenario:
>>>
>>> A. If auth_rpcgss.ko is loaded unconditionally, make sure rpc.gssd is running unconditionally (and cull the warnings)
>>>
>>> B. If you do not want to run rpc.gssd, make sure auth_rpcgss.ko is not loaded
>> Isn't the case that nfsd will also loads auth_rpcgss. So if server was started we
>> would be right back to the 15 sec delay?
>
> Only if NFSD wants to make GSS upcalls and rpc.gssd isn't running. Is that ever the case?
Well the reason I asked was I started the server and did a
lsmod | grep rpc
which showed

rpcsec_gss_krb5 31477 1
auth_rpcgss 59369 3 nfsd,rpcsec_gss_krb5
sunrpc 280214 47 nfs,nfsd,rpcsec_gss_krb5,auth_rpcgss,lockd,nfsv

Now for me to do a rmmod auth_rpcgs, to test your theory, I had to do a
rmmod nfsd... Maybe that's just another bug on how things are depended
on each other

BTW, when I did rmmod auth_rpcgs and did a mount... the mount still hung
logging the
RPC: AUTH_GSS upcall timed out.
Please check user daemon is running.
messages...

>
> The point is we currently have a very direct method of telling the kernel
> NFS components not to use GSS or make upcalls. We should use it.
Yes, I agree...

steved.

>
>
>>
>> steved.
>>
>>>
>>> Both of these workarounds are configuration changes, not code changes. B. is accomplished simply by blacklisting auth_rpcgss.ko.
>>>
>>> We could go a step further and remove the module alias added by upstream kernel commit 71afa85e to prevent the kernel from loading auth_rpcgss.ko automatically, and then make sure the nfs-secure.service script (and the server-side equivalent) loads auth_rpcgss.ko before starting rpc.gssd.
>>>
>>> Since you are opposed to running rpc.gssd, blacklisting auth_rpcgss.ko is the easy choice. Adding auth_rpcgss.ko to the module blacklist is no more difficult than setting a kernel module parameter or toggling nfs-secure.service.
>>>
>>> However, IMO, in the long run Linux should be installed with rpc.gssd running unconditionally, or dynamically start-able by mount.nfs (like statd works today). We cannot guess a priori whether a user or administrator will want NFS support for GSS flavors, just as we cannot guess a priori whether the user wants support for NFSv3.
>>>
>>> Yet we always have NFSv3 support available. Likewise, support for GSS should always be available, and there really is no good reason any more not to leave it running all the time.
>>>
>>> As Trond points out, NFSv4 implementations are required to implement GSS security.
>>>
>> --
>> 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
>

2013-11-11 12:59:13

by Steve Dickson

[permalink] [raw]
Subject: Re: [PATCH] Adding the nfs4_secure_mounts bool



On 10/11/13 17:45, Myklebust, Trond wrote:
>
> On Nov 10, 2013, at 17:31, Steve Dickson <[email protected]> wrote:
>
>> Hey Trond,
>>
>> On 09/11/13 18:12, Myklebust, Trond wrote:
>>>
>>> On Nov 9, 2013, at 17:47, Steve Dickson <[email protected]> wrote:
>>>
>>>> The nfs4_secure_mounts controls whether security
>>>> flavors will be tried during the establishing of
>>>> NFSv4 state and the pseudoroot lookups.
>>>>
>>>> This allows security daemon like rpc.gssd to
>>>> tell the kernel that secure mounts should be tried.
>>>>
>>>> To enable secure mounts:
>>>> echo "on" > /proc/fs/nfsfs/secure
>>>>
>>>> To disable secure mounts:
>>>> echo "off" > /proc/fs/nfsfs/secure
>>>>
>>>> Signed-off-by: Steve Dickson <[email protected]>
>>>
>>> Hi Steve,
>>>
>>> So the rpc.gssd would flip the switch to ?on? when it starts up and
>>> to ?off? when it quits?
>> Yes that is the idea... rpc.gssd would be the gatekeeper if you will...
>> I think its better than a mount option or module parameter since
>> they, to used Jeff's words, "tend to live forever!"
>>
>>> What if someone does a ?kill -9??
>> Things always break when they are killed with -9... It is what it is... :-)
>> I am planning on used the atexit() API to manage this...
>>
>>>
>>> One alternative to the above scheme, which I believe that I?ve
>>> suggested before, is to have a permanent entry in rpc_pipefs
>>> that rpc.gssd can open and that the kernel can use to detect that
>>> it is running. If we make it /var/lib/nfs/rpc_pipefs/gssd/clnt00/gssd,
>>> then AFAICS we don?t need to change nfs-utils at all, since all newer
>>> versions of rpc.gssd will try to open for read anything of the form
>>> /var/lib/nfs/rpc_pipefs/*/clntXX/gssd...
>> This seems like this would be reasonable... But from my understanding
>> (which is limited in this area) this would not be an easy thing to do.
>
> Actually, it should be very easy. We have all the machinery to create new pipes today,
> and they already track whether or not there is a gssd daemon listening.
Hmm... I have to wonder out loud, if it was all that trivial,
why was this change include with Chuck's commit back in March?

> The nice thing about this scheme is that if the gssd daemon dies,
> then the pipe is automatically closed, and the kernel tracking will
> immediately notice that.
Sound great!

>
>> So I'm hoping we could used this patch as a bridge from here to there.
>> To give the community a seamless way out of this 15 second delay, today!
>>
>> This patch does not create a API changes like a module parameter or mount
>> option would, plus it eliminates needless message being logged whether
>> rpc.gssd is or is not running. Since it is seamless, pulling it out
>> would will also be seamless?
>> At the end of the day... rpc.gssd is going to have to change in the
>> short term. I think having rpc.gssd enabling secure mounts, like it
>> has since Fedora 2, is still the right path.
>
> I disagree. I think that we can solve this particular problem without
> changing rpc.gssd or any of the sysinit/systemd start/end scripts.
Unfortunately since problem has not been addressed alternate
pathes my be needed...

steved.

>
> --
> Trond Myklebust
> Linux NFS client maintainer
>
> NetApp
> [email protected]
> http://www.netapp.com
>

2013-11-13 01:13:47

by NeilBrown

[permalink] [raw]
Subject: Re: [PATCH] Adding the nfs4_secure_mounts bool

On Wed, 13 Nov 2013 00:30:53 +0000 "Myklebust, Trond"
<[email protected]> wrote:

> On Wed, 2013-11-13 at 11:23 +1100, NeilBrown wrote:
> > But back to my problem: Following Trond's suggestion I've come up with the
> > following patch. Does it look right?
> >
> > The "fd = -1" is just to stop us trying to close a non-open fd in an error
> > path.
> >
> > The change from testing ->servicename to ->prog stops us from repeating the
> > failed DNS lookup on every request, not that the failure isn't fatal.
> >
> > The last stanza makes sure we always reply to an upcall, with EINVAL if
> > nothing else seems appropriate.
>
> Wouldn't EACCES be more appropriate as a default?
>

Maybe. And that is what you suggested before and I mis-remembered - sorry.

However EACCES is "Permission denied" which doesn't quite seem right to me.
It isn't really "you aren't allowed to do that", but "your question doesn't
make sense".

However I'm not fussed. If you prefer EACCES, then I'll make it EACCES.

Thanks,
NeilBrown


Attachments:
signature.asc (828.00 B)

2013-11-10 22:45:23

by Myklebust, Trond

[permalink] [raw]
Subject: Re: [PATCH] Adding the nfs4_secure_mounts bool


On Nov 10, 2013, at 17:31, Steve Dickson <[email protected]> wrote:

> Hey Trond,
>
> On 09/11/13 18:12, Myklebust, Trond wrote:
>>
>> On Nov 9, 2013, at 17:47, Steve Dickson <[email protected]> wrote:
>>
>>> The nfs4_secure_mounts controls whether security
>>> flavors will be tried during the establishing of
>>> NFSv4 state and the pseudoroot lookups.
>>>
>>> This allows security daemon like rpc.gssd to
>>> tell the kernel that secure mounts should be tried.
>>>
>>> To enable secure mounts:
>>> echo "on" > /proc/fs/nfsfs/secure
>>>
>>> To disable secure mounts:
>>> echo "off" > /proc/fs/nfsfs/secure
>>>
>>> Signed-off-by: Steve Dickson <[email protected]>
>>
>> Hi Steve,
>>
>> So the rpc.gssd would flip the switch to ?on? when it starts up and
>> to ?off? when it quits?
> Yes that is the idea... rpc.gssd would be the gatekeeper if you will...
> I think its better than a mount option or module parameter since
> they, to used Jeff's words, "tend to live forever!"
>
>> What if someone does a ?kill -9??
> Things always break when they are killed with -9... It is what it is... :-)
> I am planning on used the atexit() API to manage this...
>
>>
>> One alternative to the above scheme, which I believe that I?ve
>> suggested before, is to have a permanent entry in rpc_pipefs
>> that rpc.gssd can open and that the kernel can use to detect that
>> it is running. If we make it /var/lib/nfs/rpc_pipefs/gssd/clnt00/gssd,
>> then AFAICS we don?t need to change nfs-utils at all, since all newer
>> versions of rpc.gssd will try to open for read anything of the form
>> /var/lib/nfs/rpc_pipefs/*/clntXX/gssd...
> This seems like this would be reasonable... But from my understanding
> (which is limited in this area) this would not be an easy thing to do.

Actually, it should be very easy. We have all the machinery to create new pipes today, and they already track whether or not there is a gssd daemon listening.
The nice thing about this scheme is that if the gssd daemon dies, then the pipe is automatically closed, and the kernel tracking will immediately notice that.

> So I'm hoping we could used this patch as a bridge from here to there.
> To give the community a seamless way out of this 15 second delay, today!
>
> This patch does not create a API changes like a module parameter or mount
> option would, plus it eliminates needless message being logged whether
> rpc.gssd is or is not running. Since it is seamless, pulling it out
> would will also be seamless?
> At the end of the day... rpc.gssd is going to have to change in the
> short term. I think having rpc.gssd enabling secure mounts, like it
> has since Fedora 2, is still the right path.

I disagree. I think that we can solve this particular problem without changing rpc.gssd or any of the sysinit/systemd start/end scripts.

--
Trond Myklebust
Linux NFS client maintainer

NetApp
[email protected]
http://www.netapp.com


2013-11-13 00:24:00

by NeilBrown

[permalink] [raw]
Subject: Re: [PATCH] Adding the nfs4_secure_mounts bool

On Tue, 12 Nov 2013 11:16:34 -0500 "J. Bruce Fields" <[email protected]>
wrote:

> On Tue, Nov 12, 2013 at 05:29:46AM +0000, Myklebust, Trond wrote:
> >
> > On Nov 12, 2013, at 0:11, NeilBrown <[email protected]> wrote:
> >
> > > On Mon, 11 Nov 2013 15:33:14 -0500 Chuck Lever <[email protected]> wrote:
> > >
> > >>
> > >> On Nov 11, 2013, at 1:59 PM, Steve Dickson <[email protected]> wrote:
> > >>
> > >>> On 11/11/13 13:30, Chuck Lever wrote:
> > >>>>
> > >>>> On Nov 11, 2013, at 1:06 PM, Steve Dickson <[email protected]> wrote:
> > >>>>
> > >>>>>
> > >>>>>
> > >>>>> On 09/11/13 18:12, Myklebust, Trond wrote:
> > >>>>>> One alternative to the above scheme, which I believe that I’ve
> > >>>>>> suggested before, is to have a permanent entry in rpc_pipefs
> > >>>>>> that rpc.gssd can open and that the kernel can use to detect
> > >>>>>> that it is running. If we make it /var/lib/nfs/rpc_pipefs/gssd/clnt00/gssd,
> > >>>>>> then AFAICS we don’t need to change nfs-utils at all, since all newer
> > >>>>>> versions of rpc.gssd will try to open for read anything of the form
> > >>>>>> /var/lib/nfs/rpc_pipefs/*/clntXX/gssd...
> > >>>>>
> > >>>>> After further review I am going going have to disagree with you on this.
> > >>>>> Since all the context is cached on the initial mount the kernel
> > >>>>> should be using the call_usermodehelper() to call up to rpc.gssd
> > >>>>> to get the context, which means we could put this upcall noise
> > >>>>> to bed... forever! :-)
> > >>>>
> > >>>> Ask Al Viro for his comments on whether the kernel should start
> > >>>> gssd (either a daemon or a script). Hint: wear your kevlar underpants.
> > >>> I was thinking gssd would become a the gssd-cmd command... Al does not
> > >>> like the call_usermodehelper() interface?
> > >>
> > >> He doesn't have a problem with call_usermodehelper() in general. However, the kernel cannot guarantee security if it has to run a fixed command line. Go ask him to explain.
> > >>
> > >>
> > >>>
> > >>>>
> > >>>> Have you tried Trond's approach yet?
> > >>> Looking into it... But nothing is trivial in that code...
> > >>>
> > >>>>
> > >>>>> I realize this is not going happen overnight, so I would still
> > >>>>> like to propose my nfs4_secure_mounts bool patch as bridge
> > >>>>> to the new call_usermodehelper() since its the cleanest
> > >>>>> solution so far...
> > >>>>>
> > >>>>> Thoughts?
> > >>>>
> > >>>> We have workarounds already that work on every kernel since 3.8.
> > >>>>
> > >>> The one that logs 5 to 20 lines (depending on thins are setup or not)
> > >>> per mount? That does work in some environments but no all. ;-)
> > >>
> > >> When does running rpc.gssd not work?
> > >
> > > Oohh ooh.. Pick me. Pick me!! I can answer that one.
> > >
> > > Running rpc.gssd does not work if you are mounting a filesystem using the IP
> > > address of the server and that IP address doesn't have a matching hostname
> > > anywhere that can be found:
> > >
> > > In a newly creating minimal kvm install without rpc.gssd running,
> > > mount 10.0.2.2:/home /mnt
> > >
> > > sleeps for 15 seconds then succeeds.
> > > If I start rpc.gssd, then the same command takes forever.
> > >
> > > strace of rpc.gssd shows that it complains about not being able to resolve
> > > the host name and "ERROR: failed to read service info". Then it keeps the
> > > pipes open but never sends any message on them, so the kernel just keeps on
> > > waiting.
> > >
> > > If I change "fail_keep_client" to "fail_destroy_client", then it closes the
> > > pipe and we get the 15 second timeout back.
> > > If I change NI_NAMEREQD to 0, then the mount completes instantly. (of course
> > > that make serious compromise security so it was just for testing).
> > > (Adding an entry to /etc/hosts also gives instant success).
> > >
> > > I'm hoping that someone who understands this code will suggest something
> > > clever so I don't have to dig through all of it ;-)
> >
> > rpc.gssd is supposed to do a downcall with a zero-length window and an error message in any situation where it cannot establish a GSS context. Normally, I’d expect an EACCES for the above scenario.
> >
> > IOW: that’s a blatant rpc.gssd bug. One that will also affect you when you're doing NFSv3 and add ‘sec=krb5’ to the mount options.
>
> Also why is gssd trying to do a DNS lookup in this case? This sounds
> similar to what f9f5450f8f94 "Avoid DNS reverse resolution for server
> names (take 3)" was trying to fix?

It is quite possible that I misunderstand something. But this is my
understanding.

1/ "mount" allows you to use either an IP address or a host name to mount a
filesystem.
2/ gss requires a hostname to identify the server and find it's key (IP not
sufficient).
3/ If you use a host name to mount a filesystem, then that exact same host
name should be used by gssd to identify the server and its key.
The above mentioned patch was trying to enforce this. The idea was to
collect the name given to the 'mount', see if it looked like an IP address
or a Server name. If the later, just use it. If the former, do a reverse
lookup because an IP address is no use by itself for gss.
Previously it would always do a reverse DNS lookup from the IP address
that was determined from the server-name-or-IP-address.
Unfortunately this patch was broken - got the test backwards.
A follow-up patch fixed the test: c93e8d8eeafec3e32

4/ So the above patch was not intended to address the case of mount-by-IP
address at all - and this is the case that is causing me problems.


But back to my problem: Following Trond's suggestion I've come up with the
following patch. Does it look right?

The "fd = -1" is just to stop us trying to close a non-open fd in an error
path.

The change from testing ->servicename to ->prog stops us from repeating the
failed DNS lookup on every request, not that the failure isn't fatal.

The last stanza makes sure we always reply to an upcall, with EINVAL if
nothing else seems appropriate.

The patch seems to work for my particular case but a more general review
would be appreciated.

Thanks,
NeilBrown



diff --git a/utils/gssd/gssd_proc.c b/utils/gssd/gssd_proc.c
index b48d1637cd36..00b4bc779b7c 100644
--- a/utils/gssd/gssd_proc.c
+++ b/utils/gssd/gssd_proc.c
@@ -256,6 +256,7 @@ read_service_info(char *info_file_name, char **servicename, char **servername,
if ((nbytes = read(fd, buf, INFOBUFLEN)) == -1)
goto fail;
close(fd);
+ fd = -1;
buf[nbytes] = '\0';

numfields = sscanf(buf,"RPC server: %127s\n"
@@ -403,11 +404,10 @@ process_clnt_dir_files(struct clnt_info * clp)
return -1;
snprintf(info_file_name, sizeof(info_file_name), "%s/info",
clp->dirname);
- if ((clp->servicename == NULL) &&
- read_service_info(info_file_name, &clp->servicename,
- &clp->servername, &clp->prog, &clp->vers,
- &clp->protocol, (struct sockaddr *) &clp->addr))
- return -1;
+ if (clp->prog == 0)
+ read_service_info(info_file_name, &clp->servicename,
+ &clp->servername, &clp->prog, &clp->vers,
+ &clp->protocol, (struct sockaddr *) &clp->addr);
return 0;
}

@@ -1320,11 +1320,14 @@ handle_gssd_upcall(struct clnt_info *clp)
}
}

- if (strcmp(mech, "krb5") == 0)
+ if (strcmp(mech, "krb5") == 0 && clp->servername)
process_krb5_upcall(clp, uid, clp->gssd_fd, target, service);
- else
- printerr(0, "WARNING: handle_gssd_upcall: "
- "received unknown gss mech '%s'\n", mech);
+ else {
+ if (clp->servername)
+ printerr(0, "WARNING: handle_gssd_upcall: "
+ "received unknown gss mech '%s'\n", mech);
+ do_error_downcall(clp->gssd_fd, uid, -EINVAL);
+ }

out:
free(lbuf);


Attachments:
signature.asc (828.00 B)

2013-11-14 01:11:23

by NeilBrown

[permalink] [raw]
Subject: Re: [PATCH] Adding the nfs4_secure_mounts bool

On Wed, 13 Nov 2013 01:26:56 +0000 "Myklebust, Trond"
<[email protected]> wrote:

> On Wed, 2013-11-13 at 12:13 +1100, NeilBrown wrote:
> > On Wed, 13 Nov 2013 00:30:53 +0000 "Myklebust, Trond"
> > <[email protected]> wrote:
> >
> > > On Wed, 2013-11-13 at 11:23 +1100, NeilBrown wrote:
> > > > But back to my problem: Following Trond's suggestion I've come up with the
> > > > following patch. Does it look right?
> > > >
> > > > The "fd = -1" is just to stop us trying to close a non-open fd in an error
> > > > path.
> > > >
> > > > The change from testing ->servicename to ->prog stops us from repeating the
> > > > failed DNS lookup on every request, not that the failure isn't fatal.
> > > >
> > > > The last stanza makes sure we always reply to an upcall, with EINVAL if
> > > > nothing else seems appropriate.
> > >
> > > Wouldn't EACCES be more appropriate as a default?
> > >
> >
> > Maybe. And that is what you suggested before and I mis-remembered - sorry.
> >
> > However EACCES is "Permission denied" which doesn't quite seem right to me.
> > It isn't really "you aren't allowed to do that", but "your question doesn't
> > make sense".
> >
> > However I'm not fussed. If you prefer EACCES, then I'll make it EACCES.
>
> If you look at gss_pipe_downcall(), then you'll note that it treats
> EINVAL as a temporary error, and converts it to EAGAIN. That again
> causes call_refreshresult to retry the upcall 2 more times before
> failing with EACCES anyway...
>

Yes, I see now, thanks.

I also see a 'BUG()' in there if the error code returned from user-space
isn't in the known list. I suspect that should at most be a WARN, and
probably removed altogether.

Thanks,
NeilBrown


Attachments:
signature.asc (828.00 B)

2013-11-11 20:33:21

by Chuck Lever

[permalink] [raw]
Subject: Re: [PATCH] Adding the nfs4_secure_mounts bool


On Nov 11, 2013, at 1:59 PM, Steve Dickson <[email protected]> wrote:

> On 11/11/13 13:30, Chuck Lever wrote:
>>
>> On Nov 11, 2013, at 1:06 PM, Steve Dickson <[email protected]> wrote:
>>
>>>
>>>
>>> On 09/11/13 18:12, Myklebust, Trond wrote:
>>>> One alternative to the above scheme, which I believe that I?ve
>>>> suggested before, is to have a permanent entry in rpc_pipefs
>>>> that rpc.gssd can open and that the kernel can use to detect
>>>> that it is running. If we make it /var/lib/nfs/rpc_pipefs/gssd/clnt00/gssd,
>>>> then AFAICS we don?t need to change nfs-utils at all, since all newer
>>>> versions of rpc.gssd will try to open for read anything of the form
>>>> /var/lib/nfs/rpc_pipefs/*/clntXX/gssd...
>>>
>>> After further review I am going going have to disagree with you on this.
>>> Since all the context is cached on the initial mount the kernel
>>> should be using the call_usermodehelper() to call up to rpc.gssd
>>> to get the context, which means we could put this upcall noise
>>> to bed... forever! :-)
>>
>> Ask Al Viro for his comments on whether the kernel should start
>> gssd (either a daemon or a script). Hint: wear your kevlar underpants.
> I was thinking gssd would become a the gssd-cmd command... Al does not
> like the call_usermodehelper() interface?

He doesn't have a problem with call_usermodehelper() in general. However, the kernel cannot guarantee security if it has to run a fixed command line. Go ask him to explain.


>
>>
>> Have you tried Trond's approach yet?
> Looking into it... But nothing is trivial in that code...
>
>>
>>> I realize this is not going happen overnight, so I would still
>>> like to propose my nfs4_secure_mounts bool patch as bridge
>>> to the new call_usermodehelper() since its the cleanest
>>> solution so far...
>>>
>>> Thoughts?
>>
>> We have workarounds already that work on every kernel since 3.8.
>>
> The one that logs 5 to 20 lines (depending on thins are setup or not)
> per mount? That does work in some environments but no all. ;-)

When does running rpc.gssd not work?

> Or am I missing one? Please tell me I am!!! :-)

OK: You're missing one.

Client configurations that have auth_rpcgss.ko loaded but do not run rpc.gssd are, quite simply, broken. The gss upcall will not work in that configuration. There is no reason to load auth_rpcgss.ko if user space does not have rpc.gssd enabled and running.

This has been a problem for a very long time, but use cases where the 15 second upcall timeout is encountered have until recently been infrequent, and only occur in situations where the mount options don't work anyway, so no one has bothered to address it.

This broken configuration is due to incorrect system initialization scripts.

If an administrator chooses not to run rpc.gssd, then auth_rpcgss.ko should not be loaded. The kernel deals quite correctly when auth_rpcgss.ko is not loaded -- GSS flavors are simply not supported, and the kernel uses AUTH_SYS by default for everything. (Right there is your magical administrative interface for controlling what security flavors are used and supported).

But the current init scripts load auth_rpcgss.ko unconditionally. Thus any gss upcall will fail until rpc.gssd is started.

With upstream kernel commit 4edaa308 (and following) I added a use case that exposes the upcall timeout much more often. It's a latent bug, but we now hit it during the first mount after every client reboot, so it is noticeable to anyone who leaves nfs-secure.service disabled.

Therefore the scenario we want to avoid is where auth_rpcgss.ko is loaded, but rpc.gssd is not running. There are two obvious ways to avoid this scenario:

A. If auth_rpcgss.ko is loaded unconditionally, make sure rpc.gssd is running unconditionally (and cull the warnings)

B. If you do not want to run rpc.gssd, make sure auth_rpcgss.ko is not loaded

Both of these workarounds are configuration changes, not code changes. B. is accomplished simply by blacklisting auth_rpcgss.ko.

We could go a step further and remove the module alias added by upstream kernel commit 71afa85e to prevent the kernel from loading auth_rpcgss.ko automatically, and then make sure the nfs-secure.service script (and the server-side equivalent) loads auth_rpcgss.ko before starting rpc.gssd.

Since you are opposed to running rpc.gssd, blacklisting auth_rpcgss.ko is the easy choice. Adding auth_rpcgss.ko to the module blacklist is no more difficult than setting a kernel module parameter or toggling nfs-secure.service.

However, IMO, in the long run Linux should be installed with rpc.gssd running unconditionally, or dynamically start-able by mount.nfs (like statd works today). We cannot guess a priori whether a user or administrator will want NFS support for GSS flavors, just as we cannot guess a priori whether the user wants support for NFSv3.

Yet we always have NFSv3 support available. Likewise, support for GSS should always be available, and there really is no good reason any more not to leave it running all the time.

As Trond points out, NFSv4 implementations are required to implement GSS security.

--
Chuck Lever
chuck[dot]lever[at]oracle[dot]com





2013-11-11 18:58:25

by Steve Dickson

[permalink] [raw]
Subject: Re: [PATCH] Adding the nfs4_secure_mounts bool

On 11/11/13 13:30, Chuck Lever wrote:
>
> On Nov 11, 2013, at 1:06 PM, Steve Dickson <[email protected]> wrote:
>
>>
>>
>> On 09/11/13 18:12, Myklebust, Trond wrote:
>>> One alternative to the above scheme, which I believe that I?ve
>>> suggested before, is to have a permanent entry in rpc_pipefs
>>> that rpc.gssd can open and that the kernel can use to detect
>>> that it is running. If we make it /var/lib/nfs/rpc_pipefs/gssd/clnt00/gssd,
>>> then AFAICS we don?t need to change nfs-utils at all, since all newer
>>> versions of rpc.gssd will try to open for read anything of the form
>>> /var/lib/nfs/rpc_pipefs/*/clntXX/gssd...
>>
>> After further review I am going going have to disagree with you on this.
>> Since all the context is cached on the initial mount the kernel
>> should be using the call_usermodehelper() to call up to rpc.gssd
>> to get the context, which means we could put this upcall noise
>> to bed... forever! :-)
>
> Ask Al Viro for his comments on whether the kernel should start
> gssd (either a daemon or a script). Hint: wear your kevlar underpants.
I was thinking gssd would become a the gssd-cmd command... Al does not
like the call_usermodehelper() interface?

>
> Have you tried Trond's approach yet?
Looking into it... But nothing is trivial in that code...

>
>> I realize this is not going happen overnight, so I would still
>> like to propose my nfs4_secure_mounts bool patch as bridge
>> to the new call_usermodehelper() since its the cleanest
>> solution so far...
>>
>> Thoughts?
>
> We have workarounds already that work on every kernel since 3.8.
>
The one that logs 5 to 20 lines (depending on thins are setup or not)
per mount? That does work in some environments but no all. ;-)
Or am I missing one? Please tell me I am!!! :-)

steved.

2013-11-10 22:30:13

by Steve Dickson

[permalink] [raw]
Subject: Re: [PATCH] Adding the nfs4_secure_mounts bool

Hey Trond,

On 09/11/13 18:12, Myklebust, Trond wrote:
>
> On Nov 9, 2013, at 17:47, Steve Dickson <[email protected]> wrote:
>
>> The nfs4_secure_mounts controls whether security
>> flavors will be tried during the establishing of
>> NFSv4 state and the pseudoroot lookups.
>>
>> This allows security daemon like rpc.gssd to
>> tell the kernel that secure mounts should be tried.
>>
>> To enable secure mounts:
>> echo "on" > /proc/fs/nfsfs/secure
>>
>> To disable secure mounts:
>> echo "off" > /proc/fs/nfsfs/secure
>>
>> Signed-off-by: Steve Dickson <[email protected]>
>
> Hi Steve,
>
> So the rpc.gssd would flip the switch to ?on? when it starts up and
> to ?off? when it quits?
Yes that is the idea... rpc.gssd would be the gatekeeper if you will...
I think its better than a mount option or module parameter since
they, to used Jeff's words, "tend to live forever!"

> What if someone does a ?kill -9??
Things always break when they are killed with -9... It is what it is... :-)
I am planning on used the atexit() API to manage this...

>
> One alternative to the above scheme, which I believe that I?ve
> suggested before, is to have a permanent entry in rpc_pipefs
> that rpc.gssd can open and that the kernel can use to detect that
> it is running. If we make it /var/lib/nfs/rpc_pipefs/gssd/clnt00/gssd,
> then AFAICS we don?t need to change nfs-utils at all, since all newer
> versions of rpc.gssd will try to open for read anything of the form
> /var/lib/nfs/rpc_pipefs/*/clntXX/gssd...
This seems like this would be reasonable... But from my understanding
(which is limited in this area) this would not be an easy thing to do.

So I'm hoping we could used this patch as a bridge from here to there.
To give the community a seamless way out of this 15 second delay, today!

This patch does not create a API changes like a module parameter or mount
option would, plus it eliminates needless message being logged whether
rpc.gssd is or is not running. Since it is seamless, pulling it out
would will also be seamless...

At the end of the day... rpc.gssd is going to have to change in the
short term. I think having rpc.gssd enabling secure mounts, like it
has since Fedora 2, is still the right path.

BTW, thank for the cycles you (Chuck, Bruce and Jeff) have been giving
me on this... they are much appreciated!

steved.



2013-11-14 01:10:24

by NeilBrown

[permalink] [raw]
Subject: Re: [PATCH] Adding the nfs4_secure_mounts bool

On Wed, 13 Nov 2013 04:15:26 +0000 "Myklebust, Trond"
<[email protected]> wrote:

> On Tue, 2013-11-12 at 22:46 -0500, J. Bruce Fields wrote:
>
> > OK, but it still seems dumb to even attempt the reverse lookup: the
> > lookup probably isn't secure, and the mount commandline should have a
> > name that we can match to a krb5 principal without needing any other
> > lookups.
> >
> > So I'd think reasonable behavior in this case would be to just try the
> > IP address on the chance there's actually an nfs/x.y.z.w@REALM
> > principal. (Or just fail outright if kerberos doesn't allow principals
> > that look like that.)
>
> Looking through the krb5.conf manpage etc it looks as if a lot of this
> functionality should be covered by the krb protocol itself without us
> needing to do explicit reverse lookups in rpc.gssd. I'm thinking of the
> 'canonicalize' and 'rdns' options, for instance. Am I wrong?
>

I suspect there is a good chance that you are correct, though my man page
only mentions "rdns", not "canonicalize" so there may be some version
dependency to think about.

However I think fixing this is a separate (though related) issue to fixing my
current problem and would probably require more code examination and testing
than I feel inclined to at the moment. So I'll leave this side of the
question alone and just fix the bit that is clearly broken.

Thanks,
NeilBrown


Attachments:
signature.asc (828.00 B)

2013-11-13 04:15:28

by Myklebust, Trond

[permalink] [raw]
Subject: Re: [PATCH] Adding the nfs4_secure_mounts bool

On Tue, 2013-11-12 at 22:46 -0500, J. Bruce Fields wrote:

+AD4- OK, but it still seems dumb to even attempt the reverse lookup: the
+AD4- lookup probably isn't secure, and the mount commandline should have a
+AD4- name that we can match to a krb5 principal without needing any other
+AD4- lookups.
+AD4-
+AD4- So I'd think reasonable behavior in this case would be to just try the
+AD4- IP address on the chance there's actually an nfs/x.y.z.w+AEA-REALM
+AD4- principal. (Or just fail outright if kerberos doesn't allow principals
+AD4- that look like that.)

Looking through the krb5.conf manpage etc it looks as if a lot of this
functionality should be covered by the krb protocol itself without us
needing to do explicit reverse lookups in rpc.gssd. I'm thinking of the
'canonicalize' and 'rdns' options, for instance. Am I wrong?

--
Trond Myklebust
Linux NFS client maintainer

NetApp
Trond.Myklebust+AEA-netapp.com
http://www.netapp.com

2013-11-12 16:09:24

by Chuck Lever

[permalink] [raw]
Subject: Re: [PATCH] Adding the nfs4_secure_mounts bool


On Nov 11, 2013, at 6:00 PM, Steve Dickson <[email protected]> wrote:

> Hey,
>
> On 11/11/13 16:47, Chuck Lever wrote:
>>
>> On Nov 11, 2013, at 4:13 PM, Steve Dickson <[email protected]> wrote:
>>
>>>
>>>
>>> On 11/11/13 15:33, Chuck Lever wrote:
>>>>>>
>>>>>> We have workarounds already that work on every kernel since 3.8.
>>>>>>
>>>>> The one that logs 5 to 20 lines (depending on thins are setup or not)
>>>>> per mount? That does work in some environments but no all. ;-)
>>>>
>>>> When does running rpc.gssd not work?
>>> Define "work"... ;-) Logging 20 error messages on every mount
>>> when the keytab does not exist is not working or not very well.... IMHO..
>>
>> At first it was 5 warnings. Then it was "10-15 errors". Now it's 20 on every mount!
> Yeah... that happen there is no keytab...

Seems like a gssd bug to me.

>
>>
>> How many "-v" options do you specify on your rpc.gssd command line? I normally run
>> rpc.gssd with "-vv" or even "-vvv". Leaving those options off makes it much
>> quieter, and it's the default.
> None... Also you if you have a keytab and the server is not known by the KDC, you
> get 5 "ERROR: No credentials found for connection to server XXXXX"
> I'm not sure why there is 5... I have not investigated it... but I'm
> really hoping the kernel is not doing 5 upcalls as it appears...

I doubt the kernel is doing 5 upcalls. Another gssd bug, perhaps?

>
>
>> Only crazy people crank up the verbosity of rpc.gssd.
> And developers... Does that make us crazy? 8-)
>
>>
>> Is a mount failing in that case?
> No. The mount success after 15 secs.
>
>> Too many error messages is a nit compared to "no longer mounts." If gssd error
>> messages are your only beef, then it seems like the solution is pretty obvious.
> Actually its putting rpc.gssd in the mount path of every NFS mount... The
> daemon is not as harden as a mountd. I'm concern about the stability in
> a lager deployment....

If gssd isn't stable, we have bigger problems than a 15 second delay.

> In the past, if admins want rpc.gssd in the mount path they had to configure it.
> Now we are silently adding, yet another, daemon to the mount path and if
> rpc.gssd starts falling on its face, I think it will be difficult to debug,
> since the daemon is not expected to be there...

Our only real choice here is to fix gssd. Anything else is punting the problem down the road.

>
>>
>>>>
>>>>> Or am I missing one? Please tell me I am!!! :-)
>>>>
>>>> OK: You're missing one.
>>> Thank you...
>>>
>>>>
>>>> Client configurations that have auth_rpcgss.ko loaded but do not run rpc.gssd are, quite simply, broken. The gss upcall will not work in that configuration. There is no reason to load auth_rpcgss.ko if user space does not have rpc.gssd enabled and running.
>>>>
>>>> This has been a problem for a very long time, but use cases where the 15 second upcall timeout is encountered have until recently been infrequent, and only occur in situations where the mount options don't work anyway, so no one has bothered to address it.
>>>>
>>>> This broken configuration is due to incorrect system initialization scripts.
>>> Which initialization scripts are you referring to?
>>
>> The NFS initialization scripts. What else deals with auth_rpcgss.ko and rpc.gssd?
> I just looked through all the systemd scripts and don't see any explicit
> modprobs... Back in the day yes, but today no... What am I missing or
> not understanding?

The next thing to try is reverting upstream kernel commit 71afa85e.

If what I said yesterday is correct (that loading auth_rpcgss.ko is worse than useless if rpc.gssd is not running) then the kernel should never autoload auth_rpcgss.ko.


>
>>
>>>
>>>>
>>>> If an administrator chooses not to run rpc.gssd, then auth_rpcgss.ko should not be loaded. The kernel deals quite correctly when auth_rpcgss.ko is not loaded -- GSS flavors are simply not supported, and the kernel uses AUTH_SYS by default for everything. (Right there is your magical administrative interface for controlling what security flavors are used and supported).
>>>>
>>>> But the current init scripts load auth_rpcgss.ko unconditionally. Thus any gss upcall will fail until rpc.gssd is started.
>>>>
>>>> With upstream kernel commit 4edaa308 (and following) I added a use case that exposes the upcall timeout much more often. It's a latent bug, but we now hit it during the first mount after every client reboot, so it is noticeable to anyone who leaves nfs-secure.service disabled.
>>>>
>>>> Therefore the scenario we want to avoid is where auth_rpcgss.ko is loaded, but rpc.gssd is not running. There are two obvious ways to avoid this scenario:
>>>>
>>>> A. If auth_rpcgss.ko is loaded unconditionally, make sure rpc.gssd is running unconditionally (and cull the warnings)
>>>>
>>>> B. If you do not want to run rpc.gssd, make sure auth_rpcgss.ko is not loaded
>>> Isn't the case that nfsd will also loads auth_rpcgss. So if server was started we
>>> would be right back to the 15 sec delay?
>>
>> Only if NFSD wants to make GSS upcalls and rpc.gssd isn't running. Is that ever the case?
> Well the reason I asked was I started the server and did a
> lsmod | grep rpc
> which showed
>
> rpcsec_gss_krb5 31477 1
> auth_rpcgss 59369 3 nfsd,rpcsec_gss_krb5
> sunrpc 280214 47 nfs,nfsd,rpcsec_gss_krb5,auth_rpcgss,lockd,nfsv
>
> Now for me to do a rmmod auth_rpcgs, to test your theory, I had to do a
> rmmod nfsd... Maybe that's just another bug on how things are depended
> on each other
>
> BTW, when I did rmmod auth_rpcgs and did a mount... the mount still hung
> logging the
> RPC: AUTH_GSS upcall timed out.
> Please check user daemon is running.
> messages...

Those messages come from auth_rpcgss. So, obviously, "rmmod auth_rpcgss" did nothing, probably because it was already pinned.


>
>>
>> The point is we currently have a very direct method of telling the kernel
>> NFS components not to use GSS or make upcalls. We should use it.
> Yes, I agree...
>
> steved.
>
>>
>>
>>>
>>> steved.
>>>
>>>>
>>>> Both of these workarounds are configuration changes, not code changes. B. is accomplished simply by blacklisting auth_rpcgss.ko.
>>>>
>>>> We could go a step further and remove the module alias added by upstream kernel commit 71afa85e to prevent the kernel from loading auth_rpcgss.ko automatically, and then make sure the nfs-secure.service script (and the server-side equivalent) loads auth_rpcgss.ko before starting rpc.gssd.
>>>>
>>>> Since you are opposed to running rpc.gssd, blacklisting auth_rpcgss.ko is the easy choice. Adding auth_rpcgss.ko to the module blacklist is no more difficult than setting a kernel module parameter or toggling nfs-secure.service.
>>>>
>>>> However, IMO, in the long run Linux should be installed with rpc.gssd running unconditionally, or dynamically start-able by mount.nfs (like statd works today). We cannot guess a priori whether a user or administrator will want NFS support for GSS flavors, just as we cannot guess a priori whether the user wants support for NFSv3.
>>>>
>>>> Yet we always have NFSv3 support available. Likewise, support for GSS should always be available, and there really is no good reason any more not to leave it running all the time.
>>>>
>>>> As Trond points out, NFSv4 implementations are required to implement GSS security.
>>>>
>>> --
>>> 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
>>
> --
> 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

--
Chuck Lever
chuck[dot]lever[at]oracle[dot]com





2013-11-12 16:10:15

by J. Bruce Fields

[permalink] [raw]
Subject: Re: [PATCH] Adding the nfs4_secure_mounts bool

On Mon, Nov 11, 2013 at 06:00:26PM -0500, Steve Dickson wrote:
> Hey,
>
> On 11/11/13 16:47, Chuck Lever wrote:
> >
> > On Nov 11, 2013, at 4:13 PM, Steve Dickson <[email protected]> wrote:
> >
> >>
> >>
> >> On 11/11/13 15:33, Chuck Lever wrote:
> >>>>>
> >>>>> We have workarounds already that work on every kernel since 3.8.
> >>>>>
> >>>> The one that logs 5 to 20 lines (depending on thins are setup or not)
> >>>> per mount? That does work in some environments but no all. ;-)
> >>>
> >>> When does running rpc.gssd not work?
> >> Define "work"... ;-) Logging 20 error messages on every mount
> >> when the keytab does not exist is not working or not very well.... IMHO..
> >
> > At first it was 5 warnings. Then it was "10-15 errors". Now it's 20 on every mount!
> Yeah... that happen there is no keytab...
>
> >
> > How many "-v" options do you specify on your rpc.gssd command line? I normally run
> > rpc.gssd with "-vv" or even "-vvv". Leaving those options off makes it much
> > quieter, and it's the default.
> None... Also you if you have a keytab and the server is not known by the KDC, you
> get 5 "ERROR: No credentials found for connection to server XXXXX"
> I'm not sure why there is 5... I have not investigated it... but I'm
> really hoping the kernel is not doing 5 upcalls as it appears...
>
>
> > Only crazy people crank up the verbosity of rpc.gssd.
> And developers... Does that make us crazy? 8-)
>
> >
> > Is a mount failing in that case?
> No. The mount success after 15 secs.
>
> > Too many error messages is a nit compared to "no longer mounts." If gssd error
> > messages are your only beef, then it seems like the solution is pretty obvious.
> Actually its putting rpc.gssd in the mount path of every NFS mount... The
> daemon is not as harden as a mountd. I'm concern about the stability in
> a lager deployment....
>
> In the past, if admins want rpc.gssd in the mount path they had to configure it.
> Now we are silently adding, yet another, daemon to the mount path and if
> rpc.gssd starts falling on its face, I think it will be difficult to debug,
> since the daemon is not expected to be there...
>
> >
> >>>
> >>>> Or am I missing one? Please tell me I am!!! :-)
> >>>
> >>> OK: You're missing one.
> >> Thank you...
> >>
> >>>
> >>> Client configurations that have auth_rpcgss.ko loaded but do not run rpc.gssd are, quite simply, broken. The gss upcall will not work in that configuration. There is no reason to load auth_rpcgss.ko if user space does not have rpc.gssd enabled and running.
> >>>
> >>> This has been a problem for a very long time, but use cases where the 15 second upcall timeout is encountered have until recently been infrequent, and only occur in situations where the mount options don't work anyway, so no one has bothered to address it.
> >>>
> >>> This broken configuration is due to incorrect system initialization scripts.
> >> Which initialization scripts are you referring to?
> >
> > The NFS initialization scripts. What else deals with auth_rpcgss.ko and rpc.gssd?
> I just looked through all the systemd scripts and don't see any explicit
> modprobs... Back in the day yes, but today no... What am I missing or
> not understanding?
>
> >
> >>
> >>>
> >>> If an administrator chooses not to run rpc.gssd, then auth_rpcgss.ko should not be loaded. The kernel deals quite correctly when auth_rpcgss.ko is not loaded -- GSS flavors are simply not supported, and the kernel uses AUTH_SYS by default for everything. (Right there is your magical administrative interface for controlling what security flavors are used and supported).
> >>>
> >>> But the current init scripts load auth_rpcgss.ko unconditionally. Thus any gss upcall will fail until rpc.gssd is started.
> >>>
> >>> With upstream kernel commit 4edaa308 (and following) I added a use case that exposes the upcall timeout much more often. It's a latent bug, but we now hit it during the first mount after every client reboot, so it is noticeable to anyone who leaves nfs-secure.service disabled.
> >>>
> >>> Therefore the scenario we want to avoid is where auth_rpcgss.ko is loaded, but rpc.gssd is not running. There are two obvious ways to avoid this scenario:
> >>>
> >>> A. If auth_rpcgss.ko is loaded unconditionally, make sure rpc.gssd is running unconditionally (and cull the warnings)
> >>>
> >>> B. If you do not want to run rpc.gssd, make sure auth_rpcgss.ko is not loaded
> >> Isn't the case that nfsd will also loads auth_rpcgss. So if server was started we
> >> would be right back to the 15 sec delay?
> >
> > Only if NFSD wants to make GSS upcalls and rpc.gssd isn't running. Is that ever the case?
> Well the reason I asked was I started the server and did a
> lsmod | grep rpc
> which showed
>
> rpcsec_gss_krb5 31477 1
> auth_rpcgss 59369 3 nfsd,rpcsec_gss_krb5
> sunrpc 280214 47 nfs,nfsd,rpcsec_gss_krb5,auth_rpcgss,lockd,nfsv
>
> Now for me to do a rmmod auth_rpcgs, to test your theory, I had to do a
> rmmod nfsd... Maybe that's just another bug on how things are depended
> on each other
>
> BTW, when I did rmmod auth_rpcgs and did a mount... the mount still hung
> logging the
> RPC: AUTH_GSS upcall timed out.
> Please check user daemon is running.
> messages...

I seem to recall Chuck actually fixing a dependency of nfsd on gss
modules. Maybe with

a77c806fb9d0 "SUNRPC: Refactor nfsd4_do_encode_secinfo()

? That probably needs some prerequisites too. And then also maybe

ed9411a00464 NFSD: Simplify GSS flavor encoding in nfsd4_do_encode_secinfo()
676e4ebd5f2c NFSD: SECINFO doesn't handle unsupported pseudoflavors correctly

2013-11-20 21:20:53

by Steve Dickson

[permalink] [raw]
Subject: Re: [PATCH - nfs-utils] gssd: always reply to rpc-pipe requests from kernel.



On 13/11/13 20:07, NeilBrown wrote:
>
> Sometimes gssd will open a new rpc-pipe but never read requests from it
> or reply to them. This causes the kernel to wait forever for a reply.
>
> In particular, if a filesystem is mounted by IP, and the IP has no
> hostname recorded in /etc/hosts or DNS, then gssd will not listen to
> requests and the mount will hang indefinitely.
>
> The comment in process_clnt_dir() for the "fail_keep_client:" branch
> suggests that it is for the case where we couldn't open some
> subdirectories. However it is currently also taken if reverse DNS
> lookup fails (as well as some other lookup failures). Those failures
> should not be treated the same as failure-to-open directories.
>
> So this patch causes a failure from read_service_info() to *not* be
> reported by process_clnt_dir_files. This ensures that insert_clnt_poll()
> will be called and requests will be handled.
>
> In handle_gssd_upcall, the current error path (taken when the mech is
> not "krb5") does not reply to the upcall. This is wrong. A reply is
> always appropriate. The only replies which aren't treated as
> transient errors are EACCES and EKEYEXPIRED, so we return the former.
>
> If read_service_info() fails then ->servicename will be NULL which will
> cause process_krb5_upcall() (quite reasonably) to become confused. So
> in that case we don't even try to process the up-call but just reply
> with EACCES.
>
> As clp->servicename==NULL is no longer treated as fatal, it is not
> appropraite to use it to test if read_service_info() has been already
> called on a client. Instread test clp->prog.
>
> Finally, the error path of read_service_info() will close 'fd' if it
> isn't -1, so when we close it, we should set fd to -1.
>
> Signed-off-by: NeilBrown <[email protected]>
Committed (tag: nfs-utils-1-2-10-rc1)

steved.
>
> diff --git a/utils/gssd/gssd_proc.c b/utils/gssd/gssd_proc.c
> index b48d1637cd36..58c2a282524f 100644
> --- a/utils/gssd/gssd_proc.c
> +++ b/utils/gssd/gssd_proc.c
> @@ -256,6 +256,7 @@ read_service_info(char *info_file_name, char **servicename, char **servername,
> if ((nbytes = read(fd, buf, INFOBUFLEN)) == -1)
> goto fail;
> close(fd);
> + fd = -1;
> buf[nbytes] = '\0';
>
> numfields = sscanf(buf,"RPC server: %127s\n"
> @@ -403,11 +404,10 @@ process_clnt_dir_files(struct clnt_info * clp)
> return -1;
> snprintf(info_file_name, sizeof(info_file_name), "%s/info",
> clp->dirname);
> - if ((clp->servicename == NULL) &&
> - read_service_info(info_file_name, &clp->servicename,
> - &clp->servername, &clp->prog, &clp->vers,
> - &clp->protocol, (struct sockaddr *) &clp->addr))
> - return -1;
> + if (clp->prog == 0)
> + read_service_info(info_file_name, &clp->servicename,
> + &clp->servername, &clp->prog, &clp->vers,
> + &clp->protocol, (struct sockaddr *) &clp->addr);
> return 0;
> }
>
> @@ -1320,11 +1320,14 @@ handle_gssd_upcall(struct clnt_info *clp)
> }
> }
>
> - if (strcmp(mech, "krb5") == 0)
> + if (strcmp(mech, "krb5") == 0 && clp->servername)
> process_krb5_upcall(clp, uid, clp->gssd_fd, target, service);
> - else
> - printerr(0, "WARNING: handle_gssd_upcall: "
> - "received unknown gss mech '%s'\n", mech);
> + else {
> + if (clp->servername)
> + printerr(0, "WARNING: handle_gssd_upcall: "
> + "received unknown gss mech '%s'\n", mech);
> + do_error_downcall(clp->gssd_fd, uid, -EACCES);
> + }
>
> out:
> free(lbuf);
>

2013-11-13 03:46:43

by J. Bruce Fields

[permalink] [raw]
Subject: Re: [PATCH] Adding the nfs4_secure_mounts bool

On Wed, Nov 13, 2013 at 11:23:46AM +1100, NeilBrown wrote:
> On Tue, 12 Nov 2013 11:16:34 -0500 "J. Bruce Fields" <[email protected]>
> wrote:
>
> > On Tue, Nov 12, 2013 at 05:29:46AM +0000, Myklebust, Trond wrote:
> > >
> > > On Nov 12, 2013, at 0:11, NeilBrown <[email protected]> wrote:
> > >
> > > > On Mon, 11 Nov 2013 15:33:14 -0500 Chuck Lever <[email protected]> wrote:
> > > >
> > > >>
> > > >> On Nov 11, 2013, at 1:59 PM, Steve Dickson <[email protected]> wrote:
> > > >>
> > > >>> On 11/11/13 13:30, Chuck Lever wrote:
> > > >>>>
> > > >>>> On Nov 11, 2013, at 1:06 PM, Steve Dickson <[email protected]> wrote:
> > > >>>>
> > > >>>>>
> > > >>>>>
> > > >>>>> On 09/11/13 18:12, Myklebust, Trond wrote:
> > > >>>>>> One alternative to the above scheme, which I believe that I’ve
> > > >>>>>> suggested before, is to have a permanent entry in rpc_pipefs
> > > >>>>>> that rpc.gssd can open and that the kernel can use to detect
> > > >>>>>> that it is running. If we make it /var/lib/nfs/rpc_pipefs/gssd/clnt00/gssd,
> > > >>>>>> then AFAICS we don’t need to change nfs-utils at all, since all newer
> > > >>>>>> versions of rpc.gssd will try to open for read anything of the form
> > > >>>>>> /var/lib/nfs/rpc_pipefs/*/clntXX/gssd...
> > > >>>>>
> > > >>>>> After further review I am going going have to disagree with you on this.
> > > >>>>> Since all the context is cached on the initial mount the kernel
> > > >>>>> should be using the call_usermodehelper() to call up to rpc.gssd
> > > >>>>> to get the context, which means we could put this upcall noise
> > > >>>>> to bed... forever! :-)
> > > >>>>
> > > >>>> Ask Al Viro for his comments on whether the kernel should start
> > > >>>> gssd (either a daemon or a script). Hint: wear your kevlar underpants.
> > > >>> I was thinking gssd would become a the gssd-cmd command... Al does not
> > > >>> like the call_usermodehelper() interface?
> > > >>
> > > >> He doesn't have a problem with call_usermodehelper() in general. However, the kernel cannot guarantee security if it has to run a fixed command line. Go ask him to explain.
> > > >>
> > > >>
> > > >>>
> > > >>>>
> > > >>>> Have you tried Trond's approach yet?
> > > >>> Looking into it... But nothing is trivial in that code...
> > > >>>
> > > >>>>
> > > >>>>> I realize this is not going happen overnight, so I would still
> > > >>>>> like to propose my nfs4_secure_mounts bool patch as bridge
> > > >>>>> to the new call_usermodehelper() since its the cleanest
> > > >>>>> solution so far...
> > > >>>>>
> > > >>>>> Thoughts?
> > > >>>>
> > > >>>> We have workarounds already that work on every kernel since 3.8.
> > > >>>>
> > > >>> The one that logs 5 to 20 lines (depending on thins are setup or not)
> > > >>> per mount? That does work in some environments but no all. ;-)
> > > >>
> > > >> When does running rpc.gssd not work?
> > > >
> > > > Oohh ooh.. Pick me. Pick me!! I can answer that one.
> > > >
> > > > Running rpc.gssd does not work if you are mounting a filesystem using the IP
> > > > address of the server and that IP address doesn't have a matching hostname
> > > > anywhere that can be found:
> > > >
> > > > In a newly creating minimal kvm install without rpc.gssd running,
> > > > mount 10.0.2.2:/home /mnt
> > > >
> > > > sleeps for 15 seconds then succeeds.
> > > > If I start rpc.gssd, then the same command takes forever.
> > > >
> > > > strace of rpc.gssd shows that it complains about not being able to resolve
> > > > the host name and "ERROR: failed to read service info". Then it keeps the
> > > > pipes open but never sends any message on them, so the kernel just keeps on
> > > > waiting.
> > > >
> > > > If I change "fail_keep_client" to "fail_destroy_client", then it closes the
> > > > pipe and we get the 15 second timeout back.
> > > > If I change NI_NAMEREQD to 0, then the mount completes instantly. (of course
> > > > that make serious compromise security so it was just for testing).
> > > > (Adding an entry to /etc/hosts also gives instant success).
> > > >
> > > > I'm hoping that someone who understands this code will suggest something
> > > > clever so I don't have to dig through all of it ;-)
> > >
> > > rpc.gssd is supposed to do a downcall with a zero-length window and an error message in any situation where it cannot establish a GSS context. Normally, I’d expect an EACCES for the above scenario.
> > >
> > > IOW: that’s a blatant rpc.gssd bug. One that will also affect you when you're doing NFSv3 and add ‘sec=krb5’ to the mount options.
> >
> > Also why is gssd trying to do a DNS lookup in this case? This sounds
> > similar to what f9f5450f8f94 "Avoid DNS reverse resolution for server
> > names (take 3)" was trying to fix?
>
> It is quite possible that I misunderstand something. But this is my
> understanding.
>
> 1/ "mount" allows you to use either an IP address or a host name to mount a
> filesystem.
> 2/ gss requires a hostname to identify the server and find it's key (IP not
> sufficient).
> 3/ If you use a host name to mount a filesystem, then that exact same host
> name should be used by gssd to identify the server and its key.
> The above mentioned patch was trying to enforce this. The idea was to
> collect the name given to the 'mount', see if it looked like an IP address
> or a Server name. If the later, just use it. If the former, do a reverse
> lookup because an IP address is no use by itself for gss.
> Previously it would always do a reverse DNS lookup from the IP address
> that was determined from the server-name-or-IP-address.
> Unfortunately this patch was broken - got the test backwards.
> A follow-up patch fixed the test: c93e8d8eeafec3e32
>
> 4/ So the above patch was not intended to address the case of mount-by-IP
> address at all - and this is the case that is causing me problems.

OK, but it still seems dumb to even attempt the reverse lookup: the
lookup probably isn't secure, and the mount commandline should have a
name that we can match to a krb5 principal without needing any other
lookups.

So I'd think reasonable behavior in this case would be to just try the
IP address on the chance there's actually an nfs/x.y.z.w@REALM
principal. (Or just fail outright if kerberos doesn't allow principals
that look like that.)

--b.

2013-11-12 16:51:39

by Steve Dickson

[permalink] [raw]
Subject: Re: [PATCH] Adding the nfs4_secure_mounts bool



On 12/11/13 11:46, Chuck Lever wrote:
>
> On Nov 12, 2013, at 11:24 AM, Steve Dickson <[email protected]> wrote:
>
>>
>>
>> On 12/11/13 11:09, Chuck Lever wrote:
>>>> In the past, if admins want rpc.gssd in the mount path they had to configure it.
>>>>> Now we are silently adding, yet another, daemon to the mount path and if
>>>>> rpc.gssd starts falling on its face, I think it will be difficult to debug,
>>>>> since the daemon is not expected to be there...
>>> Our only real choice here is to fix gssd. Anything else is punting the problem down the road.
>>>
>> No. The last there was a daemon was involved in all NFS client mounts
>> (at least that I can remember) was when lockd was a user level daemon.
>> The main reason it was ported to the kernel was to get ride of the
>> bottle neck it caused... Now we adding similar bottle neck back??
>>
>> Architecturally, put a daemon in the direct NFS mount path just does
>> not make sense... IMHO...
>
> Don't be ridiculous. rpc.gssd is ALREADY in the direct mount path for all Kerberos mounts, and has been for years.
The key words being "Kerberos mounts"....

>
> Forget lease management security for a moment, and consider this: There is no possibility of moving forward with a secure NFS solution on Linux if we can't depend on rpc.gssd. Therefore, our only real choice if we want Kerberos to be a first class NFS feature on Linux is to make sure rpc.gssd works reliably.
>
> Last I checked, we are making a robust effort to harden Kerberos support for NFS. So I don't see any contradiction here.
>
> Now, specifically regarding when rpc.gssd is invoked for lease management security: it is invoked the first time each new server is contacted. If you mount the same server many times, there should be just one upcall.
>
> And, if auth_rpcgss.ko is not loaded, there will be no upcall. Ever.
Perfect!

steved.

>

2013-11-11 18:30:18

by Chuck Lever

[permalink] [raw]
Subject: Re: [PATCH] Adding the nfs4_secure_mounts bool


On Nov 11, 2013, at 1:06 PM, Steve Dickson <[email protected]> wrote:

>
>
> On 09/11/13 18:12, Myklebust, Trond wrote:
>> One alternative to the above scheme, which I believe that I?ve
>> suggested before, is to have a permanent entry in rpc_pipefs
>> that rpc.gssd can open and that the kernel can use to detect
>> that it is running. If we make it /var/lib/nfs/rpc_pipefs/gssd/clnt00/gssd,
>> then AFAICS we don?t need to change nfs-utils at all, since all newer
>> versions of rpc.gssd will try to open for read anything of the form
>> /var/lib/nfs/rpc_pipefs/*/clntXX/gssd...
>
> After further review I am going going have to disagree with you on this.
> Since all the context is cached on the initial mount the kernel
> should be using the call_usermodehelper() to call up to rpc.gssd
> to get the context, which means we could put this upcall noise
> to bed... forever! :-)

Ask Al Viro for his comments on whether the kernel should start gssd (either a daemon or a script). Hint: wear your kevlar underpants.

Have you tried Trond's approach yet?

> I realize this is not going happen overnight, so I would still
> like to propose my nfs4_secure_mounts bool patch as bridge
> to the new call_usermodehelper() since its the cleanest
> solution so far...
>
> Thoughts?

We have workarounds already that work on every kernel since 3.8.

--
Chuck Lever
chuck[dot]lever[at]oracle[dot]com





2013-11-13 01:26:58

by Myklebust, Trond

[permalink] [raw]
Subject: Re: [PATCH] Adding the nfs4_secure_mounts bool

On Wed, 2013-11-13 at 12:13 +-1100, NeilBrown wrote:
+AD4- On Wed, 13 Nov 2013 00:30:53 +-0000 +ACI-Myklebust, Trond+ACI-
+AD4- +ADw-Trond.Myklebust+AEA-netapp.com+AD4- wrote:
+AD4-
+AD4- +AD4- On Wed, 2013-11-13 at 11:23 +-1100, NeilBrown wrote:
+AD4- +AD4- +AD4- But back to my problem: Following Trond's suggestion I've come up with the
+AD4- +AD4- +AD4- following patch. Does it look right?
+AD4- +AD4- +AD4-
+AD4- +AD4- +AD4- The +ACI-fd +AD0- -1+ACI- is just to stop us trying to close a non-open fd in an error
+AD4- +AD4- +AD4- path.
+AD4- +AD4- +AD4-
+AD4- +AD4- +AD4- The change from testing -+AD4-servicename to -+AD4-prog stops us from repeating the
+AD4- +AD4- +AD4- failed DNS lookup on every request, not that the failure isn't fatal.
+AD4- +AD4- +AD4-
+AD4- +AD4- +AD4- The last stanza makes sure we always reply to an upcall, with EINVAL if
+AD4- +AD4- +AD4- nothing else seems appropriate.
+AD4- +AD4-
+AD4- +AD4- Wouldn't EACCES be more appropriate as a default?
+AD4- +AD4-
+AD4-
+AD4- Maybe. And that is what you suggested before and I mis-remembered - sorry.
+AD4-
+AD4- However EACCES is +ACI-Permission denied+ACI- which doesn't quite seem right to me.
+AD4- It isn't really +ACI-you aren't allowed to do that+ACI-, but +ACI-your question doesn't
+AD4- make sense+ACI-.
+AD4-
+AD4- However I'm not fussed. If you prefer EACCES, then I'll make it EACCES.

If you look at gss+AF8-pipe+AF8-downcall(), then you'll note that it treats
EINVAL as a temporary error, and converts it to EAGAIN. That again
causes call+AF8-refreshresult to retry the upcall 2 more times before
failing with EACCES anyway...

--
Trond Myklebust
Linux NFS client maintainer

NetApp
Trond.Myklebust+AEA-netapp.com
http://www.netapp.com

2013-11-12 16:23:41

by Steve Dickson

[permalink] [raw]
Subject: Re: [PATCH] Adding the nfs4_secure_mounts bool



On 12/11/13 11:09, Chuck Lever wrote:
>> In the past, if admins want rpc.gssd in the mount path they had to configure it.
>> > Now we are silently adding, yet another, daemon to the mount path and if
>> > rpc.gssd starts falling on its face, I think it will be difficult to debug,
>> > since the daemon is not expected to be there...
> Our only real choice here is to fix gssd. Anything else is punting the problem down the road.
>
No. The last there was a daemon was involved in all NFS client mounts
(at least that I can remember) was when lockd was a user level daemon.
The main reason it was ported to the kernel was to get ride of the
bottle neck it caused... Now we adding similar bottle neck back??

Architecturally, put a daemon in the direct NFS mount path just does
not make sense... IMHO...

steved.

2013-11-11 18:53:15

by Myklebust, Trond

[permalink] [raw]
Subject: Re: [PATCH] Adding the nfs4_secure_mounts bool


On Nov 11, 2013, at 13:43, Steve Dickson <[email protected]> wrote:

>
>
> On 11/11/13 13:25, Myklebust, Trond wrote:
>>
>> On Nov 11, 2013, at 13:06, Steve Dickson <[email protected]> wrote:
>>
>>>
>>>
>>> On 09/11/13 18:12, Myklebust, Trond wrote:
>>>> One alternative to the above scheme, which I believe that I?ve
>>>> suggested before, is to have a permanent entry in rpc_pipefs
>>>> that rpc.gssd can open and that the kernel can use to detect
>>>> that it is running. If we make it /var/lib/nfs/rpc_pipefs/gssd/clnt00/gssd,
>>>> then AFAICS we don?t need to change nfs-utils at all, since all newer
>>>> versions of rpc.gssd will try to open for read anything of the form
>>>> /var/lib/nfs/rpc_pipefs/*/clntXX/gssd...
>>>
>>> After further review I am going going have to disagree with you on this.
>>> Since all the context is cached on the initial mount the kernel
>>
>> What context?
> The krb5 blob that the kernel is call up to rpc.gssd to get.. Maybe
> I'm using the wrong terminology???

That?s only the machine cred. User credentials get allocated and freed all the time.

When the server reboots, then all GSS contexts need to be re-established, which can be a lot of call_usermodehelper() upcalls; that?s one of the reasons why we decided in favour of a gssd daemon in the first place.


2013-11-14 13:34:20

by Jeff Layton

[permalink] [raw]
Subject: Re: [PATCH - nfs-utils] gssd: always reply to rpc-pipe requests from kernel.

On Thu, 14 Nov 2013 12:07:11 +1100
NeilBrown <[email protected]> wrote:

>
> Sometimes gssd will open a new rpc-pipe but never read requests from it
> or reply to them. This causes the kernel to wait forever for a reply.
>
> In particular, if a filesystem is mounted by IP, and the IP has no
> hostname recorded in /etc/hosts or DNS, then gssd will not listen to
> requests and the mount will hang indefinitely.
>
> The comment in process_clnt_dir() for the "fail_keep_client:" branch
> suggests that it is for the case where we couldn't open some
> subdirectories. However it is currently also taken if reverse DNS
> lookup fails (as well as some other lookup failures). Those failures
> should not be treated the same as failure-to-open directories.
>
> So this patch causes a failure from read_service_info() to *not* be
> reported by process_clnt_dir_files. This ensures that insert_clnt_poll()
> will be called and requests will be handled.
>
> In handle_gssd_upcall, the current error path (taken when the mech is
> not "krb5") does not reply to the upcall. This is wrong. A reply is
> always appropriate. The only replies which aren't treated as
> transient errors are EACCES and EKEYEXPIRED, so we return the former.
>
> If read_service_info() fails then ->servicename will be NULL which will
> cause process_krb5_upcall() (quite reasonably) to become confused. So
> in that case we don't even try to process the up-call but just reply
> with EACCES.
>
> As clp->servicename==NULL is no longer treated as fatal, it is not
> appropraite to use it to test if read_service_info() has been already
> called on a client. Instread test clp->prog.
>
> Finally, the error path of read_service_info() will close 'fd' if it
> isn't -1, so when we close it, we should set fd to -1.
>
> Signed-off-by: NeilBrown <[email protected]>
>
> diff --git a/utils/gssd/gssd_proc.c b/utils/gssd/gssd_proc.c
> index b48d1637cd36..58c2a282524f 100644
> --- a/utils/gssd/gssd_proc.c
> +++ b/utils/gssd/gssd_proc.c
> @@ -256,6 +256,7 @@ read_service_info(char *info_file_name, char **servicename, char **servername,
> if ((nbytes = read(fd, buf, INFOBUFLEN)) == -1)
> goto fail;
> close(fd);
> + fd = -1;
> buf[nbytes] = '\0';
>
> numfields = sscanf(buf,"RPC server: %127s\n"
> @@ -403,11 +404,10 @@ process_clnt_dir_files(struct clnt_info * clp)
> return -1;
> snprintf(info_file_name, sizeof(info_file_name), "%s/info",
> clp->dirname);
> - if ((clp->servicename == NULL) &&
> - read_service_info(info_file_name, &clp->servicename,
> - &clp->servername, &clp->prog, &clp->vers,
> - &clp->protocol, (struct sockaddr *) &clp->addr))
> - return -1;
> + if (clp->prog == 0)
> + read_service_info(info_file_name, &clp->servicename,
> + &clp->servername, &clp->prog, &clp->vers,
> + &clp->protocol, (struct sockaddr *) &clp->addr);
> return 0;
> }
>
> @@ -1320,11 +1320,14 @@ handle_gssd_upcall(struct clnt_info *clp)
> }
> }
>
> - if (strcmp(mech, "krb5") == 0)
> + if (strcmp(mech, "krb5") == 0 && clp->servername)
> process_krb5_upcall(clp, uid, clp->gssd_fd, target, service);
> - else
> - printerr(0, "WARNING: handle_gssd_upcall: "
> - "received unknown gss mech '%s'\n", mech);
> + else {
> + if (clp->servername)
> + printerr(0, "WARNING: handle_gssd_upcall: "
> + "received unknown gss mech '%s'\n", mech);
> + do_error_downcall(clp->gssd_fd, uid, -EACCES);
> + }
>
> out:
> free(lbuf);

Looks good.

Acked-by: Jeff Layton <[email protected]>

2013-11-14 01:07:24

by NeilBrown

[permalink] [raw]
Subject: [PATCH - nfs-utils] gssd: always reply to rpc-pipe requests from kernel.


Sometimes gssd will open a new rpc-pipe but never read requests from it
or reply to them. This causes the kernel to wait forever for a reply.

In particular, if a filesystem is mounted by IP, and the IP has no
hostname recorded in /etc/hosts or DNS, then gssd will not listen to
requests and the mount will hang indefinitely.

The comment in process_clnt_dir() for the "fail_keep_client:" branch
suggests that it is for the case where we couldn't open some
subdirectories. However it is currently also taken if reverse DNS
lookup fails (as well as some other lookup failures). Those failures
should not be treated the same as failure-to-open directories.

So this patch causes a failure from read_service_info() to *not* be
reported by process_clnt_dir_files. This ensures that insert_clnt_poll()
will be called and requests will be handled.

In handle_gssd_upcall, the current error path (taken when the mech is
not "krb5") does not reply to the upcall. This is wrong. A reply is
always appropriate. The only replies which aren't treated as
transient errors are EACCES and EKEYEXPIRED, so we return the former.

If read_service_info() fails then ->servicename will be NULL which will
cause process_krb5_upcall() (quite reasonably) to become confused. So
in that case we don't even try to process the up-call but just reply
with EACCES.

As clp->servicename==NULL is no longer treated as fatal, it is not
appropraite to use it to test if read_service_info() has been already
called on a client. Instread test clp->prog.

Finally, the error path of read_service_info() will close 'fd' if it
isn't -1, so when we close it, we should set fd to -1.

Signed-off-by: NeilBrown <[email protected]>

diff --git a/utils/gssd/gssd_proc.c b/utils/gssd/gssd_proc.c
index b48d1637cd36..58c2a282524f 100644
--- a/utils/gssd/gssd_proc.c
+++ b/utils/gssd/gssd_proc.c
@@ -256,6 +256,7 @@ read_service_info(char *info_file_name, char **servicename, char **servername,
if ((nbytes = read(fd, buf, INFOBUFLEN)) == -1)
goto fail;
close(fd);
+ fd = -1;
buf[nbytes] = '\0';

numfields = sscanf(buf,"RPC server: %127s\n"
@@ -403,11 +404,10 @@ process_clnt_dir_files(struct clnt_info * clp)
return -1;
snprintf(info_file_name, sizeof(info_file_name), "%s/info",
clp->dirname);
- if ((clp->servicename == NULL) &&
- read_service_info(info_file_name, &clp->servicename,
- &clp->servername, &clp->prog, &clp->vers,
- &clp->protocol, (struct sockaddr *) &clp->addr))
- return -1;
+ if (clp->prog == 0)
+ read_service_info(info_file_name, &clp->servicename,
+ &clp->servername, &clp->prog, &clp->vers,
+ &clp->protocol, (struct sockaddr *) &clp->addr);
return 0;
}

@@ -1320,11 +1320,14 @@ handle_gssd_upcall(struct clnt_info *clp)
}
}

- if (strcmp(mech, "krb5") == 0)
+ if (strcmp(mech, "krb5") == 0 && clp->servername)
process_krb5_upcall(clp, uid, clp->gssd_fd, target, service);
- else
- printerr(0, "WARNING: handle_gssd_upcall: "
- "received unknown gss mech '%s'\n", mech);
+ else {
+ if (clp->servername)
+ printerr(0, "WARNING: handle_gssd_upcall: "
+ "received unknown gss mech '%s'\n", mech);
+ do_error_downcall(clp->gssd_fd, uid, -EACCES);
+ }

out:
free(lbuf);


Attachments:
signature.asc (828.00 B)

2013-11-09 23:12:49

by Myklebust, Trond

[permalink] [raw]
Subject: Re: [PATCH] Adding the nfs4_secure_mounts bool


On Nov 9, 2013, at 17:47, Steve Dickson <[email protected]> wrote:

> The nfs4_secure_mounts controls whether security
> flavors will be tried during the establishing of
> NFSv4 state and the pseudoroot lookups.
>
> This allows security daemon like rpc.gssd to
> tell the kernel that secure mounts should be tried.
>
> To enable secure mounts:
> echo "on" > /proc/fs/nfsfs/secure
>
> To disable secure mounts:
> echo "off" > /proc/fs/nfsfs/secure
>
> Signed-off-by: Steve Dickson <[email protected]>

Hi Steve,

So the rpc.gssd would flip the switch to ?on? when it starts up and to ?off? when it quits? What if someone does a ?kill -9??

One alternative to the above scheme, which I believe that I?ve suggested before, is to have a permanent entry in rpc_pipefs that rpc.gssd can open and that the kernel can use to detect that it is running. If we make it /var/lib/nfs/rpc_pipefs/gssd/clnt00/gssd, then AFAICS we don?t need to change nfs-utils at all, since all newer versions of rpc.gssd will try to open for read anything of the form /var/lib/nfs/rpc_pipefs/*/clntXX/gssd...

Cheers
Trond