369013 - need read-only ssh access to cvs.mozilla.org

Status: RESOLVED FIXED

(mozilla.org Graveyard :: Server Operations, task)

Description

[Robert Helmer [:rhelmer]]

Currently cltbld has read/write access to cvs.mozilla.org, and this key is on all the build machines. Can we get an account on cvs.mozilla.org that we can log into via ssh but only gives us read-only access?

This is necessary for the community network machines, and there is no good reason to have all the build machines have CVS write access.

Finally, this is blocking bhearsum's work as well, as he is working on a test machine and we don't want any risk of it making changes anywhere else (ditto for the staging tinderbox).

Comment 1

[Justin Fitzhugh]

Preed, Aravind and I had a long discussion about this.  It's a bit more complicated than expected and the method to do this will need some testing and repo permission changes.  Bumping the sev to a non-paging level.  Preed says there is a work around and will comment on the bug with it.

Comment 2

[J. Paul Reed [:preed]]

Let's use the local CVS mirror workaround while we get this set up properly.

Comment 3

[J. Paul Reed [:preed]]

While looking at bug 369287, I found this documentation, which sounds like it would be useful for this problem:

CVS tries to set up reasonable file permissions for new directories that are added inside the tree, but you must fix the permissions manually when a new directory should have different permissions than its parent directory. If you set the CVSUMASK environment variable that will control the file permissions which CVS uses in creating directories and/or files in the repository. The value (in octal) is used as a mask against the initial permissions on newly created files. For example, a CVSUMASK value of 066 causes new files to be created without read and write permissions for `group' and `other' (go-rw). CVSUMASK does not affect the file permissions in the working directory; such files have the permissions which are typical for newly created files, except that sometimes CVS creates them read-only (see the sections on watches, section 10.6.1 Telling CVS to watch certain files; -r, section A.4 Global options; or CVSREAD, section C All environment variables which affect CVS).

Note that using the client/server CVS (see section 2.4 Remote repositories), there is no good way to set CVSUMASK; the setting on the client machine has no effect. If you are connecting with rsh or ssh, you can set CVSUMASK in ‘.bashrc’ or ‘.cshrc’, as described in the documentation for your operating system. This behavior might change in future versions of CVS; do not rely on the setting of CVSUMASK on the client having no effect. 

http://www.network-theory.co.uk/docs/cvsmanual/Filepermissions.html

Comment 4

[Aravind Gottipati [:aravind]]

preed, so are we still going ahead with creating a new read only account for build boxes?  Also, during the last meeting, I remember you saying that you'd need to go through and fix existing permissions before we can do any of this stuff.  Is this still needed or should I close this bug?

Comment 5

[J. Paul Reed [:preed]]

