Bug 181507

Comment 1 Edwin Groothuis 2013-08-24 19:20:07 UTC

State Changed
From-To: open->feedback

Awaiting maintainers feedback (via the GNATS Auto Assign Tool)

Comment 2 Edwin Groothuis 2013-08-24 19:20:07 UTC

Maintainer of security/pks,

Please note that PR ports/181507 has just been submitted.

If it contains a patch for an upgrade, an enhancement or a bug fix
you agree on, reply to this email stating that you approve the patch
and a committer will take care of it.

The full text of the PR can be found at:
    http://www.freebsd.org/cgi/query-pr.cgi?pr=ports/181507

-- 
Edwin Groothuis via the GNATS Auto Assign Tool
edwin@FreeBSD.org

Thanks for this report. The good news is that folks are still using pks 
and it builds with STAGE :-)

I can confirm this bug, however I don't think the patch as is should be 
accepted.

The use of 'daemon' seems a bit like a workaround for something that isn't 
working quite correctly, but which I have been unable to track down. I 
believe the issue appeared after there were changes made in the rc.subr 
system and pksd.sh moved to pksd.

The patch Tassilo submitted does fix the start on boot issue but breaks 
onestart/onestatus/onestop behaviour. Setting a ${pidfile} fixes this 
follow on issue (but doesn't feel like a complete solution either since I 
am not sure why it changes the behaviour the way it does). The use of 
"daemon" adds another running process after startup but I suppose this is 
acceptable tradeoff.

In the next few days I will submit a modified patch that corrects the 
start/onestart behaviours and a couple of other issues with the sample 
configuration file and the rc script.

I will use the present PR (ports/181507) to track the changes.

Thanks for your report!

Cheers

On Sat, 24 Aug 2013, Edwin Groothuis wrote:

> Maintainer of security/pks,
>
> Please note that PR ports/181507 has just been submitted.
>
> If it contains a patch for an upgrade, an enhancement or a bug fix
> you agree on, reply to this email stating that you approve the patch
> and a committer will take care of it.
>
> The full text of the PR can be found at:
>    http://www.freebsd.org/cgi/query-pr.cgi?pr=ports/181507

Thanks for looking into this.

> Thanks for this report. The good news is that folks are still using pks 
> and it builds with STAGE :-)

Indeed - I'm using it, but have my doubts for the future. The entire key-server infrastructure as it exists currently blows a bit, to be honest: why don't they relay requests to each other, but have some weird syncing, and why does gnupg lookup one only, etc..

And the only alternative to pks seems to be unnecessarily bloated. Oh well..

So yes, there are people still using pks ;)


> I can confirm this bug, however I don't think the patch as is should be 
> accepted.
> 
> The use of 'daemon' seems a bit like a workaround for something that isn't 
> working quite correctly, but which I have been unable to track down. I 
> believe the issue appeared after there were changes made in the rc.subr 
> system and pksd.sh moved to pksd.

Hm, I wasn't aware of that, I thought daemon was actually the way it was intended to be used...


> The patch Tassilo submitted does fix the start on boot issue but breaks 
> onestart/onestatus/onestop behaviour.

That's odd - I'm sure I used the one* commands with my patch... *scratches head*. Oh well, I ran into this 6 months ago, so maybe I'm not remembering anymore.


> Setting a ${pidfile} fixes this 
> follow on issue (but doesn't feel like a complete solution either since I 
> am not sure why it changes the behaviour the way it does). The use of 
> "daemon" adds another running process after startup but I suppose this is 
> acceptable tradeoff.

Thanks for looking into this in more detail.


> In the next few days I will submit a modified patch that corrects the 
> start/onestart behaviours and a couple of other issues with the sample 
> configuration file and the rc script.
> 
> I will use the present PR (ports/181507) to track the changes.

Highly appreciated, thanks!

Comment 5 Baptiste Daroussin 2014-06-04 16:51:36 UTC

Any news here?

Created attachment 144298 [details]
STAGIFY and tweak startup files for pks

Patch created using subversion port diff.

Hi Sorry for the delay on this. Attached above is a somewhat rushed svn diff so this stays alive after the move to STAGE. 

