Re: [Freeipa-devel] [PATCH] 998 certmonger restarts services on renewal

2012-04-10 Thread Martin Kosek
On Fri, 2012-04-06 at 10:22 +0200, Martin Kosek wrote:
 On Thu, 2012-04-05 at 16:47 -0400, Rob Crittenden wrote:
  Rob Crittenden wrote:
   Martin Kosek wrote:
   On Tue, 2012-04-03 at 10:45 -0400, Rob Crittenden wrote:
   Rob Crittenden wrote:
   Martin Kosek wrote:
   On Mon, 2012-04-02 at 15:36 -0400, Rob Crittenden wrote:
   Rob Crittenden wrote:
   Martin Kosek wrote:
   On Tue, 2012-03-27 at 17:40 -0400, Rob Crittenden wrote:
   Certmonger will currently automatically renew server
   certificates but
   doesn't restart the services so you can still end up with expired
   certificates if you services never restart.
  
   This patch registers are restart command with certmonger so the
   IPA
   services will automatically be restarted to get the updated cert.
  
   Easy to test. Install IPA then resubmit the current server
   certs and
   watch the services restart:
  
   # ipa-getcert list
  
   Find the ID for either your dirsrv or httpd instance
  
   # ipa-getcert resubmit -iID
  
   Watch /var/log/httpd/error_log or
   /var/log/dirsrv/slapd-INSTANCE/errors
   to see the service restart.
  
   rob
  
   What about current instances - can we/do we want to update
   certmonger
   tracking so that their instances are restarted as well?
  
   Anyway, I found few issues SELinux issues with the patch:
  
   1) # rpm -Uvh freeipa-*
   Preparing... ### [100%]
   1:freeipa-python ### [ 20%]
   2:freeipa-client ### [ 40%]
   3:freeipa-admintools ### [
   60%]
   4:freeipa-server ### [ 80%]
   /usr/bin/chcon: failed to change context of
   `/usr/lib64/ipa/certmonger' to
   `unconfined_u:object_r:certmonger_unconfined_exec_t:s0': Invalid
   argument
   /usr/bin/chcon: failed to change context of
   `/usr/lib64/ipa/certmonger/restart_dirsrv' to
   `unconfined_u:object_r:certmonger_unconfined_exec_t:s0': Invalid
   argument
   /usr/bin/chcon: failed to change context of
   `/usr/lib64/ipa/certmonger/restart_httpd' to
   `unconfined_u:object_r:certmonger_unconfined_exec_t:s0': Invalid
   argument
   warning: %post(freeipa-server-2.1.90GIT5b895af-0.fc16.x86_64)
   scriptlet failed, exit status 1
   5:freeipa-server-selinux
   ###
   [100%]
  
   certmonger_unconfined_exec_t type was unknown with my selinux
   policy:
  
   selinux-policy-3.10.0-80.fc16.noarch
   selinux-policy-targeted-3.10.0-80.fc16.noarch
  
   If we need a higher SELinux version, we should bump the required
   package
   version spec file.
  
   Yeah, waiting on it to be backported.
  
  
   2) Change of SELinux context with /usr/bin/chcon is temporary until
   restorecon or system relabel occurs. I think we should make it
   persistent and enforce this type in our SELinux policy and
   rather call
   restorecon instead of chcon
  
   That's a good idea, why didn't I think of that :-(
  
   Ah, now I remember, it will be handled by selinux-policy. I would
   have
   used restorecon here but since the policy isn't there yet this seemed
   like a good idea.
  
   I'm trying to find out the status of this new policy, it may only
   make
   it into F-17.
  
   rob
  
   Ok. But if this policy does not go in F-16 and if we want this fix in
   F16 release too, I guess we would have to implement both approaches in
   our spec file:
  
   1) When on F16, include SELinux policy for restart scripts + run
   restorecon
   2) When on F17, do not include the SELinux policy (+ run restorecon)
  
   Martin
  
  
   Won't work without updated selinux-policy. Without the permission for
   certmonger to execute the commands things will still fail (just in
   really non-obvious and far in the future ways).
  
   It looks like this is fixed in F-17 selinux-policy-3.10.0-107.
  
   rob
  
   Updated patch which works on F-17.
  
   rob
  
   What about F-16? The restart scripts won't work with enabled enforcing
   and will raise AVCs. Maybe we really need to deliver our own SELinux
   policy allowing it on F-16.
  
   Right, I don't see this working on F-16. I don't really want to carry
   this type of policy. It goes beyond marking a few files as certmonger_t,
   it is going to let certmonger execute arbitrary scripts. This is best
   left to the SELinux team who understand the consequences better.
  
  
   I also found an issue with the restart scripts:
  
   1) restart_dirsrv: this won't work with systemd:
  
   # /sbin/service dirsrv restart
   Redirecting to /bin/systemctl restart dirsrv.service
   Failed to issue method call: Unit dirsrv.service failed to load: No such
   file or directory. See system logs and 'systemctl status dirsrv.service'
   for details.
  
   Wouldn't work so hot for sysV either as we'd be restarting all
   instances. I'll take a look.
  
   We would need to pass an instance of IPA dirsrv for this to 

Re: [Freeipa-devel] [PATCH] 998 certmonger restarts services on renewal

