730972 - signing server should support multiple token generation passwords and token secrets

Status: RESOLVED FIXED

(Release Engineering :: General, enhancement, P2)

Description

[[e-mail] (:bhearsum)]

This would it easier to cycle out leaked passwords, and maybe even use a different password in different places to limit exposure?

Comment 1

[Chris AtLee [:catlee]]

it should also support multiple token secrets to make transitioning to a new one easier.

Comment 2

[Chris AtLee [:catlee]]

This is implemented here:
https://github.com/catlee/tools/commit/568b5d61b5e6a0f2e1d2655ea94fc1703a70e9f1

I'll attach a patch for review after testing this out in staging, but the patch will be a bit of a mess since I've reorganized files so I could add testing for this feature.

Comment 3

[Chris AtLee [:catlee]]

Attachment: monster patch

sorry :\

Probably easier to look at github for this: https://github.com/catlee/tools/compare/5feb0b9...34d626

The actual implementation of this bug is:
https://github.com/catlee/tools/commit/568b5d61b5e6a0f2e1d2655ea94fc1703a70e9f1

I also implemented support for bug 736466 here:
https://github.com/catlee/tools/commit/11ac68ca92ab38a65bda0dace3c18f7fbe78324a

The rest is reorganization and refactoring to make the new code testable.

Rail did a staging run of the client-side code and it worked. I've tested the server on my system.

Comment 4

[[e-mail] (:bhearsum)]

Comment on attachment 628564 [details] [diff] [review]
monster patch

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

::: lib/python/mozilla_buildtools/test/test_signing_server.py
@@ +96,5 @@
> +        self.server.token_secret = "asdfasdf"
> +        self.assertEquals(True, self.server.verify_token(token, "1.2.3.4"))
> +
> +        # Make sure that using a bad secret to generate the token results in
> +        # failure to validate

This looks like it should be its own test.

@@ +119,5 @@
> +        nonce = self.server.verify_nonce(token, nonce)
> +        self.assertTrue(nonce)
> +
> +        # Make sure that using a bad secret to sign the nonce results in
> +        # failure to validate

Same here.

::: release/signing/signing.py
@@ -1,1 @@
> -import tempfile, os, hashlib, shutil, bz2, re, sys, time, urllib2, httplib

Happy to see the death of this file!

Comment 5

[Mook (work)]

lib/python/signing/utils.py in e7506a606609 appears to have a truncated line 12 (MAC_DESIGNATED_REQUIREMENTS).  (I was looking at it to see what we want to do for our app; thanks for having the thing to look at! :D )

Comment 6

[Chris AtLee [:catlee]]

Nice catch, thanks!

I landed a fix in http://hg.mozilla.org/build/tools/rev/b3f4e5608a9d