1. I have compiled using STAGE on 9.2 and 10.0 and everything seems to work fine. Thanks for the work on this feature of ports, and thanks for using bugzilla!

2.  On two machines I am able to test on there seems to be corruption issues when using db42.  I set USE_DB to 41.

3. There was a an error in the test condition of the start_postcmd which
caused the postcmd to never run. If the postcmd (pks-queue-run.sh) does
run, it gets stuck in a while loop and doesn't exit properly. This issue
appears to be related to configuration so, rather than adding patches for
the upstream source, I think it is best to disable the start_postcmd and
document the required installation steps for enabling it.

4. Setting a ${pidfile} fixed a start/stop issue but I can't remember the specifics. In any case this doesn't feel like a complete solution to the reported bug since I am not sure why it changes the behavior the way it does.

Beyond this patch these are the  TODOs for this port:

- the port needs to install its own UID/GID and run with those privileges. At one point I had this mostly done but not well tested.  Feel free to take this on.

- the configuration file and the rc.d script should by default disable 
interaction by mail and encourage the administrator (with installation
messages) to correctly configure their pks installation to work with the 
local mail infrastructure BEFORE running rc.d scripts which rely on it. [MOSTLY DONE ?] 

- the port needs to be easy to set up to run chrooted using rc.conf 
and have a better default chroot location set in the sample configuration (but
continue to default chroot to off of course).

Please test! security/pks needs to use the facilities of the new improved ports system, rc.subr. rc.conf to build and install an easy to install binary pkg in a reliable way to stay useful.  pks is a simple BSD licensed key management service that might fit nicely into a larger project, but to stay relevant for the longer term support for new key formats (JPEG images etc.) and/or alternative DB backends would be nice to have. pks was a very useful tool for internal key services I ran in the past. Since I do not run a key service of any kind currently, new maintainers/developers are welcome and encouraged.

Comment 8 John Marino 2014-07-28 12:49:25 UTC

Ok, I think G.Todd said it's ready to commit. ;)

Comment 9 John Marino 2014-07-30 19:27:32 UTC

It's pretty much not stage-safe at all

Tassilo, since you like stage so much, so you want to update the patch?

===========================================================================
====> Running Q/A tests (stage-qa)
Warning: 'bin/pksclient' is not stripped consider using ${STRIP_CMD}
Warning: 'bin/pgpsplit' is not stripped consider using ${STRIP_CMD}
Warning: 'bin/pksdctl' is not stripped consider using ${STRIP_CMD}
Warning: 'sbin/pksd' is not stripped consider using ${STRIP_CMD}
====> Checking for pkg-plist issues (check-plist)
===> Parsing plist
===> Checking for items in STAGEDIR missing from pkg-plist
Error: Orphaned: man/man5/pksd.conf.5.gz
Error: Orphaned: man/man8/pgpsplit.8.gz
Error: Orphaned: man/man8/pks-intro.8.gz
Error: Orphaned: man/man8/pks-mail.sh.8.gz
Error: Orphaned: man/man8/pks-queue-run.sh.8.gz
Error: Orphaned: man/man8/pksclient.8.gz
Error: Orphaned: man/man8/pksd.8.gz
Error: Orphaned: man/man8/pksdctl.8.gz
Error: Orphaned: /var/pks/index.html
Error: Orphaned: @unexec rmdir "/var/pks/db" >/dev/null 2>&1 || :
Error: Orphaned: @unexec rmdir "/var/pks/incoming" >/dev/null 2>&1 || :
Error: Orphaned: @unexec rmdir "/var/pks" >/dev/null 2>&1 || :
===> Checking for directories owned by MTREEs
===> Checking for directories handled by dependencies
===> Checking for items in pkg-plist which are not in STAGEDIR
Error: Missing: %%DOCSDIR%%/mail_intro
Error: Missing: %%DOCSDIR%%/pks_help.cz
Error: Missing: %%DOCSDIR%%/pks_help.de
Error: Missing: %%DOCSDIR%%/pks_help.en
Error: Missing: %%DOCSDIR%%/pks_help.es
Error: Missing: %%DOCSDIR%%/pks_help.fr
Error: Missing: %%DOCSDIR%%/pks_help.ja
===> Error: Plist issues found.