2012-04-10 Thread Dmitri Pal
On 04/10/2012 04:48 PM, Martin Kosek wrote:
 On Fri, 2012-04-06 at 10:22 +0200, Martin Kosek wrote:
 On Thu, 2012-04-05 at 16:47 -0400, Rob Crittenden wrote:
 Rob Crittenden wrote:
 Martin Kosek wrote:
 On Tue, 2012-04-03 at 10:45 -0400, Rob Crittenden wrote:
 Rob Crittenden wrote:
 Martin Kosek wrote:
 On Mon, 2012-04-02 at 15:36 -0400, Rob Crittenden wrote:
 Rob Crittenden wrote:
 Martin Kosek wrote:
 On Tue, 2012-03-27 at 17:40 -0400, Rob Crittenden wrote:
 Certmonger will currently automatically renew server
 certificates but
 doesn't restart the services so you can still end up with expired
 certificates if you services never restart.

 This patch registers are restart command with certmonger so the
 IPA
 services will automatically be restarted to get the updated cert.

 Easy to test. Install IPA then resubmit the current server
 certs and
 watch the services restart:

 # ipa-getcert list

 Find the ID for either your dirsrv or httpd instance

 # ipa-getcert resubmit -iID

 Watch /var/log/httpd/error_log or
 /var/log/dirsrv/slapd-INSTANCE/errors
 to see the service restart.

 rob
 What about current instances - can we/do we want to update
 certmonger
 tracking so that their instances are restarted as well?

 Anyway, I found few issues SELinux issues with the patch:

 1) # rpm -Uvh freeipa-*
 Preparing... ### [100%]
 1:freeipa-python ### [ 20%]
 2:freeipa-client ### [ 40%]
 3:freeipa-admintools ### [
 60%]
 4:freeipa-server ### [ 80%]
 /usr/bin/chcon: failed to change context of
 `/usr/lib64/ipa/certmonger' to
 `unconfined_u:object_r:certmonger_unconfined_exec_t:s0': Invalid
 argument
 /usr/bin/chcon: failed to change context of
 `/usr/lib64/ipa/certmonger/restart_dirsrv' to
 `unconfined_u:object_r:certmonger_unconfined_exec_t:s0': Invalid
 argument
 /usr/bin/chcon: failed to change context of
 `/usr/lib64/ipa/certmonger/restart_httpd' to
 `unconfined_u:object_r:certmonger_unconfined_exec_t:s0': Invalid
 argument
 warning: %post(freeipa-server-2.1.90GIT5b895af-0.fc16.x86_64)
 scriptlet failed, exit status 1
 5:freeipa-server-selinux
 ###
 [100%]

 certmonger_unconfined_exec_t type was unknown with my selinux
 policy:

 selinux-policy-3.10.0-80.fc16.noarch
 selinux-policy-targeted-3.10.0-80.fc16.noarch

 If we need a higher SELinux version, we should bump the required
 package
 version spec file.
 Yeah, waiting on it to be backported.

 2) Change of SELinux context with /usr/bin/chcon is temporary until
 restorecon or system relabel occurs. I think we should make it
 persistent and enforce this type in our SELinux policy and
 rather call
 restorecon instead of chcon
 That's a good idea, why didn't I think of that :-(
 Ah, now I remember, it will be handled by selinux-policy. I would
 have
 used restorecon here but since the policy isn't there yet this seemed
 like a good idea.

 I'm trying to find out the status of this new policy, it may only
 make
 it into F-17.

 rob
 Ok. But if this policy does not go in F-16 and if we want this fix in
 F16 release too, I guess we would have to implement both approaches in
 our spec file:

 1) When on F16, include SELinux policy for restart scripts + run
 restorecon
 2) When on F17, do not include the SELinux policy (+ run restorecon)

 Martin

 Won't work without updated selinux-policy. Without the permission for
 certmonger to execute the commands things will still fail (just in
 really non-obvious and far in the future ways).

 It looks like this is fixed in F-17 selinux-policy-3.10.0-107.

 rob
 Updated patch which works on F-17.

 rob
 What about F-16? The restart scripts won't work with enabled enforcing
 and will raise AVCs. Maybe we really need to deliver our own SELinux
 policy allowing it on F-16.
 Right, I don't see this working on F-16. I don't really want to carry
 this type of policy. It goes beyond marking a few files as certmonger_t,
 it is going to let certmonger execute arbitrary scripts. This is best
 left to the SELinux team who understand the consequences better.

 I also found an issue with the restart scripts:

 1) restart_dirsrv: this won't work with systemd:

 # /sbin/service dirsrv restart
 Redirecting to /bin/systemctl restart dirsrv.service
 Failed to issue method call: Unit dirsrv.service failed to load: No such
 file or directory. See system logs and 'systemctl status dirsrv.service'
 for details.
 Wouldn't work so hot for sysV either as we'd be restarting all
 instances. I'll take a look.

 We would need to pass an instance of IPA dirsrv for this to work.

 2) restart_httpd:
 Is reload enough for httpd to pull a new certificate? Don't we need a
 full restart? If reload is enough, I think the command should be named
 reload_httpd
 Yes, it causes the modules to be reloaded which will reload the NSS
 

Re: [Freeipa-devel] [PATCH] 998 certmonger restarts services on renewal

2012-04-10 Thread Rob Crittenden

Martin Kosek wrote:

On Fri, 2012-04-06 at 10:22 +0200, Martin Kosek wrote:

On Thu, 2012-04-05 at 16:47 -0400, Rob Crittenden wrote:

Rob Crittenden wrote:

Martin Kosek wrote:

On Tue, 2012-04-03 at 10:45 -0400, Rob Crittenden wrote:

Rob Crittenden wrote:

Martin Kosek wrote:

On Mon, 2012-04-02 at 15:36 -0400, Rob Crittenden wrote:

Rob Crittenden wrote:

Martin Kosek wrote:

On Tue, 2012-03-27 at 17:40 -0400, Rob Crittenden wrote:

Certmonger will currently automatically renew server
certificates but
doesn't restart the services so you can still end up with expired
certificates if you services never restart.

This patch registers are restart command with certmonger so the
IPA
services will automatically be restarted to get the updated cert.

Easy to test. Install IPA then resubmit the current server
certs and
watch the services restart:

# ipa-getcert list

Find the ID for either your dirsrv or httpd instance

# ipa-getcert resubmit -iID

Watch /var/log/httpd/error_log or
/var/log/dirsrv/slapd-INSTANCE/errors
to see the service restart.

rob


What about current instances - can we/do we want to update
certmonger
tracking so that their instances are restarted as well?

Anyway, I found few issues SELinux issues with the patch:

1) # rpm -Uvh freeipa-*
Preparing... ### [100%]
1:freeipa-python ### [ 20%]
2:freeipa-client ### [ 40%]
3:freeipa-admintools ### [
60%]
4:freeipa-server ### [ 80%]
/usr/bin/chcon: failed to change context of
`/usr/lib64/ipa/certmonger' to
`unconfined_u:object_r:certmonger_unconfined_exec_t:s0': Invalid
argument
/usr/bin/chcon: failed to change context of
`/usr/lib64/ipa/certmonger/restart_dirsrv' to
`unconfined_u:object_r:certmonger_unconfined_exec_t:s0': Invalid
argument
/usr/bin/chcon: failed to change context of
`/usr/lib64/ipa/certmonger/restart_httpd' to
`unconfined_u:object_r:certmonger_unconfined_exec_t:s0': Invalid
argument
warning: %post(freeipa-server-2.1.90GIT5b895af-0.fc16.x86_64)
scriptlet failed, exit status 1
5:freeipa-server-selinux
###
[100%]

certmonger_unconfined_exec_t type was unknown with my selinux
policy:

selinux-policy-3.10.0-80.fc16.noarch
selinux-policy-targeted-3.10.0-80.fc16.noarch

If we need a higher SELinux version, we should bump the required
package
version spec file.


Yeah, waiting on it to be backported.



2) Change of SELinux context with /usr/bin/chcon is temporary until
restorecon or system relabel occurs. I think we should make it
persistent and enforce this type in our SELinux policy and
rather call
restorecon instead of chcon


That's a good idea, why didn't I think of that :-(


Ah, now I remember, it will be handled by selinux-policy. I would
have
used restorecon here but since the policy isn't there yet this seemed
like a good idea.

I'm trying to find out the status of this new policy, it may only
make
it into F-17.

rob


Ok. But if this policy does not go in F-16 and if we want this fix in
F16 release too, I guess we would have to implement both approaches in
our spec file:

1) When on F16, include SELinux policy for restart scripts + run
restorecon
2) When on F17, do not include the SELinux policy (+ run restorecon)

Martin



Won't work without updated selinux-policy. Without the permission for
certmonger to execute the commands things will still fail (just in
really non-obvious and far in the future ways).

It looks like this is fixed in F-17 selinux-policy-3.10.0-107.

rob


Updated patch which works on F-17.

rob


What about F-16? The restart scripts won't work with enabled enforcing
and will raise AVCs. Maybe we really need to deliver our own SELinux
policy allowing it on F-16.


Right, I don't see this working on F-16. I don't really want to carry
this type of policy. It goes beyond marking a few files as certmonger_t,
it is going to let certmonger execute arbitrary scripts. This is best
left to the SELinux team who understand the consequences better.



I also found an issue with the restart scripts:

1) restart_dirsrv: this won't work with systemd:

# /sbin/service dirsrv restart
Redirecting to /bin/systemctl restart dirsrv.service
Failed to issue method call: Unit dirsrv.service failed to load: No such
file or directory. See system logs and 'systemctl status dirsrv.service'
for details.


Wouldn't work so hot for sysV either as we'd be restarting all
instances. I'll take a look.


We would need to pass an instance of IPA dirsrv for this to work.

2) restart_httpd:
Is reload enough for httpd to pull a new certificate? Don't we need a
full restart? If reload is enough, I think the command should be named
reload_httpd


Yes, it causes the modules to be reloaded which will reload the NSS
database, that's all we need. I named it this way for consistency. I can
rename it, though I doubt 

Re: [Freeipa-devel] [PATCH] 998 certmonger restarts services on renewal

2012-04-06 Thread Martin Kosek
On Thu, 2012-04-05 at 16:47 -0400, Rob Crittenden wrote:
 Rob Crittenden wrote:
  Martin Kosek wrote:
  On Tue, 2012-04-03 at 10:45 -0400, Rob Crittenden wrote:
  Rob Crittenden wrote:
  Martin Kosek wrote:
  On Mon, 2012-04-02 at 15:36 -0400, Rob Crittenden wrote:
  Rob Crittenden wrote:
  Martin Kosek wrote:
  On Tue, 2012-03-27 at 17:40 -0400, Rob Crittenden wrote:
  Certmonger will currently automatically renew server
  certificates but
  doesn't restart the services so you can still end up with expired
  certificates if you services never restart.
 
  This patch registers are restart command with certmonger so the
  IPA
  services will automatically be restarted to get the updated cert.
 
  Easy to test. Install IPA then resubmit the current server
  certs and
  watch the services restart:
 
  # ipa-getcert list
 
  Find the ID for either your dirsrv or httpd instance
 
  # ipa-getcert resubmit -iID
 
  Watch /var/log/httpd/error_log or
  /var/log/dirsrv/slapd-INSTANCE/errors
  to see the service restart.
 
  rob
 
  What about current instances - can we/do we want to update
  certmonger
  tracking so that their instances are restarted as well?
 
  Anyway, I found few issues SELinux issues with the patch:
 
  1) # rpm -Uvh freeipa-*
  Preparing... ### [100%]
  1:freeipa-python ### [ 20%]
  2:freeipa-client ### [ 40%]
  3:freeipa-admintools ### [
  60%]
  4:freeipa-server ### [ 80%]
  /usr/bin/chcon: failed to change context of
  `/usr/lib64/ipa/certmonger' to
  `unconfined_u:object_r:certmonger_unconfined_exec_t:s0': Invalid
  argument
  /usr/bin/chcon: failed to change context of
  `/usr/lib64/ipa/certmonger/restart_dirsrv' to
  `unconfined_u:object_r:certmonger_unconfined_exec_t:s0': Invalid
  argument
  /usr/bin/chcon: failed to change context of
  `/usr/lib64/ipa/certmonger/restart_httpd' to
  `unconfined_u:object_r:certmonger_unconfined_exec_t:s0': Invalid
  argument
  warning: %post(freeipa-server-2.1.90GIT5b895af-0.fc16.x86_64)
  scriptlet failed, exit status 1
  5:freeipa-server-selinux
  ###
  [100%]
 
  certmonger_unconfined_exec_t type was unknown with my selinux
  policy:
 
  selinux-policy-3.10.0-80.fc16.noarch
  selinux-policy-targeted-3.10.0-80.fc16.noarch
 
  If we need a higher SELinux version, we should bump the required
  package
  version spec file.
 
  Yeah, waiting on it to be backported.
 
 
  2) Change of SELinux context with /usr/bin/chcon is temporary until
  restorecon or system relabel occurs. I think we should make it
  persistent and enforce this type in our SELinux policy and
  rather call
  restorecon instead of chcon
 
  That's a good idea, why didn't I think of that :-(
 
  Ah, now I remember, it will be handled by selinux-policy. I would
  have
  used restorecon here but since the policy isn't there yet this seemed
  like a good idea.
 
  I'm trying to find out the status of this new policy, it may only
  make
  it into F-17.
 
  rob
 
  Ok. But if this policy does not go in F-16 and if we want this fix in
  F16 release too, I guess we would have to implement both approaches in
  our spec file:
 
  1) When on F16, include SELinux policy for restart scripts + run
  restorecon
  2) When on F17, do not include the SELinux policy (+ run restorecon)
 
  Martin
 
 
  Won't work without updated selinux-policy. Without the permission for
  certmonger to execute the commands things will still fail (just in
  really non-obvious and far in the future ways).
 
  It looks like this is fixed in F-17 selinux-policy-3.10.0-107.
 
  rob
 
  Updated patch which works on F-17.
 
  rob
 
  What about F-16? The restart scripts won't work with enabled enforcing
  and will raise AVCs. Maybe we really need to deliver our own SELinux
  policy allowing it on F-16.
 
  Right, I don't see this working on F-16. I don't really want to carry
  this type of policy. It goes beyond marking a few files as certmonger_t,
  it is going to let certmonger execute arbitrary scripts. This is best
  left to the SELinux team who understand the consequences better.
 
 
  I also found an issue with the restart scripts:
 
  1) restart_dirsrv: this won't work with systemd:
 
  # /sbin/service dirsrv restart
  Redirecting to /bin/systemctl restart dirsrv.service
  Failed to issue method call: Unit dirsrv.service failed to load: No such
  file or directory. See system logs and 'systemctl status dirsrv.service'
  for details.
 
  Wouldn't work so hot for sysV either as we'd be restarting all
  instances. I'll take a look.
 
  We would need to pass an instance of IPA dirsrv for this to work.
 
  2) restart_httpd:
  Is reload enough for httpd to pull a new certificate? Don't we need a
  full restart? If reload is enough, I think the command should be named
  reload_httpd
 
  Yes, it 

