Last Comment Bug 706832 - Implement master side token generation for signing on demand
: Implement master side token generation for signing on demand
Status: RESOLVED FIXED
:
Product: Release Engineering
Classification: Other
Component: Other (show other bugs)
: other
: x86_64 Linux
: P2 normal (vote)
: ---
Assigned To: Rail Aliiev [:rail]
:
Mentors:
Depends on:
Blocks: 509158
  Show dependency treegraph
 
Reported: 2011-12-01 07:26 PST by Rail Aliiev [:rail]
Modified: 2013-08-12 21:54 PDT (History)
1 user (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
token generation code (9.76 KB, patch)
2011-12-01 07:26 PST, Rail Aliiev [:rail]
bhearsum: review+
catlee: review+
Details | Diff | Review
token generation code (11.17 KB, patch)
2011-12-02 05:25 PST, Rail Aliiev [:rail]
catlee: review+
Details | Diff | Review
token generation code (11.16 KB, patch)
2011-12-02 12:48 PST, Rail Aliiev [:rail]
rail: review+
rail: checked‑in+
Details | Diff | Review

Description Rail Aliiev [:rail] 2011-12-01 07:26:45 PST
Created attachment 578257 [details] [diff] [review]
token generation code

Requirements:

1) master talks to the signing server over https
2) master validates https cert
3) master uses user/password http authentication over https
4) custom step should support multiple signing servers and should retry n times before switching to another server from the list
5) token should be downloaded from master to slave

Please, see attached patch.
Comment 1 Chris AtLee [:catlee] 2011-12-01 07:47:50 PST
Comment on attachment 578257 [details] [diff] [review]
token generation code

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

r+ with these fixed

::: process/factory.py
@@ +1130,5 @@
> +        server_cert = os.path.join(
> +            os.path.dirname(build.paths.__file__),
> +            '../../../release/signing/host.cert')
> +        token = WithProperties('%(basedir)s/build/token')
> +        nonce = WithProperties('%(basedir)s/build/nonce')

I think we can use 'build/token', 'build/nonce' here without properties

@@ +1143,5 @@
> +        ))
> +        self.addStep(SigningServerAuthenication(
> +            servers=self.signingServers,
> +            server_cert=server_cert,
> +            s="",

this is a bit weird. can we make SigningServerAuthentication use a default empty string?

::: steps/signing.py
@@ +114,5 @@
> +    def downloadSignature(self, res):
> +        self.s = res
> +        StringDownload.start(self)
> +
> +    def interrupt(self, reason='Interrupted'):

Should you be calling the base class's interrupt here as well?
Comment 2 Rail Aliiev [:rail] 2011-12-02 05:25:31 PST
Created attachment 578556 [details] [diff] [review]
token generation code

* Address comments
* l10n repacks work now
Comment 3 Rail Aliiev [:rail] 2011-12-02 12:48:26 PST
Created attachment 578686 [details] [diff] [review]
token generation code

* moved token generation after the compile step
* tested in staging
* keeping r+
Comment 4 Rail Aliiev [:rail] 2011-12-02 13:07:59 PST
Comment on attachment 578686 [details] [diff] [review]
token generation code

http://hg.mozilla.org/build/buildbotcustom/rev/202dfd424ea1

Note You need to log in before you can comment on or make changes to this bug.