You got me ;)
But sure, I can look at it, happy to help out.

So, just to make sure, I understand the current state and work needed... I need to:

1) start with the latest version of the port
2) apply G. Todd's patch that is in this pr
3) fix the problems you mentioned
4) create new patch and upload to this pr

Correct?

Comment 11 John Marino 2014-07-30 20:20:36 UTC

(In reply to tphilipp from comment #10)
> 1) start with the latest version of the port
> 2) apply G. Todd's patch that is in this pr
> 3) fix the problems you mentioned
> 4) create new patch and upload to this pr


Yes.  the patch should apply without issue.  Just make the proper fixes and generate a new patch (a combination of his work plus your fixes).

We're on the same page.
Thanks!

Created attachment 145200 [details]
pks diff with pkglist fixes

Here you go...
- I fixed the pkg-plist depending on what needed to be changed for the changes that came with staging support
- I also switched LICENSE=BSD to LICENSE=BSD4CLAUSE, however, please double check if that is correct

Comment 13 John Marino 2014-08-01 10:57:51 UTC

okay thanks. The stripping warnings weren't addressed though?  (Just asking)

Comment 14 John Marino 2014-08-01 11:01:12 UTC

Is it true this port is limited to BDB 41 or 42?  That seems kind of antiquated.  In any case, the makefile hardcodes to 41 so it needs to allow for 42 if that's already installed.

It would be great to understand the real requirement of BDB on this port.