Re: [Freeipa-devel] [PATCH] 998 certmonger restarts services on renewal

2012-04-05 Thread Rob Crittenden

Rob Crittenden wrote:

Martin Kosek wrote:

On Tue, 2012-04-03 at 10:45 -0400, Rob Crittenden wrote:

Rob Crittenden wrote:

Martin Kosek wrote:

On Mon, 2012-04-02 at 15:36 -0400, Rob Crittenden wrote:

Rob Crittenden wrote:

Martin Kosek wrote:

On Tue, 2012-03-27 at 17:40 -0400, Rob Crittenden wrote:

Certmonger will currently automatically renew server
certificates but
doesn't restart the services so you can still end up with expired
certificates if you services never restart.

This patch registers are restart command with certmonger so the
IPA
services will automatically be restarted to get the updated cert.

Easy to test. Install IPA then resubmit the current server
certs and
watch the services restart:

# ipa-getcert list

Find the ID for either your dirsrv or httpd instance

# ipa-getcert resubmit -iID

Watch /var/log/httpd/error_log or
/var/log/dirsrv/slapd-INSTANCE/errors
to see the service restart.

rob


What about current instances - can we/do we want to update
certmonger
tracking so that their instances are restarted as well?

Anyway, I found few issues SELinux issues with the patch:

1) # rpm -Uvh freeipa-*
Preparing... ### [100%]
1:freeipa-python ### [ 20%]
2:freeipa-client ### [ 40%]
3:freeipa-admintools ### [
60%]
4:freeipa-server ### [ 80%]
/usr/bin/chcon: failed to change context of
`/usr/lib64/ipa/certmonger' to
`unconfined_u:object_r:certmonger_unconfined_exec_t:s0': Invalid
argument
/usr/bin/chcon: failed to change context of
`/usr/lib64/ipa/certmonger/restart_dirsrv' to
`unconfined_u:object_r:certmonger_unconfined_exec_t:s0': Invalid
argument
/usr/bin/chcon: failed to change context of
`/usr/lib64/ipa/certmonger/restart_httpd' to
`unconfined_u:object_r:certmonger_unconfined_exec_t:s0': Invalid
argument
warning: %post(freeipa-server-2.1.90GIT5b895af-0.fc16.x86_64)
scriptlet failed, exit status 1
5:freeipa-server-selinux
###
[100%]

certmonger_unconfined_exec_t type was unknown with my selinux
policy:

selinux-policy-3.10.0-80.fc16.noarch
selinux-policy-targeted-3.10.0-80.fc16.noarch

If we need a higher SELinux version, we should bump the required
package
version spec file.


Yeah, waiting on it to be backported.



2) Change of SELinux context with /usr/bin/chcon is temporary until
restorecon or system relabel occurs. I think we should make it
persistent and enforce this type in our SELinux policy and
rather call
restorecon instead of chcon


That's a good idea, why didn't I think of that :-(


Ah, now I remember, it will be handled by selinux-policy. I would
have
used restorecon here but since the policy isn't there yet this seemed
like a good idea.

I'm trying to find out the status of this new policy, it may only
make
it into F-17.

rob


Ok. But if this policy does not go in F-16 and if we want this fix in
F16 release too, I guess we would have to implement both approaches in
our spec file:

1) When on F16, include SELinux policy for restart scripts + run
restorecon
2) When on F17, do not include the SELinux policy (+ run restorecon)

Martin



Won't work without updated selinux-policy. Without the permission for
certmonger to execute the commands things will still fail (just in
really non-obvious and far in the future ways).

It looks like this is fixed in F-17 selinux-policy-3.10.0-107.

