1172753 - Insufficient protection against PEBKAC in signing servers

Status: RESOLVED WORKSFORME

(Release Engineering :: General, defect)

Description

[hwine]

Thanks to :nthomas for finding this:

    It appeared that the signers had been running as root for some
    months, leaving signing.log* and signing.pid owned by root:root. The
    docs
        https://mana.mozilla.org/wiki/display/RelEng/Signing#Signing-%28Re%29starting
    say to use cltsign.

    I've chown'd the mentioned files to get the servers running, ...
    ... Also, fun bug - server exits silently if it can't
    write to signing.pid, just when you think all is well.

I likely mis-started the servers during the scl3 power work (bug 1160608).

Comment 1

[hwine]

I'm thinking mostly of low hanging fruit, like changing the MOTD. 

If there were a start wrapper, that could holler if run incorrectly, but we start directly. Not sure a code change is worth it, since this mostly seemed to work.

Any other ideas?

Comment 2

[hwine]

Attachment: Change MOTD to emphasize CLTSIGN user

Reinforce the docs.

Comment 3

[Nick Thomas [:nthomas] (UTC+12)]

Comment on attachment 8617057 [details] [diff] [review]
Change MOTD to emphasize CLTSIGN user

How about 'ONLY START SIGNING PROCESSES AS cltsign\n\n....'. Then you get the 'hey you' attention while preserving the proper case for the logon. r+ to land something like this.

Comment 4

[hwine]

Comment on attachment 8617057 [details] [diff] [review]
Change MOTD to emphasize CLTSIGN user

https://hg.mozilla.org/build/puppet/rev/aacdba2d6cf7

Comment 5

[hwine]

Comment on attachment 8617057 [details] [diff] [review]
Change MOTD to emphasize CLTSIGN user

I missed including comment 3 in prior commit

https://hg.mozilla.org/build/puppet/rev/9151ec576637

Comment 6

[hwine]

Changed MOTD deployed.

Leaving open for other cool ideas.

Comment 7

[Nick Thomas [:nthomas] (UTC+12)]

Maybe add this to signing-server.py, just after it starts in script mode.

if getpass.getuser() != 'cltsign':
    log.error("I'm sorry Hal, I can't let do that (without being cltsign)"
    sys.exit(1)

Comment 8

[hwine]

:) go all the way: sys.exit(2001)

Comment 9

[hwine]

Attachment: exit if not user cltsign

simple patch to close long open bug

Comment 10

[Nick Thomas [:nthomas] (UTC+12)]

Comment on attachment 8798932 [details] [diff] [review]
exit if not user cltsign

Review of attachment 8798932 [details] [diff] [review]:
-----------------------------------------------------------------

lgtm

Comment 11

[hwine]

Comment on attachment 8798932 [details] [diff] [review]
exit if not user cltsign

https://hg.mozilla.org/build/tools/rev/13448e3aaea54870baa1e6078ea2c4da45dc301a

Comment 12

[hwine]

Comment on attachment 8798932 [details] [diff] [review]
exit if not user cltsign

r- by catlee -- wants test to be "non root":
 - will ease development
 - expectation is we may have signing servers using different accounts in future

Note: a robust check (uid matches what is configured somewhere and/or nagios checks for correct user) are for the future.

Comment 13

[hwine]

Attachment: exit if root

take 2 -- incorporating catlee's comments

Comment 14

[hwine]

:catlee -- anything left to do here? I'm not the right person to take action on it anymore, so unassigning myself.