[PATCH] selinux: allow kernfs symlinks to inherit parent directory context

[PATCH] selinux: allow kernfs symlinks to inherit parent directory context

[Christian Göttsche]

Currently symlinks on kernel filesystems, like sysfs, are labeled on
creation with the parent fs root sid.

Allow symlinks to inherit the parent directory context, so fine-grained
kernfs labeling can be applied to symlinks too and checking contexts
doesn't complain about them.

For backward-compatibility this behavior is contained in a new policy
capability: kernfs_sovereign_symlinks

Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
---
 security/selinux/hooks.c            | 5 ++++-
 security/selinux/include/security.h | 8 ++++++++
 security/selinux/ss/services.c      | 3 ++-
 3 files changed, 14 insertions(+), 2 deletions(-)

diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index d9e8b2131..1303bc8c4 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -1475,7 +1475,10 @@ static int inode_doinit_with_dentry(struct inode *inode, struct dentry *opt_dent
 		/* Default to the fs superblock SID. */
 		sid = sbsec->sid;
 
-		if ((sbsec->flags & SE_SBGENFS) && !S_ISLNK(inode->i_mode)) {
+		if (((sbsec->flags & SE_SBGENFS) &&
+		     (!S_ISLNK(inode->i_mode))) ||
+		    (selinux_policycap_kernfs_sovereign_symlinks() &&
+		     (sbsec->flags & SE_SBGENFS_XATTR))) {
 			/* We must have a dentry to determine the label on
 			 * procfs inodes */
 			if (opt_dentry) {
diff --git a/security/selinux/include/security.h b/security/selinux/include/security.h
index a39f9565d..cc8217848 100644
--- a/security/selinux/include/security.h
+++ b/security/selinux/include/security.h
@@ -79,6 +79,7 @@ enum {
 	POLICYDB_CAPABILITY_ALWAYSNETWORK,
 	POLICYDB_CAPABILITY_CGROUPSECLABEL,
 	POLICYDB_CAPABILITY_NNP_NOSUID_TRANSITION,
+	POLICYDB_CAPABILITY_KERNFS_SOVEREIGN_SYMLINKS,
 	__POLICYDB_CAPABILITY_MAX
 };
 #define POLICYDB_CAPABILITY_MAX (__POLICYDB_CAPABILITY_MAX - 1)
@@ -209,6 +210,13 @@ static inline bool selinux_policycap_nnp_nosuid_transition(void)
 	return state->policycap[POLICYDB_CAPABILITY_NNP_NOSUID_TRANSITION];
 }
 
+static inline bool selinux_policycap_kernfs_sovereign_symlinks(void)
+{
+	struct selinux_state *state = &selinux_state;
+
+	return state->policycap[POLICYDB_CAPABILITY_KERNFS_SOVEREIGN_SYMLINKS];
+}
+
 int security_mls_enabled(struct selinux_state *state);
 int security_load_policy(struct selinux_state *state,
 			 void *data, size_t len);
diff --git a/security/selinux/ss/services.c b/security/selinux/ss/services.c
index 216ce602a..b70380947 100644
--- a/security/selinux/ss/services.c
+++ b/security/selinux/ss/services.c
@@ -73,7 +73,8 @@ const char *selinux_policycap_names[__POLICYDB_CAPABILITY_MAX] = {
 	"extended_socket_class",
 	"always_check_network",
 	"cgroup_seclabel",
-	"nnp_nosuid_transition"
+	"nnp_nosuid_transition",
+	"kernfs_sovereign_symlinks"
 };
 
 static struct selinux_ss selinux_ss;
-- 
2.25.0

Re: [PATCH] selinux: allow kernfs symlinks to inherit parent directory context

[Stephen Smalley]

On 1/24/20 1:42 PM, Christian Göttsche wrote:
> Currently symlinks on kernel filesystems, like sysfs, are labeled on
> creation with the parent fs root sid.
> 
> Allow symlinks to inherit the parent directory context, so fine-grained
> kernfs labeling can be applied to symlinks too and checking contexts
> doesn't complain about them.
> 
> For backward-compatibility this behavior is contained in a new policy
> capability: kernfs_sovereign_symlinks
> 
> Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
> ---
>   security/selinux/hooks.c            | 5 ++++-
>   security/selinux/include/security.h | 8 ++++++++
>   security/selinux/ss/services.c      | 3 ++-
>   3 files changed, 14 insertions(+), 2 deletions(-)
> 
> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> index d9e8b2131..1303bc8c4 100644
> --- a/security/selinux/hooks.c
> +++ b/security/selinux/hooks.c
> @@ -1475,7 +1475,10 @@ static int inode_doinit_with_dentry(struct inode *inode, struct dentry *opt_dent
>   		/* Default to the fs superblock SID. */
>   		sid = sbsec->sid;
>   
> -		if ((sbsec->flags & SE_SBGENFS) && !S_ISLNK(inode->i_mode)) {
> +		if (((sbsec->flags & SE_SBGENFS) &&
> +		     (!S_ISLNK(inode->i_mode))) ||
> +		    (selinux_policycap_kernfs_sovereign_symlinks() &&

Not fond of the name.  1) kernfs is a kernel implementation detail, 
shouldn't be exposed to policy; genfs is the policy construct 2) 
sovereign doesn't seem to fit the meaning of this capability; seclabel 
would be more appropriate.

> +		     (sbsec->flags & SE_SBGENFS_XATTR))) {

Why limit this to SE_SBGENFS_XATTR filesystems?  Why not just make the test:
	if ((sbsec->flags & SE_SBGENFS) && (!S_ISLNK(inode->i_mode) || 
selinux_policycap_genfs_symlinkseclabel()))
or similar.

>   			/* We must have a dentry to determine the label on
>   			 * procfs inodes */
>   			if (opt_dentry) {
> diff --git a/security/selinux/include/security.h b/security/selinux/include/security.h
> index a39f9565d..cc8217848 100644
> --- a/security/selinux/include/security.h
> +++ b/security/selinux/include/security.h
> @@ -79,6 +79,7 @@ enum {
>   	POLICYDB_CAPABILITY_ALWAYSNETWORK,
>   	POLICYDB_CAPABILITY_CGROUPSECLABEL,
>   	POLICYDB_CAPABILITY_NNP_NOSUID_TRANSITION,
> +	POLICYDB_CAPABILITY_KERNFS_SOVEREIGN_SYMLINKS,
>   	__POLICYDB_CAPABILITY_MAX
>   };
>   #define POLICYDB_CAPABILITY_MAX (__POLICYDB_CAPABILITY_MAX - 1)
> @@ -209,6 +210,13 @@ static inline bool selinux_policycap_nnp_nosuid_transition(void)
>   	return state->policycap[POLICYDB_CAPABILITY_NNP_NOSUID_TRANSITION];
>   }
>   
> +static inline bool selinux_policycap_kernfs_sovereign_symlinks(void)
> +{
> +	struct selinux_state *state = &selinux_state;
> +
> +	return state->policycap[POLICYDB_CAPABILITY_KERNFS_SOVEREIGN_SYMLINKS];
> +}
> +
>   int security_mls_enabled(struct selinux_state *state);
>   int security_load_policy(struct selinux_state *state,
>   			 void *data, size_t len);
> diff --git a/security/selinux/ss/services.c b/security/selinux/ss/services.c
> index 216ce602a..b70380947 100644
> --- a/security/selinux/ss/services.c
> +++ b/security/selinux/ss/services.c
> @@ -73,7 +73,8 @@ const char *selinux_policycap_names[__POLICYDB_CAPABILITY_MAX] = {
>   	"extended_socket_class",
>   	"always_check_network",
>   	"cgroup_seclabel",
> -	"nnp_nosuid_transition"
> +	"nnp_nosuid_transition",
> +	"kernfs_sovereign_symlinks"
>   };
>   
>   static struct selinux_ss selinux_ss;
> 

Re: [PATCH] selinux: allow kernfs symlinks to inherit parent directory context

[Christian Göttsche]

Am Fr., 24. Jan. 2020 um 19:53 Uhr schrieb Stephen Smalley <sds@tycho.nsa.gov>:
>
> On 1/24/20 1:42 PM, Christian Göttsche wrote:
> > Currently symlinks on kernel filesystems, like sysfs, are labeled on
> > creation with the parent fs root sid.
> >
> > Allow symlinks to inherit the parent directory context, so fine-grained
> > kernfs labeling can be applied to symlinks too and checking contexts
> > doesn't complain about them.
> >
> > For backward-compatibility this behavior is contained in a new policy
> > capability: kernfs_sovereign_symlinks
> >
> > Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
> > ---
> >   security/selinux/hooks.c            | 5 ++++-
> >   security/selinux/include/security.h | 8 ++++++++
> >   security/selinux/ss/services.c      | 3 ++-
> >   3 files changed, 14 insertions(+), 2 deletions(-)
> >
> > diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> > index d9e8b2131..1303bc8c4 100644
> > --- a/security/selinux/hooks.c
> > +++ b/security/selinux/hooks.c
> > @@ -1475,7 +1475,10 @@ static int inode_doinit_with_dentry(struct inode *inode, struct dentry *opt_dent
> >               /* Default to the fs superblock SID. */
> >               sid = sbsec->sid;
> >
> > -             if ((sbsec->flags & SE_SBGENFS) && !S_ISLNK(inode->i_mode)) {
> > +             if (((sbsec->flags & SE_SBGENFS) &&
> > +                  (!S_ISLNK(inode->i_mode))) ||
> > +                 (selinux_policycap_kernfs_sovereign_symlinks() &&
>
> Not fond of the name.  1) kernfs is a kernel implementation detail,
> shouldn't be exposed to policy; genfs is the policy construct 2)
> sovereign doesn't seem to fit the meaning of this capability; seclabel
> would be more appropriate.

Something like genfs_seclabel_symlinks?

> > +                  (sbsec->flags & SE_SBGENFS_XATTR))) {
>
> Why limit this to SE_SBGENFS_XATTR filesystems?  Why not just make the test:
>         if ((sbsec->flags & SE_SBGENFS) && (!S_ISLNK(inode->i_mode) ||
> selinux_policycap_genfs_symlinkseclabel()))
> or similar.

I somehow thought that this functionality is limited to filesystems
with SE_SBGENFS_XATTR;
so I can expand the check to SE_SBGENFS.

> >                       /* We must have a dentry to determine the label on
> >                        * procfs inodes */
> >                       if (opt_dentry) {
> > diff --git a/security/selinux/include/security.h b/security/selinux/include/security.h
> > index a39f9565d..cc8217848 100644
> > --- a/security/selinux/include/security.h
> > +++ b/security/selinux/include/security.h
> > @@ -79,6 +79,7 @@ enum {
> >       POLICYDB_CAPABILITY_ALWAYSNETWORK,
> >       POLICYDB_CAPABILITY_CGROUPSECLABEL,
> >       POLICYDB_CAPABILITY_NNP_NOSUID_TRANSITION,
> > +     POLICYDB_CAPABILITY_KERNFS_SOVEREIGN_SYMLINKS,
> >       __POLICYDB_CAPABILITY_MAX
> >   };
> >   #define POLICYDB_CAPABILITY_MAX (__POLICYDB_CAPABILITY_MAX - 1)
> > @@ -209,6 +210,13 @@ static inline bool selinux_policycap_nnp_nosuid_transition(void)
> >       return state->policycap[POLICYDB_CAPABILITY_NNP_NOSUID_TRANSITION];
> >   }
> >
> > +static inline bool selinux_policycap_kernfs_sovereign_symlinks(void)
> > +{
> > +     struct selinux_state *state = &selinux_state;
> > +
> > +     return state->policycap[POLICYDB_CAPABILITY_KERNFS_SOVEREIGN_SYMLINKS];
> > +}
> > +
> >   int security_mls_enabled(struct selinux_state *state);
> >   int security_load_policy(struct selinux_state *state,
> >                        void *data, size_t len);
> > diff --git a/security/selinux/ss/services.c b/security/selinux/ss/services.c
> > index 216ce602a..b70380947 100644
> > --- a/security/selinux/ss/services.c
> > +++ b/security/selinux/ss/services.c
> > @@ -73,7 +73,8 @@ const char *selinux_policycap_names[__POLICYDB_CAPABILITY_MAX] = {
> >       "extended_socket_class",
> >       "always_check_network",
> >       "cgroup_seclabel",
> > -     "nnp_nosuid_transition"
> > +     "nnp_nosuid_transition",
> > +     "kernfs_sovereign_symlinks"
> >   };
> >
> >   static struct selinux_ss selinux_ss;
> >
>

Re: [PATCH] selinux: allow kernfs symlinks to inherit parent directory context

[Stephen Smalley]

On 1/24/20 2:08 PM, Christian Göttsche wrote:
> Am Fr., 24. Jan. 2020 um 19:53 Uhr schrieb Stephen Smalley <sds@tycho.nsa.gov>:
>>
>> On 1/24/20 1:42 PM, Christian Göttsche wrote:
>>> Currently symlinks on kernel filesystems, like sysfs, are labeled on
>>> creation with the parent fs root sid.
>>>
>>> Allow symlinks to inherit the parent directory context, so fine-grained
>>> kernfs labeling can be applied to symlinks too and checking contexts
>>> doesn't complain about them.
>>>
>>> For backward-compatibility this behavior is contained in a new policy
>>> capability: kernfs_sovereign_symlinks
>>>
>>> Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
>>> ---

>> Not fond of the name.  1) kernfs is a kernel implementation detail,
>> shouldn't be exposed to policy; genfs is the policy construct 2)
>> sovereign doesn't seem to fit the meaning of this capability; seclabel
>> would be more appropriate.
> 
> Something like genfs_seclabel_symlinks?

Works for me.

> 
>>> +                  (sbsec->flags & SE_SBGENFS_XATTR))) {
>>
>> Why limit this to SE_SBGENFS_XATTR filesystems?  Why not just make the test:
>>          if ((sbsec->flags & SE_SBGENFS) && (!S_ISLNK(inode->i_mode) ||
>> selinux_policycap_genfs_symlinkseclabel()))
>> or similar.
> 
> I somehow thought that this functionality is limited to filesystems
> with SE_SBGENFS_XATTR;
> so I can expand the check to SE_SBGENFS.

I could be wrong but I don't see why it would need to be limited in that 
way.

[PATCH v2] selinux: allow kernfs symlinks to inherit parent directory context

[Christian Göttsche]

Currently symlinks on kernel filesystems, like sysfs, are labeled on
creation with the parent filesystem root sid.

Allow symlinks to inherit the parent directory context, so fine-grained
kernfs labeling can be applied to symlinks too and checking contexts
doesn't complain about them.

For backward-compatibility this behavior is contained in a new policy
capability: genfs_seclabel_symlinks

Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
---
v2:
incorporate feedback from Stephen Smalley
  - changed polcap name
  - extended affected filesystems from SE_SBGENFS_XATTR to SE_SBGENFS

 security/selinux/hooks.c            | 4 +++-
 security/selinux/include/security.h | 8 ++++++++
 security/selinux/ss/services.c      | 3 ++-
 3 files changed, 13 insertions(+), 2 deletions(-)

diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index d9e8b2131a65..60a0b3553c70 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -1475,7 +1475,9 @@ static int inode_doinit_with_dentry(struct inode *inode, struct dentry *opt_dent
 		/* Default to the fs superblock SID. */
 		sid = sbsec->sid;
 
-		if ((sbsec->flags & SE_SBGENFS) && !S_ISLNK(inode->i_mode)) {
+		if ((sbsec->flags & SE_SBGENFS) &&
+		     (!S_ISLNK(inode->i_mode) ||
+		      selinux_policycap_genfs_seclabel_symlinks())) {
 			/* We must have a dentry to determine the label on
 			 * procfs inodes */
 			if (opt_dentry) {
diff --git a/security/selinux/include/security.h b/security/selinux/include/security.h
index a39f9565d80b..863ccf2bb629 100644
--- a/security/selinux/include/security.h
+++ b/security/selinux/include/security.h
@@ -79,6 +79,7 @@ enum {
 	POLICYDB_CAPABILITY_ALWAYSNETWORK,
 	POLICYDB_CAPABILITY_CGROUPSECLABEL,
 	POLICYDB_CAPABILITY_NNP_NOSUID_TRANSITION,
+	POLICYDB_CAPABILITY_GENFS_SECLABEL_SYMLINKS,
 	__POLICYDB_CAPABILITY_MAX
 };
 #define POLICYDB_CAPABILITY_MAX (__POLICYDB_CAPABILITY_MAX - 1)
@@ -209,6 +210,13 @@ static inline bool selinux_policycap_nnp_nosuid_transition(void)
 	return state->policycap[POLICYDB_CAPABILITY_NNP_NOSUID_TRANSITION];
 }
 
+static inline bool selinux_policycap_genfs_seclabel_symlinks(void)
+{
+	struct selinux_state *state = &selinux_state;
+
+	return state->policycap[POLICYDB_CAPABILITY_GENFS_SECLABEL_SYMLINKS];
+}
+
 int security_mls_enabled(struct selinux_state *state);
 int security_load_policy(struct selinux_state *state,
 			 void *data, size_t len);
diff --git a/security/selinux/ss/services.c b/security/selinux/ss/services.c
index 216ce602a2b5..d9306f489060 100644
--- a/security/selinux/ss/services.c
+++ b/security/selinux/ss/services.c
@@ -73,7 +73,8 @@ const char *selinux_policycap_names[__POLICYDB_CAPABILITY_MAX] = {
 	"extended_socket_class",
 	"always_check_network",
 	"cgroup_seclabel",
-	"nnp_nosuid_transition"
+	"nnp_nosuid_transition",
+	"genfs_seclabel_symlinks"
 };
 
 static struct selinux_ss selinux_ss;
-- 
2.25.0

Re: [PATCH v2] selinux: allow kernfs symlinks to inherit parent directory context

[Stephen Smalley]

On 1/28/20 2:16 PM, Christian Göttsche wrote:
> Currently symlinks on kernel filesystems, like sysfs, are labeled on
> creation with the parent filesystem root sid.
> 
> Allow symlinks to inherit the parent directory context, so fine-grained
> kernfs labeling can be applied to symlinks too and checking contexts
> doesn't complain about them.
> 
> For backward-compatibility this behavior is contained in a new policy
> capability: genfs_seclabel_symlinks
> 
> Signed-off-by: Christian Göttsche <cgzones@googlemail.com>

(pruned cc list to omit non-selinux kernel maintainers since this change 
is entirely self-contained to selinux; I'm guessing you blindly took the 
results of scripts/get_maintainer.pl which tends to over-approximate - I 
only use it as a guide/hint and rarely use the full list it provides.)

This looks fine to me code-wise.  Have you tried enabling this new 
policy capability in policy and seeing the effects of it?  I remember a 
problem in the way-back time that motivated adding the S_ISLNK() 
exception for proc.  IIRC, the issue was that policies specified 
"genfscon proc /net system_u:object_r:proc_net_t:s0" to label everything 
under /proc/net with proc_net_t by default, and when /proc/net was 
changed to be a symlink to /proc/self/net as part of the network 
namespaces work, this caused the symlink to be labeled proc_net_t, but 
since previously there were no symlinks labeled proc_net_t, no processes 
were allowed to read/follow the symlink and existing systems broke. 
Exempting symlinks caused the /proc/net symlink to be labeled proc_t 
instead, which was already widely allowed.  To avoid this problem when 
enabling this capability, you will either need to allow 
proc_net_t:lnk_file read widely or you will need to change the genfscon 
statement to avoid labeling the symlink (there is an optional file mode 
field in genfscon statements similar to that of file_contexts, e.g.
  genfscon proc /net -d system_u:object_r:proc_net_t:s0
  genfscon proc /net -- system_u:object_r:proc_net_t:s0
would only label directories and regular files with proc_net_t.

Acked-by: Stephen Smalley <sds@tycho.nsa.gov>

[...]

Re: [PATCH v2] selinux: allow kernfs symlinks to inherit parent directory context

[Christian Göttsche]

Am Mi., 29. Jan. 2020 um 14:34 Uhr schrieb Stephen Smalley <sds@tycho.nsa.gov>:
> This looks fine to me code-wise.  Have you tried enabling this new
> policy capability in policy and seeing the effects of it?  I remember a
> problem in the way-back time that motivated adding the S_ISLNK()
> exception for proc.  IIRC, the issue was that policies specified
> "genfscon proc /net system_u:object_r:proc_net_t:s0" to label everything
> under /proc/net with proc_net_t by default, and when /proc/net was
> changed to be a symlink to /proc/self/net as part of the network
> namespaces work, this caused the symlink to be labeled proc_net_t, but
> since previously there were no symlinks labeled proc_net_t, no processes
> were allowed to read/follow the symlink and existing systems broke.
> Exempting symlinks caused the /proc/net symlink to be labeled proc_t
> instead, which was already widely allowed.  To avoid this problem when
> enabling this capability, you will either need to allow
> proc_net_t:lnk_file read widely or you will need to change the genfscon
> statement to avoid labeling the symlink (there is an optional file mode
> field in genfscon statements similar to that of file_contexts, e.g.
>   genfscon proc /net -d system_u:object_r:proc_net_t:s0
>   genfscon proc /net -- system_u:object_r:proc_net_t:s0
> would only label directories and regular files with proc_net_t.

I tested it with a refpolicy alike policy, where
kernel_read_network_state() allows access to symlinks [1].
Just systemd pid 1 wants to read the symlink only without reading any
files, so I added kernel_search_network_state(systemd_t).

[1]: https://github.com/SELinuxProject/refpolicy/blob/7e191b008e70ca535ceca2405967ddddd2ed975d/policy/modules/kernel/kernel.if#L1437

Re: [PATCH v2] selinux: allow kernfs symlinks to inherit parent directory context

[Paul Moore]

On Wed, Jan 29, 2020 at 11:45 AM Christian Göttsche
<cgzones@googlemail.com> wrote:
> Am Mi., 29. Jan. 2020 um 14:34 Uhr schrieb Stephen Smalley <sds@tycho.nsa.gov>:
> > This looks fine to me code-wise.  Have you tried enabling this new
> > policy capability in policy and seeing the effects of it?  I remember a
> > problem in the way-back time that motivated adding the S_ISLNK()
> > exception for proc.  IIRC, the issue was that policies specified
> > "genfscon proc /net system_u:object_r:proc_net_t:s0" to label everything
> > under /proc/net with proc_net_t by default, and when /proc/net was
> > changed to be a symlink to /proc/self/net as part of the network
> > namespaces work, this caused the symlink to be labeled proc_net_t, but
> > since previously there were no symlinks labeled proc_net_t, no processes
> > were allowed to read/follow the symlink and existing systems broke.
> > Exempting symlinks caused the /proc/net symlink to be labeled proc_t
> > instead, which was already widely allowed.  To avoid this problem when
> > enabling this capability, you will either need to allow
> > proc_net_t:lnk_file read widely or you will need to change the genfscon
> > statement to avoid labeling the symlink (there is an optional file mode
> > field in genfscon statements similar to that of file_contexts, e.g.
> >   genfscon proc /net -d system_u:object_r:proc_net_t:s0
> >   genfscon proc /net -- system_u:object_r:proc_net_t:s0
> > would only label directories and regular files with proc_net_t.
>
> I tested it with a refpolicy alike policy, where
> kernel_read_network_state() allows access to symlinks [1].
> Just systemd pid 1 wants to read the symlink only without reading any
> files, so I added kernel_search_network_state(systemd_t).
>
> [1]: https://github.com/SELinuxProject/refpolicy/blob/7e191b008e70ca535ceca2405967ddddd2ed975d/policy/modules/kernel/kernel.if#L1437

Thanks.  FWIW, I think the netns procfs case is a little different
simply because it was a change in the core kernel behavior and not a
change in the access controls.  While we have tricks, e.g. policy
capabilities, to help with migrating users/admins to new access
control changes, we don't always have the same abilities when the core
kernel changes.

One could make the argument that the netns procfs change was a "break
userspace" type of change, but I doubt you would have gotten far with
that because well ... containers.

Anyway, I've gone ahead and queued the patch up in selinux/next, you
should see it in the tree once the merge window closes.

-- 
paul moore
www.paul-moore.com