rob


Updated patch which works on F-17.

rob


What about F-16? The restart scripts won't work with enabled enforcing
and will raise AVCs. Maybe we really need to deliver our own SELinux
policy allowing it on F-16.


Right, I don't see this working on F-16. I don't really want to carry
this type of policy. It goes beyond marking a few files as certmonger_t,
it is going to let certmonger execute arbitrary scripts. This is best
left to the SELinux team who understand the consequences better.



I also found an issue with the restart scripts:

1) restart_dirsrv: this won't work with systemd:

# /sbin/service dirsrv restart
Redirecting to /bin/systemctl restart dirsrv.service
Failed to issue method call: Unit dirsrv.service failed to load: No such
file or directory. See system logs and 'systemctl status dirsrv.service'
for details.


Wouldn't work so hot for sysV either as we'd be restarting all
instances. I'll take a look.


We would need to pass an instance of IPA dirsrv for this to work.

2) restart_httpd:
Is reload enough for httpd to pull a new certificate? Don't we need a
full restart? If reload is enough, I think the command should be named
reload_httpd


Yes, it causes the modules to be reloaded which will reload the NSS
database, that's all we need. I named it this way for consistency. I can
rename it, though I doubt it would cause any confusion either way.

rob


revised patch.

rob
From 100689ed25dcd216644f2d41f7f09091e71c0f68 Mon Sep 17 00:00:00 

Re: [Freeipa-devel] [PATCH] 998 certmonger restarts services on renewal

2012-04-03 Thread Martin Kosek
On Mon, 2012-04-02 at 15:36 -0400, Rob Crittenden wrote:
 Rob Crittenden wrote:
  Martin Kosek wrote:
  On Tue, 2012-03-27 at 17:40 -0400, Rob Crittenden wrote:
  Certmonger will currently automatically renew server certificates but
  doesn't restart the services so you can still end up with expired
  certificates if you services never restart.
 
  This patch registers are restart command with certmonger so the IPA
  services will automatically be restarted to get the updated cert.
 
  Easy to test. Install IPA then resubmit the current server certs and
  watch the services restart:
 
  # ipa-getcert list
 
  Find the ID for either your dirsrv or httpd instance
 
  # ipa-getcert resubmit -iID
 
  Watch /var/log/httpd/error_log or /var/log/dirsrv/slapd-INSTANCE/errors
  to see the service restart.
 
  rob
 
  What about current instances - can we/do we want to update certmonger
  tracking so that their instances are restarted as well?
 
  Anyway, I found few issues SELinux issues with the patch:
 
  1) # rpm -Uvh freeipa-*
  Preparing... ### [100%]
  1:freeipa-python ### [ 20%]
  2:freeipa-client ### [ 40%]
  3:freeipa-admintools ### [ 60%]
  4:freeipa-server ### [ 80%]
  /usr/bin/chcon: failed to change context of
  `/usr/lib64/ipa/certmonger' to
  `unconfined_u:object_r:certmonger_unconfined_exec_t:s0': Invalid argument
  /usr/bin/chcon: failed to change context of
  `/usr/lib64/ipa/certmonger/restart_dirsrv' to
  `unconfined_u:object_r:certmonger_unconfined_exec_t:s0': Invalid argument
  /usr/bin/chcon: failed to change context of
  `/usr/lib64/ipa/certmonger/restart_httpd' to
  `unconfined_u:object_r:certmonger_unconfined_exec_t:s0': Invalid argument
  warning: %post(freeipa-server-2.1.90GIT5b895af-0.fc16.x86_64)
  scriptlet failed, exit status 1
  5:freeipa-server-selinux ###
  [100%]
 
  certmonger_unconfined_exec_t type was unknown with my selinux policy:
 
  selinux-policy-3.10.0-80.fc16.noarch
  selinux-policy-targeted-3.10.0-80.fc16.noarch
 
  If we need a higher SELinux version, we should bump the required package
  version spec file.
 
  Yeah, waiting on it to be backported.
 
 
  2) Change of SELinux context with /usr/bin/chcon is temporary until
  restorecon or system relabel occurs. I think we should make it
  persistent and enforce this type in our SELinux policy and rather call
  restorecon instead of chcon
 
  That's a good idea, why didn't I think of that :-(
 
 Ah, now I remember, it will be handled by selinux-policy. I would have 
 used restorecon here but since the policy isn't there yet this seemed 
 like a good idea.
 
 I'm trying to find out the status of this new policy, it may only make 
 it into F-17.
 
 rob

Ok. But if this policy does not go in F-16 and if we want this fix in
F16 release too, I guess we would have to implement both approaches in
our spec file:

1) When on F16, include SELinux policy for restart scripts + run
restorecon
2) When on F17, do not include the SELinux policy (+ run restorecon)

Martin

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel


Re: [Freeipa-devel] [PATCH] 998 certmonger restarts services on renewal

2012-04-03 Thread Rob Crittenden

Martin Kosek wrote:

On Mon, 2012-04-02 at 15:36 -0400, Rob Crittenden wrote:

Rob Crittenden wrote:

Martin Kosek wrote:

On Tue, 2012-03-27 at 17:40 -0400, Rob Crittenden wrote:

Certmonger will currently automatically renew server certificates but
doesn't restart the services so you can still end up with expired
certificates if you services never restart.

This patch registers are restart command with certmonger so the IPA
services will automatically be restarted to get the updated cert.

Easy to test. Install IPA then resubmit the current server certs and
watch the services restart:

# ipa-getcert list

Find the ID for either your dirsrv or httpd instance

# ipa-getcert resubmit -iID

Watch /var/log/httpd/error_log or /var/log/dirsrv/slapd-INSTANCE/errors
to see the service restart.

rob


What about current instances - can we/do we want to update certmonger
tracking so that their instances are restarted as well?

Anyway, I found few issues SELinux issues with the patch:

