Bug#823781: sks: init script issues

2016-05-18 Thread Daniel Kahn Gillmor
On Thu 2016-05-12 09:43:12 -0400, Christoph Anton Mitterer wrote:
> btw: It seems that -8, (in which sks starts again), the service is not
> enabled per default.
>
> I personally think that's good, because I consider Debian's usual
> behaviour of enabling services at installation insecure and stupid, but
> perhaps you may still want to follow that.

the sysvinit approach also doesn't start the service by default.  the
admin has to manually edit /etc/default/sks in order to get it to start
up.

With systemd as pid 1, the admin needs instead to:

  systemctl enable systemd

This is a superior arrangement (esp. considering that it's not possible
to start up sks without jumping through keydump fetches and db builds),
and I am happy with it.  It's all documented in
/usr/share/doc/README.Debian.

--dkg



Bug#823781: sks: init script issues

2016-05-18 Thread Daniel Kahn Gillmor
On Thu 2016-05-12 09:00:30 -0400, Christoph Anton Mitterer wrote:
> I've had seen that, but as I wrote in my mail before, it may actually
> not even be required (and my original request would thus be wrong).

sks definitely needs access to the local filesystem (/var/lib/sks) and
to the network (it listens on ports on its own).

> Perhaps these targets are already implied much earlier, or at least by
> multi-user.target... then we could argue, that if any user hooks in
> sks.service to some earlier target he probably knows what he's doing.

users who do that can override the .service file with one of their own
as well.

> That's why my advice would have been to have three services:
> sks.service => simply starts/stops/restarts both and is to be used for admins
> sks-hkp.service => for sks db
> sks-recon.service => for sks recon, likely with a After=sks-hkp.service, and 
> a Wants=sks-hkp.servce
>  simply because sks-recon.service "needs" hkp to work, 
> but won't crash, without it


This doesn't make any sense in some of the corner cases.  what happens
if i do:

 systemctl start sks.service
 systemctl stop sks-recon.service


what is the outcome now of:

  systemctl status sks.service

?

One .service file per actually running service makes a lot more sense.


> Well, this is probably true for the majority of all cases, I agree, but
> there could be special use-cases where one doesn't want/need to have
> recon running.

and an admin of that system can easily prohibit sks-recon from launching
with:

 systemctl mask sks-recon.service

or its equivalent:

 ln -s /dev/null /etc/systemd/system/sks-recon.service

The rest of the system should operate as normal.

>>   But if the recon process goes down, there's no need to
>> tear down the db process.  This is expressed by having
>> sks.service
>> "Wants=sks-recon.service"
>
> Uhm... I found that a bit strange... if sks.service = sks DB, than it
> shouldn't need in principle recon, and having it vice versa, i.e. sks-
> recon.service wanting a DB service seems more natural.

sks-db in normal operation *does* want recon -- so that it can provide
up-to-date keys to people who query it.  But it dosn't need it.

"sks recon" does explicitly depend on "sks db" because of the
"BindsTo=sks.service" directive in sks-recon.service.

> Well right now, AFAIU, I cannot just start DB, can I?
> If one says systemctl start sks it will run db and because of
> the Wants=sks-recon.service, it will also start recon.

If you don't want to run recon at all, you should mask
sks-recon.service, as described above.

> Well I'd probably also say one should rather concentrate on
> systemd,...  especially perhaps also employing some more confinement
> (e.g. sks shouldn't need access to /home).

There's lots more that could be done to improve the systemd situation,
including upstream work on socket activation.  Frankly, if i could drop
the sysvinit scripts without upsetting people, i'd be happy to do so.
If someone wants to step up to maintain the sysvinit scripts, that would
be really great.

Regards,

--dkg


signature.asc
Description: PGP signature


Bug#823781: sks: init script issues

2016-05-18 Thread Christoph Anton Mitterer
On Wed, 2016-05-18 at 11:32 -0400, Daniel Kahn Gillmor wrote:
> the sysvinit approach also doesn't start the service by default.  the
> admin has to manually edit /etc/default/sks in order to get it to
> start
> up.
Well sure, but that's just something that some services offer, and it
kinda conntradicts the idea of rc*.d, which already *is* the
configuration in sysvinit, whether to start or not - now all these
services which control it additionally via /etc/default/foo, have
basically two switches for the same thing.


> With systemd as pid 1, the admin needs instead to:
> 
>   systemctl enable systemd
> 
> This is a superior arrangement (esp. considering that it's not
> possible
> to start up sks without jumping through keydump fetches and db
> builds),
> and I am happy with it.  It's all documented in
> /usr/share/doc/README.Debian.
Sure I didn't disagree with that part =)
The bug was mainly about the dependency model of the two services, as
described before.


Cheers,
Chris.

smime.p7s
Description: S/MIME cryptographic signature


Bug#823781: sks: init script issues

2016-05-08 Thread Christoph Anton Mitterer
Package: sks
Version: 1.1.5-5
Severity: important


Hi.

As for the systemd unit files:
1) Almost for sure, they miss quite some dependencies,
   at least things like network.target and perhaps
   something that makes sure the local fs is there?
