Opened 8 years ago

Last modified 4 years ago

#844 new enhancement

Perforce authentication is misleading

Reported by: pcmantz Owned by:
Priority: patches-accepted Milestone: 0.9.+
Version: 0.7.12 Keywords: p4
Cc: kovarththanan.rajaratnam@…, bugeater327@…

Description

From an email I wrote the other day:

In buildbot/buildbot/changes/p4poller.py, the class takes self.p4user self.p4passwd from the config file. This isn’t exactly what you want, since perforce actually has a certificate based system (called a ticket in perforce lingo). You use your username & password to obtain a certificate via p4 login, then your certificate & username are cached in environment variables, so you can do command-line operations without authenticating manually.

Code wise, it appears that p4poller.py is actually written so that the ticket is your password. The model there would be that you’d log in, throw your ticket in your master.cfg file, then rock and roll. This is fine and dandy for your average user, but not for us.

Our company policy dictates that all perforce tickets must expire after 12 hours. Our problem is that there’s no code in the poller to automate the updating of expired keys, so we would have to manually change the password (actually the ticket) every 12 hours. Buildbot does not poll the environment for the ticket; it collects the password, so it does not work for us.

In order to fix this, we’d need to update the authentication model in p4poller.py to have a preliminary login step. Instead of storing the ticket, the poller would need to call a couple perforce commands, retrieve and cache the ticket, and then use that for authentication.

Attachments (1)

diffs_for_p4tickets.txt (3.4 KB) - added by Bugeater 8 years ago.
Description of change and diffs to enable use of P4 Tickets.

Download all attachments as: .zip

Change History (15)

comment:1 Changed 8 years ago by dustin

  • Milestone changed from undecided to 0.8.1
  • Type changed from undecided to enhancement

This sounds like a good plan.

Will the P4 source step need similar changes?

comment:2 Changed 8 years ago by dustin

  • Keywords p4 added

comment:3 Changed 8 years ago by dustin

  • Milestone changed from 0.8.2 to 0.8.3

Bumping to 0.8.3 since Paul's busy. We don't have a P4 maintainer right now, so this might get dropped altogether unless one comes forward..

comment:4 Changed 8 years ago by krajaratnam

  • Cc kovarththanan.rajaratnam@… added

Changed 8 years ago by Bugeater

Description of change and diffs to enable use of P4 Tickets.

comment:5 Changed 8 years ago by Bugeater

  • Cc bugeater327@… added

Hi,

I just added a diff of the change I made to BuildBot 0.8.1 to resolve this P4 Tickets issue. Info:

  • P4 Tickets can be passed in directly on the command line of p4 but that can be a security issue.
  • P4 Tickets can be passed in via an ENV var "P4TICKETS" that contains a reference to a P4 Tickets file. Much better. I used this option.

The trick is to set the ENV variable "P4TICKETS" before invoking BuildBot so it gets passed in. Then all you need to do is assure that the tickets in the P4 Tickets file are valid.

Could this be ready for 0.8.2?

TVB

comment:6 Changed 8 years ago by dustin

Paul, what do you think?

comment:7 Changed 8 years ago by dustin

  • Milestone changed from 0.8.3 to 0.8.2

comment:8 Changed 8 years ago by pcmantz

I haven't tested this patch, but it doesn't appear to address the nomenclature issue in the perforce poller; i.e., that what is called self.p4passwd in the poller is actually the ticket.

To really solve this issue as described, the poller needs to authenticate and retrieve a new ticket when the old one expires using the *real* perforce password. This could be settled with a cron job, for instance, but it would be better if this were handled directly by the poller.

There appears to be a project for perforce bindings in native python located at http://code.google.com/p/python-p4lib/ which may prove fruitful if people think that wrapping comman-line calls is too nasty. However, I haven't even looked at the code; this is only a suggestion.

comment:9 Changed 8 years ago by Bugeater

pcmantz,

At my company they generate long life tickets for the build user. That's why it was important to keep them secure.

So no. This does not address the issue directly, but instead provides a possibly suitable workaround. Instead of putting the actual ticket into self.p4passwd, putting the ticket file location in the env var will get the same result and no one will ever see the actual ticket.

Although it is possible to feed "p4 login" a password from stdin, it still exposes that password to at least some degree. In our case, the build machines are relatively inaccessible so keeping the P4 Tickets file there isn't too bad.

If you setup a cron job on the machine to twice daily to a "p4 login" with the password on stdin and placing the P4 Tickets file into the correct place, then even with 24 hour tickets would be workable.

TVB

comment:10 Changed 8 years ago by dustin

Fair enough!

Two questions, then:

  1. can you update the patch to apply against the latest version?
  2. can you add a paragraph or two to the texinfo documentation describing the solution you recommended?

comment:11 Changed 8 years ago by dustin

  • Milestone changed from 0.8.2 to 0.8.3

bumping

comment:12 Changed 8 years ago by dustin

  • Milestone changed from 0.8.3 to 0.8.+

comment:13 Changed 6 years ago by tom.prince

  • Priority changed from major to patches-accepted

comment:14 Changed 4 years ago by dustin

  • Milestone changed from 0.8.+ to 0.9.+

Ticket retargeted after milestone closed

Note: See TracTickets for help on using tickets.