1) # rpm -Uvh freeipa-*
Preparing... ### [100%]
1:freeipa-python ### [ 20%]
2:freeipa-client ### [ 40%]
3:freeipa-admintools ### [ 60%]
4:freeipa-server ### [ 80%]
/usr/bin/chcon: failed to change context of
`/usr/lib64/ipa/certmonger' to
`unconfined_u:object_r:certmonger_unconfined_exec_t:s0': Invalid argument
/usr/bin/chcon: failed to change context of
`/usr/lib64/ipa/certmonger/restart_dirsrv' to
`unconfined_u:object_r:certmonger_unconfined_exec_t:s0': Invalid argument
/usr/bin/chcon: failed to change context of
`/usr/lib64/ipa/certmonger/restart_httpd' to
`unconfined_u:object_r:certmonger_unconfined_exec_t:s0': Invalid argument
warning: %post(freeipa-server-2.1.90GIT5b895af-0.fc16.x86_64)
scriptlet failed, exit status 1
5:freeipa-server-selinux ###
[100%]

certmonger_unconfined_exec_t type was unknown with my selinux policy:

selinux-policy-3.10.0-80.fc16.noarch
selinux-policy-targeted-3.10.0-80.fc16.noarch

If we need a higher SELinux version, we should bump the required package
version spec file.


Yeah, waiting on it to be backported.



2) Change of SELinux context with /usr/bin/chcon is temporary until
restorecon or system relabel occurs. I think we should make it
persistent and enforce this type in our SELinux policy and rather call
restorecon instead of chcon


That's a good idea, why didn't I think of that :-(


Ah, now I remember, it will be handled by selinux-policy. I would have
used restorecon here but since the policy isn't there yet this seemed
like a good idea.

I'm trying to find out the status of this new policy, it may only make
it into F-17.

rob


Ok. But if this policy does not go in F-16 and if we want this fix in
F16 release too, I guess we would have to implement both approaches in
our spec file:

1) When on F16, include SELinux policy for restart scripts + run
restorecon
2) When on F17, do not include the SELinux policy (+ run restorecon)

Martin



Won't work without updated selinux-policy. Without the permission for 
certmonger to execute the commands things will still fail (just in 
really non-obvious and far in the future ways).


It looks like this is fixed in F-17 selinux-policy-3.10.0-107.

rob

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel


Re: [Freeipa-devel] [PATCH] 998 certmonger restarts services on renewal

2012-04-03 Thread Martin Kosek
On Tue, 2012-04-03 at 10:45 -0400, Rob Crittenden wrote:
 Rob Crittenden wrote:
  Martin Kosek wrote:
  On Mon, 2012-04-02 at 15:36 -0400, Rob Crittenden wrote:
  Rob Crittenden wrote:
  Martin Kosek wrote:
  On Tue, 2012-03-27 at 17:40 -0400, Rob Crittenden wrote:
  Certmonger will currently automatically renew server certificates but
  doesn't restart the services so you can still end up with expired
  certificates if you services never restart.
 
  This patch registers are restart command with certmonger so the IPA
  services will automatically be restarted to get the updated cert.
 
  Easy to test. Install IPA then resubmit the current server certs and
  watch the services restart:
 
  # ipa-getcert list
 
  Find the ID for either your dirsrv or httpd instance
 
  # ipa-getcert resubmit -iID
 
  Watch /var/log/httpd/error_log or
  /var/log/dirsrv/slapd-INSTANCE/errors
  to see the service restart.
 
  rob
 
  What about current instances - can we/do we want to update certmonger
  tracking so that their instances are restarted as well?
 
  Anyway, I found few issues SELinux issues with the patch:
 
  1) # rpm -Uvh freeipa-*
  Preparing... ### [100%]
  1:freeipa-python ### [ 20%]
  2:freeipa-client ### [ 40%]
  3:freeipa-admintools ### [
  60%]
  4:freeipa-server ### [ 80%]
  /usr/bin/chcon: failed to change context of
  `/usr/lib64/ipa/certmonger' to
  `unconfined_u:object_r:certmonger_unconfined_exec_t:s0': Invalid
  argument
  /usr/bin/chcon: failed to change context of
  `/usr/lib64/ipa/certmonger/restart_dirsrv' to
  `unconfined_u:object_r:certmonger_unconfined_exec_t:s0': Invalid
  argument
  /usr/bin/chcon: failed to change context of
  `/usr/lib64/ipa/certmonger/restart_httpd' to
  `unconfined_u:object_r:certmonger_unconfined_exec_t:s0': Invalid
  argument
  warning: %post(freeipa-server-2.1.90GIT5b895af-0.fc16.x86_64)
  scriptlet failed, exit status 1
  5:freeipa-server-selinux ###
  [100%]
 
  certmonger_unconfined_exec_t type was unknown with my selinux policy:
 
  selinux-policy-3.10.0-80.fc16.noarch
  selinux-policy-targeted-3.10.0-80.fc16.noarch
 
  If we need a higher SELinux version, we should bump the required
  package
  version spec file.
 
  Yeah, waiting on it to be backported.
 
 
  2) Change of SELinux context with /usr/bin/chcon is temporary until
  restorecon or system relabel occurs. I think we should make it
  persistent and enforce this type in our SELinux policy and rather call
  restorecon instead of chcon
 
  That's a good idea, why didn't I think of that :-(
 
  Ah, now I remember, it will be handled by selinux-policy. I would have
  used restorecon here but since the policy isn't there yet this seemed
  like a good idea.
 
  I'm trying to find out the status of this new policy, it may only make
  it into F-17.
 
  rob
 
  Ok. But if this policy does not go in F-16 and if we want this fix in
  F16 release too, I guess we would have to implement both approaches in
  our spec file:
 
  1) When on F16, include SELinux policy for restart scripts + run
  restorecon
  2) When on F17, do not include the SELinux policy (+ run restorecon)
 
  Martin
 
 
  Won't work without updated selinux-policy. Without the permission for
  certmonger to execute the commands things will still fail (just in
  really non-obvious and far in the future ways).
 
  It looks like this is fixed in F-17 selinux-policy-3.10.0-107.
 
  rob
 
 Updated patch which works on F-17.
 
 rob