2) Right now sks.service wants sks-recon.service, but shouldn't
   it be vice versa?
   AFAIU, sks (i.e. sks db) can in principle run without sks-recon
   which just means no further reconciliation would take place.
   But sks-recon cannot run without sks db, which is also why
   the dependency should be vice versa, actually Wants isn't
   probably enough, and Requires would be appropriate.
   After/Before is however okay, AFAIU, as db should run before recon.
3) I think sks.service should become sks-db.service (or perhaps
   sks-hkp.serivce, which makes it more clear to people what it
   actually does, namely providing the actual HKP protocol).
   Additionally there could be a sks.service, which Wants the
   two single services (sks-hkp and sks-recon) and also stops
   both (when stopped).

As for the sysvinit style initscripts
4) The same splitting into two services (db/hkp and recon)
   should happen for /etc/init.d/sks

Cheers,
Chris.


PS: Priority important due to the missing networking dependency (1).

-- System Information:
Debian Release: stretch/sid
  APT prefers unstable
  APT policy: (500, 'unstable')
Architecture: amd64 (x86_64)

Kernel: Linux 4.5.0-2-amd64 (SMP w/8 CPU cores)
Locale: LANG=en_DE.UTF-8, LC_CTYPE=en_DE.UTF-8 (charmap=UTF-8)
Shell: /bin/sh linked to /bin/dash
Init: systemd (via /run/systemd/system)



Bug#823781: sks: init script issues

2016-05-08 Thread Christoph Anton Mitterer
Oh and I forgot:

>BindTo=sks.serviceYou probably mean BindsTo, BindTo, AFAICS doesn't exist.

smime.p7s
Description: S/MIME cryptographic signature


Bug#823781: sks: init script issues

2016-05-11 Thread Christoph Anton Mitterer
Hi Daniel.

I've seen commit eff25289e1244ac74ecae5a1a1c04113e052a27c ...


On Sun, 2016-05-08 at 23:14 +0200, Christoph Anton Mitterer wrote:
> 1) Almost for sure, they miss quite some dependencies,
>    at least things like network.target and perhaps
>    something that makes sure the local fs is there?

Is the After= really enough? Cause that controls only the order, not
what's pulled in... 
Perhaps one should ask the systemd guys what's actually intended
here... perhaps services shouldn't require these targets and things are
handled by the target that pulls in sks.service (e.g. multi-
user.target) already assuring that local-fs and network is there.
So maybe my point (1) was wrong, and one shuldn't include those.


What about the following? You closed the bug without commenting on
these:

> 2) Right now sks.service wants sks-recon.service, but shouldn't
>    it be vice versa?
>    AFAIU, sks (i.e. sks db) can in principle run without sks-recon
>    which just means no further reconciliation would take place.
>    But sks-recon cannot run without sks db, which is also why
>    the dependency should be vice versa, actually Wants isn't
>    probably enough, and Requires would be appropriate.
>    After/Before is however okay, AFAIU, as db should run before
> recon.
> 3) I think sks.service should become sks-db.service (or perhaps
>    sks-hkp.serivce, which makes it more clear to people what it
>    actually does, namely providing the actual HKP protocol).
>    Additionally there could be a sks.service, which Wants the
>    two single services (sks-hkp and sks-recon) and also stops
>    both (when stopped).
> 
> As for the sysvinit style initscripts
> 4) The same splitting into two services (db/hkp and recon)
>    should happen for /etc/init.d/sks