(In reply to comment #4)
> preed, so are we still going ahead with creating a new read only account for
> build boxes?

I believe so; we should start with camino, so work w/ Coop on generating a key and an account name (caminobld?)

We'll need one for Thunderbird (tbirdbld?) and Seamonkey (smbld? seabld?) at some point too, and maybe even firefox (ffxbld?), so if it's easy to create all of them upfront, then we can do that.

> Also, during the last meeting, I remember you saying that you'd
> need to go through and fix existing permissions before we can do any of this
> stuff.  Is this still needed or should I close this bug?

Yeah, we probably do, but I can't do that, I don't think. That'll need to be done as root on the CVS box (or are you talking about something different?)

Comment 6

[Chris Cooper [:coop] (he/him)]

FYI, there will be 6 separate product accounts:

calbld
caminobld
ffxbld
seabld
tbirdbld
xrbld

I'll begin posting the public keys right away.

Comment 7

[Chris Cooper [:coop] (he/him)]

Attachment: Public key for calbld user

Comment 8

[Chris Cooper [:coop] (he/him)]

Attachment: Public key for caminobld user

Comment 9

[Chris Cooper [:coop] (he/him)]

Attachment: Public key for ffxbld user

Comment 10

[Chris Cooper [:coop] (he/him)]

Attachment: Public key for seabld user

Comment 11

[Chris Cooper [:coop] (he/him)]

Attachment: Public key for tbirdbld user

Comment 12

[Chris Cooper [:coop] (he/him)]

Attachment: Public key for xrbld user

Comment 13

[Aravind Gottipati [:aravind]]

I created all these accounts with their respective keys.  I also confirmed that these accounts were able to access (read only) the contents of the entire CVSROOT hierarchy. 

Paul:  Since the accounts can read the mozilla hierarchy just fine, I don't see any reason to mess with the directory permissions at this point.

I am also fairly certain that the accounts do not have any write privileges on the box.

You can begin using these accounts for testing and let me know if you notice any problems with them.

Please re-open the bug as needed.

Comment 14

[Chris Cooper [:coop] (he/him)]

CVS checkouts are failing:

cg-xserve03:/builds/tinderbox calbld$ cvs -d :ext:[e-mail]:/cvsroot co mozilla/tools/tinderbox
cvs checkout: Updating mozilla/tools/tinderbox
cvs checkout: failed to create lock directory for `/cvsroot/mozilla/tools/tinderbox' (/cvsroot/mozilla/tools/tinderbox/#cvs.lock): Permission denied
cvs checkout: failed to obtain dir lock in repository `/cvsroot/mozilla/tools/tinderbox'
cvs [checkout aborted]: read lock failed - giving up
cg-xserve03:/builds/tinderbox calbld$

Comment 15

[Aravind Gottipati [:aravind]]

Please try this again.  Adjusted the commitcheck script to explicitly deny commit perms to these users.

Also can you please test that commits don't actually work for them?

Comment 16

[Chris Cooper [:coop] (he/him)]

I can checkout files now. Unfortunately, seabld (probably all the new keys) also has commit access. Witness:

cg-xserve02:/builds/tinderbox/mozilla/tools/performance seabld$ mkdir newdir 
cg-xserve02:/builds/tinderbox/mozilla/tools/performance seabld$ cvs add newdir 
Directory /cvsroot/mozilla/tools/performance/newdir added to the repository
cg-xserve02:/builds/tinderbox/mozilla/tools/performance seabld$ cd newdir 
cg-xserve02:/builds/tinderbox/mozilla/tools/performance/newdir seabld$ touch test_file_please_remove_me
cg-xserve02:/builds/tinderbox/mozilla/tools/performance/newdir seabld$ cvs add test_file_please_remove_me
cvs add: scheduling file `test_file_please_remove_me' for addition
cvs add: use 'cvs commit' to add this file permanently
cg-xserve02:/builds/tinderbox/mozilla/tools/performance/newdir seabld$ cvs commit 
cvs commit: Examining .
RCS file: /cvsroot/mozilla/tools/performance/newdir/test_file_please_remove_me,v
done
Checking in test_file_please_remove_me;
/cvsroot/mozilla/tools/performance/newdir/test_file_please_remove_me,v  <--  test_file_please_remove_me
initial revision: 1.1
done

Needless to say, mozilla/tools/performance/newdir and any files thereunder should not exist and should be removed from the server, if you get the chance.

Comment 17

[Aravind Gottipati [:aravind]]

Deleted the newdir stuff.

This time, I actually created a dummy cvs account and tested my script.  Should have done this the last time I resolved the bug.

You should now be able to checkout using those accounts, but checkins should fail.

Comment 18

[Chris Cooper [:coop] (he/him)]

Aravind, can you please give the new keys read-only access to the l10n repository as well? Thanks.

Comment 19

[Dave Miller [:justdave]]

l10n repo isn't under despot control.  have to come up with a new way to lock them down to pull that off.

Comment 20

[Dave Miller [:justdave]]

easiest way to do this I think is to make sure the repo is only writable to the mozsrc group (or l10nsrc in the case of l10n) and make sure these users are not members of either of those groups.

Comment 21

[Aravind Gottipati [:aravind]]

(In reply to comment #20)
> easiest way to do this I think is to make sure the repo is only writable to the
> mozsrc group (or l10nsrc in the case of l10n) and make sure these users are not
> members of either of those groups.
> 

This didn't work for the regular mozilla repo, the problem was that cvs co processes create lock files, if you cant write to the repo, the co process fails.

Comment 22

[Aravind Gottipati [:aravind]]

coop, reed suggested an alternative approach to this than worrying about who is allowed to checkin, controlling their checkins through pre-commit scripts etc.

We can enable read-only pserver (on a different port than 2401) access to the repos and allow only the community build network access to this.  We could then do some networking magic to translate port 2401 on the community network to the real pserver port on the cvs server.  Would this be acceptable to you guys?

Would this need to be accessable to the outside world in general or is this needed only from the community build network?

Comment 23

[Chris Cooper [:coop] (he/him)]

This only needs to work for the community network. Project builds pulling from other sites can use personal CVS keys or anonymous.

Comment 24

[Aravind Gottipati [:aravind]]

I don't think I want to add the pserver stuff to this.  I will just create a pretty basic cvscheck script and use that instead.  I don't see any real advantage gained complicating the network setup (doing port translations) etc.

Comment 25

[Matthew (lilmatt) Willis]

This is now holding up translation of calendar for 0.5 as we're now in string freeze.  We'd appreciate whatever can be done to get it fixed as soon as possible.

Comment 26

[Aravind Gottipati [:aravind]]

Okay, stuff should finally be in.

We have decided to implement this using the cvs pserver.  So there is no reason for having the read-only bld accounts in cvs.

All these community builds can just use the anonymous user.  If you guys don't mind, I will go ahead and delete all the comminity build accounts I created on the cvs server.

Note:  I will leave these accounts in-tact on stage and the aus-upload servers.

These are the CVSROOTs you will need to use for checkouts.  You will need to login first of course (empty password).

for the mozilla repo,
CVSROOT=:pserver:[e-mail]:/cvsroot

For the l10n repo,
CVSROOT=:pserver:[e-mail]:/l10n

Let me know if you guys don't think this will work.

Thanks to reed, justdave and preed for their suggestions and help in implementing this.

coop:  please comment on this bug indicating if its okay to delete the bld user accounts from CVS.

Comment 27

[Chris Cooper [:coop] (he/him)]

/me goes to recheckout everything on cg-xserve01 and cg-xserve02.

lilmatt: you've got cg-xserve03 under control? Recall that you'll need to recheckout the tinderbox configs and /builds/tinderbox/mozilla from CVS using pserver since they were originally checked out with the new keys.

Comment 28

[Dave Miller [:justdave]]

The more I think about this, the more I think this is a *really* bad idea.  I don't like the idea of having pserver open to that box, even internally.  It only takes one compromised host inside our network, be it a virus accidently downloaded on a Windows tinderbox, or an insecurely configured community machine.  The source code is our bread and butter, and we *cannot* afford to have it compromised.  Being paranoid about the security of that box can only be a good thing.

I still think you guys should pull from cvs-mirror with -D"10 minutes ago" to get around the rsync artifacts.

Comment 29

[Matthew (lilmatt) Willis]

(In reply to comment #27)
> lilmatt: you've got cg-xserve03 under control?

Looks like I do.  I recursively find/replaced :ext:calbld with :pserver:anonymous in all CVS/Root files.  Seems to have worked.


(In reply to comment #28)
> The more I think about this, the more I think this is a *really* bad idea
Is the worry that the *bld user can commit?  It seems like we could use a hook script to prevent that, or perhaps checkout anonymously tunnelled over ssh.

Comment 30

[Aravind Gottipati [:aravind]]

(In reply to comment #28)
> The more I think about this, the more I think this is a *really* bad idea.  I
> don't like the idea of having pserver open to that box, even internally. It
> only takes one compromised host inside our network, be it a virus accidentl y
> downloaded on a Windows tinderbox, or an insecurely configured community
> machine.

dave, its a read-only pserver.  And that too only usable by the anonymous user.  I could not think of any other ways to make it more secure.  I am open to suggestions however.

I originally wanted to just go with regular cvs accounts (ssh access) and make them read only (using explicit controls in the cvscommit.pl script).  reed suggested (and preed agreed) to this pserver solution.  Other than being paranoid, I couldn't come up with any objections to it...

Comment 31

[Aravind Gottipati [:aravind]]

(In reply to comment #30)
> dave, its a read-only pserver.  And that too only usable by the anonymous user.
>  I could not think of any other ways to make it more secure.

Also, if we can upgrade cvs (to 1.20), we can start using the -R option to pserver that explicitly forces it to be read-only.  I have implemented that behavior currently through the writers file.

Comment 32

[Reed Loden [:reed]]

(In reply to comment #28)
> The more I think about this, the more I think this is a *really* bad idea.  I
> don't like the idea of having pserver open to that box, even internally.  It
> only takes one compromised host inside our network, be it a virus accidently
> downloaded on a Windows tinderbox, or an insecurely configured community
> machine.  The source code is our bread and butter, and we *cannot* afford to
> have it compromised.  Being paranoid about the security of that box can only be
> a good thing.

We've got to have a pserver server running on dm-cvs01 for when cvs-www is moved over due to Doctor. It's a bit of a requirement until we have cvs moved over to LDAP, etc.

Is this pserver using the normal pserver stuff, or is it using the chroot cvsd script?

Comment 33

[Aravind Gottipati [:aravind]]

I am using the regular pserver that comes with cvs.

Comment 34

[Dave Miller [:justdave]]

it's not using anoncvssh like cvs-mirror does? (which chroots it)

Comment 35

[Mike Shaver (:shaver emeritus)]

My worry is that pserver has a bad history of remote code exploits, which means that an attacker can run arbitrary code as whatever UID the pserver process runs as.  Such attack code will not, obviously, honour the usual cvs advisory strictures about where and how it can and can't write.

Comment 36

[Dave Miller [:justdave]]

That was my worry as well.  That's the reason we got rid of pserver off that box to begin with (as well as eliminating cleartext passwords and so forth).  pserver for doctor can be handled the same way it is on rheet now, it's locally firewalled so only the doctor server can access it.

Comment 37

[Mark Mentovai]

We were just discussing this on #build.  Moving the community boxes onto pserver has left me without a way to verify the identity of the server supplying me with source code.  I'd like to transition official Camino releases to be built by the new tinderbox on the community network, but I should have a way to trust the source that's going into those official releases as I always did in the past.  Tunnelling cvs in ssh lets me do that (via the known_hosts file).  Spoofable cleartext pserver does not.

Comment 38

[Aravind Gottipati [:aravind]]

I don't see any valid reasons to actually change how this works.  This is why..

Community bld machines are in their own network.

For someone to spoof cvs servers ip, they'd have to mess with dns thats being served from the corp network.  If we do have a compromised corp network, we have bigger problems anyway.

I guess what I am saying is that I don't think this is a valid compromise scenario.

The other situation is what happens with a compromised community build machine - in this case, the machine can put out whatever it wants, authenticated cvs or not.

Comment 39

[Mike Shaver (:shaver emeritus)]

(In reply to comment #38)
> For someone to spoof cvs servers ip, they'd have to mess with dns thats being
> served from the corp network.  If we do have a compromised corp network, we
> have bigger problems anyway.

Are there any firewall rules in place to prevent someone from sending a faked DNS response packet to the build host in question?  (Are they monitored by nagios or some such to make sure they aren't busted by another change?) Targetted DNS attacks are not as hard as we'd generally like them to be, and don't require actually compromising the corporate network.

How would we detect that such an attack had happened?  If someone targets a given community build machine, I don't know that we'd ever see it.  (Is the cvs client robust against hostile server responses?  If not, that's a great escalation path, since the community machines can in fact access a pserver process on our source repository -- even "just" corrupting the mirror server would be quite the prize.)

> I guess what I am saying is that I don't think this is a valid compromise
> scenario.

I'm pretty sure that we can find recorded cases of exactly this happening to lower-profile projects than ours.

Can we maintain a tunnel of some sort via ssh or ssl between the hosts, so that we have _some_ form of authenticated connection to the CVS host?

This whole thing really makes my spidey sense tingle, and the downside risk is _so_ huge.  Not only have we moved from ssh to pserver, and created a brutal escalation path from hacking a random community network box (hacking == "able to run userlevel code on") to being able to bang on a pserver process on our _primary_ source repository, but we've also lost a ton of auditing since all the accounts have been folded down to "anonymous".  I really hope the gain in administrative convenience is massive... :/

Comment 40

[Mark Mentovai]

I don't want to get into a massive reopen-close war here, but I think there's enough contention to justify this bug being open until we've reached true resolution.

> For someone to spoof cvs servers ip, they'd have to mess with dns thats being
> served from the corp network.

It's not really true that that's the only way to spoof pserver.

> The other situation is what happens with a compromised community build machine
> - in this case, the machine can put out whatever it wants, authenticated cvs
> or not.

Which, I thought, was one of the motivations behind tightening access controls anyway.  For example, I'm not so hot on the fact that our new build machine is sitting around with its VNC port wide open for the world to see, and wide open for anyone to sniff out the unencrypted password from, or for anyone to connect to and launch a remote exploit should one be found in the ARD server.  That's not what this bug is about, and I'll deal with those problems separately, but "there are other avenues for abuse" shouldn't really fly as an excuse to not close off other avenues that we know about and can fix.

Comment 41

[J. Paul Reed [:preed]]

I think it's becoming pretty clear that the pserver approach has a number of drawbacks.

The original goal here was to have specific role-based accounts for each project that would allow us to move towards retiring cltbld, and make it such that operations against the repository were made by real people, not role-accounts.

The current implementation, though, has a number of drawbacks:

-- Security concerns about pserver (which seem quite valid, given the history)
-- Identity concerns about the machine, since there's no cryptographic assurance

These seem like unreasonable drawbacks.

What are some other approaches to solve this?

pre-commit script that denies checkins based upon username?

Comment 42

[J. Paul Reed [:preed]]

(In reply to comment #40)

> For example, I'm not so hot on the fact that our new build machine is
> sitting around with its VNC port wide open for the world to see, and wide open
> for anyone to sniff out the unencrypted password from, or for anyone to connect
> to and launch a remote exploit should one be found in the ARD server.  That's
> not what this bug is about, and I'll deal with those problems separately,

Sunbird and Lightning have already changed things on their machines, and instead of everyone reinventing the wheel separately, it'd be nice if we could coordinate and roll the changes out in a standard way, so they're consistent across all the machines, so more than one person can help if there are problems with them.

I've filed bug 376416 to help out with this; let's talk about it there.

Comment 43

[Mark Mentovai]

Is anything in the works here?  I had sort of been waiting on this getting figured out before moving my nightly and release builds to cg-x1.

Comment 44

[Aravind Gottipati [:aravind]]

I originally implemented this using ssh and pre-commit scripts, that changed to a pserver based access after discussions on irc.  I guess we all pretty much want to go back to ssh now.  I will work with reed to roll back the despot fixes and get this back to using ssh accounts.

Comment 45

[Reed Loden [:reed]]

After talking to mento on IRC, he said he would be happy with either an ssh tunnel or a wrapper script for ssh. Either of those ideas will work fine, as it'll allow for authenticated access to the cvs server before using the pserver.

Comment 46

[Aravind Gottipati [:aravind]]

Does this mean, we don't need anything else done on this bug?  If so, can I close it?

Comment 47

[Reed Loden [:reed]]

No, it means what was said in comment #45 needs to be implemented. However, that could be done in another bug, I suppose.

Comment 48

[J. Paul Reed [:preed]]

Where's this wrapper script? We're going to be doing this for every product, including firefox, so if that's the solution, we need something checked in somewhere to use.

I'm not entirely keen about the pserver solution; have we decided that doing it via direct ssh is out?

Comment 49

[Aravind Gottipati [:aravind]]

I am fine with doing it through ssh.  The only downside to that is that it allows for the possibility of creating empty directories throughout the tree.  

My first solution was implemented through ssh before we decided to use pserver.  So if you (build and the other devs) want it, I cant implement it using ssh.

Comment 50

[Aravind Gottipati [:aravind]]

(In reply to comment #49)
Also - fwiw, I think implementing this through ssh is a more secure way to do it.

Comment 51

[Chris Cooper [:coop] (he/him)]

We had it working with direct cvs-over-ssh access via product keys before...let's just go back to that. 

I don't care about the per-product keys being read-only now that we have the community build network firewalled appropriately. We're only handing over access to these machines to trusted developers who *should* know better than to do anything but checkout with these accounts.

I'll need to update the community macs to use ssh again. Can we re-enable ssh access for the product keys, let me switch the Macs back to using ssh from pserver, and _then_ disable the pserver?

Comment 52

[Aravind Gottipati [:aravind]]

I am okay with doing this.  I am going to assume that this is the consensus and implement it.

reed: please roll back the despot changes, so specific users are not allowed to checkin.

Comment 53

[Reed Loden [:reed]]

(In reply to comment #52)
> reed: please roll back the despot changes, so specific users are not allowed to
> checkin.

I really don't want this in Despot. I'll write a small script you can use that's better than editing Despot, and you can just add it as another pre-commit check script. This is needed anyway, as the l10n repo doesn't use Despot, so it'll be easy to add it there, too. Sound good?

Comment 54

[Reed Loden [:reed]]

Also, do you want me to back-out the fix in bug 374282?

Comment 55

[Aravind Gottipati [:aravind]]

The script your write has to be a part of commitcheck somehow.  I don't mind keeping it as a diff script, but thats up to you.

Regd bug 374282, yeah will need to back it out, but we have to co-ordinate that with when we implement the changes, or the users currently using pserver to checkout will break.

Comment 56

[Aravind Gottipati [:aravind]]

Please assign this back to me once the script is ready.

Comment 57

[Reed Loden [:reed]]

Attachment: Read only commit-check script - v1

This is a simple script that has a hardcoded list of users that are not allowed to commit to the repository.

Comment 58

[Reed Loden [:reed]]

Attachment: Read only commit-check script - v2

Better version after comments from aravind.

Comment 59

[Reed Loden [:reed]]

Aravind has committed the new read-only user commit check script for /cvsroot. You should now be able to use SSH to checkout from the repository without having commit access. He is currently adding the script to /l10n now.

Comment 60

[Reed Loden [:reed]]

Aravind has added the script to /l10n. Everything should be ready to be used now. If you have problems, please reopen the bug.