What about F-16? The restart scripts won't work with enabled enforcing
and will raise AVCs. Maybe we really need to deliver our own SELinux
policy allowing it on F-16.

I also found an issue with the restart scripts:

1) restart_dirsrv: this won't work with systemd:

# /sbin/service dirsrv restart
Redirecting to /bin/systemctl  restart dirsrv.service
Failed to issue method call: Unit dirsrv.service failed to load: No such
file or directory. See system logs and 'systemctl status dirsrv.service'
for details.

We would need to pass an instance of IPA dirsrv for this to work.

2) restart_httpd:
Is reload enough for httpd to pull a new certificate? Don't we need a
full restart? If reload is enough, I think the command should be named
reload_httpd

Martin

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel


Re: [Freeipa-devel] [PATCH] 998 certmonger restarts services on renewal

2012-04-03 Thread Rob Crittenden

Martin Kosek wrote:

On Tue, 2012-04-03 at 10:45 -0400, Rob Crittenden wrote:

Rob Crittenden wrote:

Martin Kosek wrote:

On Mon, 2012-04-02 at 15:36 -0400, Rob Crittenden wrote:

Rob Crittenden wrote:

Martin Kosek wrote:

On Tue, 2012-03-27 at 17:40 -0400, Rob Crittenden wrote:

Certmonger will currently automatically renew server certificates but
doesn't restart the services so you can still end up with expired
certificates if you services never restart.

This patch registers are restart command with certmonger so the IPA
services will automatically be restarted to get the updated cert.

Easy to test. Install IPA then resubmit the current server certs and
watch the services restart:

# ipa-getcert list

Find the ID for either your dirsrv or httpd instance

# ipa-getcert resubmit -iID

Watch /var/log/httpd/error_log or
/var/log/dirsrv/slapd-INSTANCE/errors
to see the service restart.

rob


What about current instances - can we/do we want to update certmonger
tracking so that their instances are restarted as well?

Anyway, I found few issues SELinux issues with the patch:

1) # rpm -Uvh freeipa-*
Preparing... ### [100%]
1:freeipa-python ### [ 20%]
2:freeipa-client ### [ 40%]
3:freeipa-admintools ### [
60%]
4:freeipa-server ### [ 80%]
/usr/bin/chcon: failed to change context of
`/usr/lib64/ipa/certmonger' to
`unconfined_u:object_r:certmonger_unconfined_exec_t:s0': Invalid
argument
/usr/bin/chcon: failed to change context of
`/usr/lib64/ipa/certmonger/restart_dirsrv' to
`unconfined_u:object_r:certmonger_unconfined_exec_t:s0': Invalid
argument
/usr/bin/chcon: failed to change context of
`/usr/lib64/ipa/certmonger/restart_httpd' to
`unconfined_u:object_r:certmonger_unconfined_exec_t:s0': Invalid
argument
warning: %post(freeipa-server-2.1.90GIT5b895af-0.fc16.x86_64)
scriptlet failed, exit status 1
5:freeipa-server-selinux ###
[100%]

certmonger_unconfined_exec_t type was unknown with my selinux policy:

selinux-policy-3.10.0-80.fc16.noarch
selinux-policy-targeted-3.10.0-80.fc16.noarch

If we need a higher SELinux version, we should bump the required
package
version spec file.


Yeah, waiting on it to be backported.



2) Change of SELinux context with /usr/bin/chcon is temporary until
restorecon or system relabel occurs. I think we should make it
persistent and enforce this type in our SELinux policy and rather call
restorecon instead of chcon


That's a good idea, why didn't I think of that :-(


Ah, now I remember, it will be handled by selinux-policy. I would have
used restorecon here but since the policy isn't there yet this seemed
like a good idea.

I'm trying to find out the status of this new policy, it may only make
it into F-17.

rob


Ok. But if this policy does not go in F-16 and if we want this fix in
F16 release too, I guess we would have to implement both approaches in
our spec file:

1) When on F16, include SELinux policy for restart scripts + run
restorecon
2) When on F17, do not include the SELinux policy (+ run restorecon)

Martin



Won't work without updated selinux-policy. Without the permission for
certmonger to execute the commands things will still fail (just in
really non-obvious and far in the future ways).

It looks like this is fixed in F-17 selinux-policy-3.10.0-107.

rob


Updated patch which works on F-17.

rob


What about F-16? The restart scripts won't work with enabled enforcing
and will raise AVCs. Maybe we really need to deliver our own SELinux
policy allowing it on F-16.


Right, I don't see this working on F-16. I don't really want to carry 
this type of policy. It goes beyond marking a few files as certmonger_t, 
it is going to let certmonger execute arbitrary scripts. This is best 
left to the SELinux team who understand the consequences better.




I also found an issue with the restart scripts:

1) restart_dirsrv: this won't work with systemd:

# /sbin/service dirsrv restart
Redirecting to /bin/systemctl  restart dirsrv.service
Failed to issue method call: Unit dirsrv.service failed to load: No such
file or directory. See system logs and 'systemctl status dirsrv.service'
for details.


Wouldn't work so hot for sysV either as we'd be restarting all 
instances. I'll take a look.



We would need to pass an instance of IPA dirsrv for this to work.

2) restart_httpd:
Is reload enough for httpd to pull a new certificate? Don't we need a
full restart? If reload is enough, I think the command should be named
reload_httpd


Yes, it causes the modules to be reloaded which will reload the NSS 
database, that's all we need. I named it this way for consistency. I can 
rename it, though I doubt it would cause any confusion either way.


rob

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com

Re: [Freeipa-devel] [PATCH] 998 certmonger restarts services on renewal

2012-04-02 Thread Martin Kosek
On Tue, 2012-03-27 at 17:40 -0400, Rob Crittenden wrote:
 Certmonger will currently automatically renew server certificates but 
 doesn't restart the services so you can still end up with expired 
 certificates if you services never restart.
 
 This patch registers are restart command with certmonger so the IPA 
 services will automatically be restarted to get the updated cert.
 
 Easy to test. Install IPA then resubmit the current server certs and 
 watch the services restart:
 
 # ipa-getcert list
 
 Find the ID for either your dirsrv or httpd instance
 
 # ipa-getcert resubmit -i ID
 
 Watch /var/log/httpd/error_log or /var/log/dirsrv/slapd-INSTANCE/errors 
 to see the service restart.
 
 rob