(In reply to John Marino from comment #13)
> okay thanks. The stripping warnings weren't addressed though?  (Just asking)

Oh dang... sorry, I totally spaced those 4 warnings. Let me create another patch, tonight, or tomorrow.

About BDB 41,42 - not sure what the requirements are, pks itself is kinda antiquated, maybe G.Todd has an idea?
I can check that, also, to at least allow the port to use either 41 or 42, if the latter is installed.

Comment 16 John Marino 2014-08-07 08:43:02 UTC

ping?

Sorry, simply haven't gotten around it, yet... last days have been busier than expected.

Hi again,
I'm still working on it, wanted to share a few insights, though:
- the port does indeed only work with BDB 41 and 42
- with BDB 43 and greater, pks doesn't compile
- I'm not sure how to tell USE_BDB to use those 2 versions, only, as I only see support for '+', so version X or greater (e.g. 41+)

Also, the port doesn't work for me in the current version (on either BDB 41 or 42) - when I start pksd I get: "txn_checkpoint interface requires an environment configured for the transaction subsystem"
This is logged from bdb if I'm not mistaken, but I don't understade how to configure the transaction subsystem...
any idea?

Comment 19 John Marino 2014-08-10 08:05:22 UTC

(In reply to tphilipp from comment #18)
> - the port does indeed only work with BDB 41 and 42
> - with BDB 43 and greater, pks doesn't compile
> - I'm not sure how to tell USE_BDB to use those 2 versions, only, as I only
> see support for '+', so version X or greater (e.g. 41+)

My reading of Mk/bsd.database.mk where all these terms are defined, there are only 3 possibilities:

USE_BDB=42            (This would exclude 41, so not great)
WANT_BDB_VERSION=42   (This one overrides WITH_BDB_VER so it's probably better than USE_BDB, but still limited to 1 version AFAICT)

You could use INVALID_BDB_VER= 40 43 44 45 46 47 48 50 along with USE_BDB=yes

 
> Also, the port doesn't work for me in the current version (on either BDB 41
> or 42) - when I start pksd I get: "txn_checkpoint interface requires an
> environment configured for the transaction subsystem"
> This is logged from bdb if I'm not mistaken, but I don't understade how to
> configure the transaction subsystem...
> any idea?


No, I have no idea.
At some point ports that only work with old version of BDB are going to be deprecated.  Weren't you using pks before?   Since you opened the PR?  Has something changed on your side?

(In reply to John Marino from comment #19)
> (In reply to tphilipp from comment #18)
> > - the port does indeed only work with BDB 41 and 42
> > - with BDB 43 and greater, pks doesn't compile
> > - I'm not sure how to tell USE_BDB to use those 2 versions, only, as I only
> > see support for '+', so version X or greater (e.g. 41+)
> 
> My reading of Mk/bsd.database.mk where all these terms are defined, there
> are only 3 possibilities:
> 
> USE_BDB=42            (This would exclude 41, so not great)
> WANT_BDB_VERSION=42   (This one overrides WITH_BDB_VER so it's probably
> better than USE_BDB, but still limited to 1 version AFAICT)
> 
> You could use INVALID_BDB_VER= 40 43 44 45 46 47 48 50 along with USE_BDB=yes

Well, the INVALID_BDB_VER would fail for new versions of BDB... :(

I checked all the tree's USE_BDB statements, and other than 'yes' or '5' there are a bunch of '40+'. And from all the ones picking a specific version out of 4x, it's all '41+', '42' and '42+', not a single time '41'. The ones picking '41+' are mail/{bogofilter,dk-milter,drac,evolution-exchange} and comms/xastir, but as said, it's '41+', not a single '41'.
In other words, it's very likely that if a matching version for security/pks is already installed on the system, it'll be 42. Should I simply set USE_BDB=42 then, ignoring 41?


> > Also, the port doesn't work for me in the current version (on either BDB 41
> > or 42) - when I start pksd I get: "txn_checkpoint interface requires an
> > environment configured for the transaction subsystem"
> > This is logged from bdb if I'm not mistaken, but I don't understade how to
> > configure the transaction subsystem...
> > any idea?
> 
> No, I have no idea.
> At some point ports that only work with old version of BDB are going to be
> deprecated.

Well, pks is unfortunately pretty much unmaintained for years. There's work underway to use DANE to locate and use a public key through DNS, which is (from what I can tell) a much better solution anyways, but until that's done, I don't see a lot of alternatives.
For current use, there's security/sks, but from what I see, it's bloated compared to security/pks. The former's bloat comes from dependencies on X11 through USE_OCAML which is lang/ocaml and maybe should be lang/ocaml-nox11? I know, I went off on a tangent, here ;)

Back to the subject - not sure what to do with security/pks once older BDB versions are phased out and dependents are flagged as deprecated. The real solution, if security/pks is kept, to probably just fix it to work with newer BDB versions.

Given that the pgp, etc. can not to my know round-robin through a list of keyservers specified in the config file, pretty much kills alternative, non-syncing keyservers other than the main one's like MIT's, anyways. Except for very dedicated use, of course.


> Weren't you using pks before?   Since you opened the PR?  Has
> something changed on your side?

That's what is puzzling me. I use pks, and still run it (http://cumulus.hopto.org:11371/), but that's the version from when I wrote this PR, and it's running on a different machine than the one I was working on yesterday. So far I don't see the difference.
I don't recall having had this problem back then, though... the only thing I find about it online is to db_recover, but that doesn't seem to do anything.

I guess I'll have to dive into the source ;)

At this point, should I take over maintainership? G.Todd was actually also welcoming that idea, as he isn't actively using pks anymore.

Comment 21 John Marino 2014-08-10 17:10:14 UTC

(In reply to tphilipp from comment #20)
> (In reply to John Marino from comment #19)
> > You could use INVALID_BDB_VER= 40 43 44 45 46 47 48 50 along with USE_BDB=yes
> 
> Well, the INVALID_BDB_VER would fail for new versions of BDB... :(

You could set 51, 52, 52, ... 75 to pre-emptively kill all new versions even before they exist, terminator-style.

> 
> I checked all the tree's USE_BDB statements, and other than 'yes' or '5'
> there are a bunch of '40+'. And from all the ones picking a specific version
> out of 4x, it's all '41+', '42' and '42+', not a single time '41'. The ones
> picking '41+' are mail/{bogofilter,dk-milter,drac,evolution-exchange} and
> comms/xastir, but as said, it's '41+', not a single '41'.
> In other words, it's very likely that if a matching version for security/pks
> is already installed on the system, it'll be 42. Should I simply set
> USE_BDB=42 then, ignoring 41?

41 is the still the default, so 41 is more likely to be on the system.  That's why INVALID_BDB_VER is the most technically correct option.


> Back to the subject - not sure what to do with security/pks once older BDB
> versions are phased out and dependents are flagged as deprecated. The real
> solution, if security/pks is kept, to probably just fix it to work with
> newer BDB versions.

That's the best approach.

> I guess I'll have to dive into the source ;)
> 
> At this point, should I take over maintainership? G.Todd was actually also
> welcoming that idea, as he isn't actively using pks anymore.

I'm sure everyone is happy to let you assume maintainership.

Created attachment 145642 [details]
final (?) patch of discussed changes and taking over maintainership

Hi, I have good news, bad news and kinda good ones...


Good: I got it to work. My problem was a typo I had in my config file in combination with pksd sometimes not creating the database files on first start. So, the typo was a wrong userid for a user that didn't exist on the system. So this lead to permission problems accessing the db files ('truss' for the win). Once I figured that out I had it working, briefly, and my then final test did not even create the db files anymore. I never figured out, why - but it seems to do just fine to use 'pksclient /path/to/db create', pksclient being part of the port.
Maybe it's actually always intended to be used that way? I could swear the daemon created the db by itself at least once... So for now, I added this to pkg-message as littler helping notice.


Bad: It does build with BDB 42, but dumps core when adding a key:

Aug 10 20:11:15 gallon pksd[10796]: pksd: listener: pksd: host localhost/127.0.0.1 connected
Aug 10 20:11:15 gallon pksd[10796]: pksd: db_errcall: PANIC: fatal region error detected; run recovery
Aug 10 20:11:15 gallon pksd[10796]: pksd: db_errcall: PANIC: fatal region error detected; run recovery
Aug 10 20:11:16 gallon kernel: pid 10796 (pksd), uid 0: exited on signal 11 (core dumped)

DB itself doesn't seem to need recovery after this, actually... however the bigger problem is of course that the daemon crashes everytime, so it's unusable with BDB 42.

Kinda good: this means that the port is effectively locked to BDB 41 for now, which is ancient, but well... at least for the current state, the Makefile stays simple in that regard.


Find attached the patch with the stripping and extra-notes.

Comment 23 John Marino 2014-08-11 05:43:42 UTC

Well, being locking into DBD 41 exclusively does make the port makefile a bit cleaner but do you think WANT_BDB_VERSION would be better than USE_BSD?

Everything else looks okay to me.

(In reply to John Marino from comment #23)
> Well, being locking into DBD 41 exclusively does make the port makefile a
> bit cleaner but do you think WANT_BDB_VERSION would be better than USE_BSD?
> 
> Everything else looks okay to me.

Not sure what the difference is between USE_BDB=version and USE_BDB=yes + WANT_BDB_VER=version. I thought the former was the more modern way?

Let me know if I should change it over.

Thanks

Comment 25 John Marino 2014-08-11 14:31:39 UTC

read Mk/bsd.databases.mk.  I think the WANT prevents user settings from installing a definite BDB.  "WANT" is stronger as I read it (and the code itself)

Created attachment 145664 [details]
final patch

makes sense - find attached the final patch, then

Created attachment 145665 [details]
final (sorry, previous one was a mistake)

sorry, picked wrong file - now this is the correct one

Comment 28 commit-hook 2014-08-11 21:35:58 UTC

A commit references this bug:

Author: marino
Date: Mon Aug 11 21:34:58 UTC 2014
New revision: 364668
URL: http://svnweb.freebsd.org/changeset/ports/364668

Log:
  Stage security/pks and pass maintainership to submitter

  PR:		181507
  Submitted by:	Tassilo Philipp
  Approved by:	former maintainer (G. Todd)

Changes:
  head/security/pks/Makefile
  head/security/pks/files/EMAIL
  head/security/pks/files/patch-mkpksdconf.in
  head/security/pks/files/pkg-message.in
  head/security/pks/files/pksd.in
  head/security/pks/pkg-plist

Comment 29 John Marino 2014-08-11 21:36:34 UTC

Thanks guys!  I am so glad not to see this in my inbox anymore. :)

Thanks to you too, and to huge G.Todd, for the actual fix of this issue!