Cheers,
Chris.

smime.p7s
Description: S/MIME cryptographic signature


Bug#823781: sks: init script issues

2016-05-11 Thread Daniel Kahn Gillmor
Hi Christoph--

thanks for these thoughtful notes, and for your prompt testing of sks
1.1.5-5.  1.1.5-7 should address these concerns (and several others) but
i wanted to comment a little here too:

On Sun 2016-05-08 17:14:35 -0400, Christoph Anton Mitterer wrote:
> 1) Almost for sure, they miss quite some dependencies,
>at least things like network.target and perhaps
>something that makes sure the local fs is there?

This is now handled by having the following line in the Unit section of
/lib/systemd/system/sks.service:

After=local-fs.target network.target 

> 2) Right now sks.service wants sks-recon.service, but shouldn't
>it be vice versa?
>AFAIU, sks (i.e. sks db) can in principle run without sks-recon
>which just means no further reconciliation would take place.
>But sks-recon cannot run without sks db, which is also why
>the dependency should be vice versa, actually Wants isn't
>probably enough, and Requires would be appropriate.
>After/Before is however okay, AFAIU, as db should run before recon.

I thought about this approach (and even tried it at one point) and i
think the logic i ended up going with is the right logic.  let me
explain why:

  * administrators want to "run sks", so the most sensible thing for
them to control is the "sks" service.

  * the main part of the service (the thing without which nothing else
can function) is "sks db".  So this should be stopped or started
when the admin does "systemctl start sks" or "systemctl stop sks"

  * when the main sks db process is running, it *also* wants to have the
recon process up and running, so that it will stay in sync with the
network.  But if the recon process goes down, there's no need to
tear down the db process.  This is expressed by having sks.service
"Wants=sks-recon.service"

  * on the flip side, if the db process goes down, we should terminate
the recon process, and recon shouldn't be started unless db is
already up.  These constraints are expressed in sks-recon.service as:

 BindsTo=sks.service
 After=sks.service

Make sense?

> 3) I think sks.service should become sks-db.service (or perhaps
>sks-hkp.serivce, which makes it more clear to people what it
>actually does, namely providing the actual HKP protocol).
>Additionally there could be a sks.service, which Wants the
>two single services (sks-hkp and sks-recon) and also stops
>both (when stopped).

I find the approach with three .service files more confusing than it
needs to be.  There's the SKS daemon (sks.service) and the
reconciliation server (sks-recon.service).  Given that we can already
express their interdependencies, i don't see what advantage a third
.service would give us.

The admin can already start and stop both services with just:

  systemctl start sks
  systemctl stop sks

> As for the sysvinit style initscripts
> 4) The same splitting into two services (db/hkp and recon)
>should happen for /etc/init.d/sks

I'd love to have someone step up to take care of the sysvinit scripts.
There are quite a few reports in the BTS (e.g. those about
weird/spurious output) that are related to sysvinit, which i'm not sure
how to handle.  I'm much more inclined to work with an initsystem like
systemd by default enforces clean process environments, sensible
filedescriptor inheritance, explicit dependencies between services, etc.

sysvinit makes most of that much harder than it needs to be,
unfortunately, and two interrelated sysvinit scripts sounds even more
troubling than the one.

If you want to offer improvements to the sysvinit scripts (in a separate
bug report, ideally), i'm happy to review it, though i have fewer and
fewer servers like that to test things on.

all the best,

--dkg



Bug#823781: sks: init script issues

2016-05-12 Thread Christoph Anton Mitterer
On Thu, 2016-05-12 at 02:13 -0400, Daniel Kahn Gillmor wrote:
> This is now handled by having the following line in the Unit section
> of
> /lib/systemd/system/sks.service:
> 
> After=local-fs.target network.target 

I've had seen that, but as I wrote in my mail before, it may actually
not even be required (and my original request would thus be wrong).

Perhaps these targets are already implied much earlier, or at least by
multi-user.target... then we could argue, that if any user hooks in
sks.service to some earlier target he probably knows what he's doing.


> 
> > 
> > 2) Right now sks.service wants sks-recon.service, but shouldn't
> >    it be vice versa?
> >    AFAIU, sks (i.e. sks db) can in principle run without sks-recon
> >    which just means no further reconciliation would take place.
> >    But sks-recon cannot run without sks db, which is also why
> >    the dependency should be vice versa, actually Wants isn't
> >    probably enough, and Requires would be appropriate.
> >    After/Before is however okay, AFAIU, as db should run before
> > recon.
> I thought about this approach (and even tried it at one point) and i
> think the logic i ended up going with is the right logic.  let me
> explain why:
> 
>   * administrators want to "run sks", so the most sensible thing for
> them to control is the "sks" service.

That's why my advice would have been to have three services:
sks.service => simply starts/stops/restarts both and is to be used for admins
sks-hkp.service => for sks db
sks-recon.service => for sks recon, likely with a After=sks-hkp.service, and a 
Wants=sks-hkp.servce
 simply because sks-recon.service "needs" hkp to work, but 
won't crash, without it


>   * the main part of the service (the thing without which nothing
> else
> can function) is "sks db".  So this should be stopped or started
> when the admin does "systemctl start sks" or "systemctl stop sks"
> 
>   * when the main sks db process is running, it *also* wants to have
> the
> recon process up and running, so that it will stay in sync with
> the
> network.

Well, this is probably true for the majority of all cases, I agree, but
there could be special use-cases where one doesn't want/need to have
recon running.
Consider e.g. I non-public SKS netowrk (or even just a single service),
with only the company keys) => no need for recon
Or one could think of a server that doesn't do recon but gets his
"updates" in form of regular key dumps from another server.


>   But if the recon process goes down, there's no need to
> tear down the db process.  This is expressed by having
> sks.service
> "Wants=sks-recon.service"

Uhm... I found that a bit strange... if sks.service = sks DB, than it
shouldn't need in principle recon, and having it vice versa, i.e. sks-
recon.service wanting a DB service seems more natural.


>   * on the flip side, if the db process goes down, we should
> terminate
> the recon process, and recon shouldn't be started unless db is
> already up.  These constraints are expressed in sks-recon.service 
> as:
> 
>  BindsTo=sks.service
>  After=sks.service
> 
> Make sense?

Well I have't said that it doesn't work... it just felt a bit unnatural
to me, i.e. not so much directly expressing the actual service
dependencies but doing it rather indirectly.

It probably makes only a difference if you really say, that there's one
sks.service which is conceptually identical to both, running DB and sks
at all (i.e. db and recon).


> > 3) I think sks.service should become sks-db.service (or perhaps
> >    sks-hkp.serivce, which makes it more clear to people what it
> >    actually does, namely providing the actual HKP protocol).
> >    Additionally there could be a sks.service, which Wants the
> >    two single services (sks-hkp and sks-recon) and also stops
> >    both (when stopped).
> I find the approach with three .service files more confusing than it
> needs to be.  There's the SKS daemon (sks.service) and the
> reconciliation server (sks-recon.service).  Given that we can already
> express their interdependencies, i don't see what advantage a third
> .service would give us.
Well right now, AFAIU, I cannot just start DB, can I?
If one says systemctl start sks it will run db and because of
the Wants=sks-recon.service, it will also start recon.


> I'd love to have someone step up to take care of the sysvinit
> scripts.
> There are quite a few reports in the BTS (e.g. those about
> weird/spurious output) that are related to sysvinit, which i'm not
> sure
> how to handle.  I'm much more inclined to work with an initsystem
> like
> systemd by default enforces clean process environments, sensible
> filedescriptor inheritance, explicit dependencies between services,
> etc.

Well I'd probably also say one should rather concentrate on systemd,...
especially perhaps also employing some more confinement (e.g. sks
shouldn't need access to /home).



Cheers,
Chris.

smime.p

Bug#823781: sks: init script issues

2016-05-12 Thread Christoph Anton Mitterer
btw: It seems that -8, (in which sks starts again), the service is not
enabled per default.

I personally think that's good, because I consider Debian's usual
behaviour of enabling services at installation insecure and stupid, but
perhaps you may still want to follow that.


Cheers,
Chris.

smime.p7s
Description: S/MIME cryptographic signature