What about current instances - can we/do we want to update certmonger
tracking so that their instances are restarted as well?

Anyway, I found few issues SELinux issues with the patch:

1) # rpm -Uvh freeipa-*
Preparing...### [100%]
   1:freeipa-python ### [ 20%]
   2:freeipa-client ### [ 40%]
   3:freeipa-admintools ### [ 60%]
   4:freeipa-server ### [ 80%]
/usr/bin/chcon: failed to change context of `/usr/lib64/ipa/certmonger' to 
`unconfined_u:object_r:certmonger_unconfined_exec_t:s0': Invalid argument
/usr/bin/chcon: failed to change context of 
`/usr/lib64/ipa/certmonger/restart_dirsrv' to 
`unconfined_u:object_r:certmonger_unconfined_exec_t:s0': Invalid argument
/usr/bin/chcon: failed to change context of 
`/usr/lib64/ipa/certmonger/restart_httpd' to 
`unconfined_u:object_r:certmonger_unconfined_exec_t:s0': Invalid argument
warning: %post(freeipa-server-2.1.90GIT5b895af-0.fc16.x86_64) scriptlet failed, 
exit status 1
   5:freeipa-server-selinux ### [100%]

certmonger_unconfined_exec_t type was unknown with my selinux policy:

selinux-policy-3.10.0-80.fc16.noarch
selinux-policy-targeted-3.10.0-80.fc16.noarch

If we need a higher SELinux version, we should bump the required package
version spec file.


2) Change of SELinux context with /usr/bin/chcon is temporary until
restorecon or system relabel occurs. I think we should make it
persistent and enforce this type in our SELinux policy and rather call
restorecon instead of chcon

Martin

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel


Re: [Freeipa-devel] [PATCH] 998 certmonger restarts services on renewal

2012-04-02 Thread Nalin Dahyabhai
On Mon, Apr 02, 2012 at 03:47:20PM +0200, Martin Kosek wrote:
 On Tue, 2012-03-27 at 17:40 -0400, Rob Crittenden wrote:
  Certmonger will currently automatically renew server certificates but 
  doesn't restart the services so you can still end up with expired 
  certificates if you services never restart.
  
  This patch registers are restart command with certmonger so the IPA 
  services will automatically be restarted to get the updated cert.
  
  Easy to test. Install IPA then resubmit the current server certs and 
  watch the services restart:
  
  # ipa-getcert list
  
  Find the ID for either your dirsrv or httpd instance
  
  # ipa-getcert resubmit -i ID
  
  Watch /var/log/httpd/error_log or /var/log/dirsrv/slapd-INSTANCE/errors 
  to see the service restart.
 
 What about current instances - can we/do we want to update certmonger
 tracking so that their instances are restarted as well?

You can use the not-exactly-well-named start-tracking command to add a
post-save command:

  ipa-getcert start-tracking \
-d /etc/dirsrv/slapd-PKI-IPA -n Server-Cert \
-C /usr/bin/logger BeenThereDoneThat

Or use the ID, as Rob did above.

HTH,

Nalin

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel


Re: [Freeipa-devel] [PATCH] 998 certmonger restarts services on renewal

2012-04-02 Thread Rob Crittenden

Rob Crittenden wrote:

Martin Kosek wrote:

On Tue, 2012-03-27 at 17:40 -0400, Rob Crittenden wrote:

Certmonger will currently automatically renew server certificates but
doesn't restart the services so you can still end up with expired
certificates if you services never restart.

This patch registers are restart command with certmonger so the IPA
services will automatically be restarted to get the updated cert.

Easy to test. Install IPA then resubmit the current server certs and
watch the services restart:

# ipa-getcert list

Find the ID for either your dirsrv or httpd instance

# ipa-getcert resubmit -iID

Watch /var/log/httpd/error_log or /var/log/dirsrv/slapd-INSTANCE/errors
to see the service restart.

rob


What about current instances - can we/do we want to update certmonger
tracking so that their instances are restarted as well?

Anyway, I found few issues SELinux issues with the patch:

1) # rpm -Uvh freeipa-*
Preparing... ### [100%]
1:freeipa-python ### [ 20%]
2:freeipa-client ### [ 40%]
3:freeipa-admintools ### [ 60%]
4:freeipa-server ### [ 80%]
/usr/bin/chcon: failed to change context of
`/usr/lib64/ipa/certmonger' to
`unconfined_u:object_r:certmonger_unconfined_exec_t:s0': Invalid argument
/usr/bin/chcon: failed to change context of
`/usr/lib64/ipa/certmonger/restart_dirsrv' to
`unconfined_u:object_r:certmonger_unconfined_exec_t:s0': Invalid argument
/usr/bin/chcon: failed to change context of
`/usr/lib64/ipa/certmonger/restart_httpd' to
`unconfined_u:object_r:certmonger_unconfined_exec_t:s0': Invalid argument
warning: %post(freeipa-server-2.1.90GIT5b895af-0.fc16.x86_64)
scriptlet failed, exit status 1
5:freeipa-server-selinux ###
[100%]

certmonger_unconfined_exec_t type was unknown with my selinux policy:

selinux-policy-3.10.0-80.fc16.noarch
selinux-policy-targeted-3.10.0-80.fc16.noarch

If we need a higher SELinux version, we should bump the required package
version spec file.


Yeah, waiting on it to be backported.



2) Change of SELinux context with /usr/bin/chcon is temporary until
restorecon or system relabel occurs. I think we should make it
persistent and enforce this type in our SELinux policy and rather call
restorecon instead of chcon


That's a good idea, why didn't I think of that :-(


Ah, now I remember, it will be handled by selinux-policy. I would have 
used restorecon here but since the policy isn't there yet this seemed 
like a good idea.


I'm trying to find out the status of this new policy, it may only make 
it into F-17.


rob

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel