Launchpad itself

Merge lp:~stub/launchpad/oauth-db into lp:launchpad

  1. oauth-db
  2. Merge into devel
Proposed by Stuart Bishop
Status: Rejected
Rejected by: Stuart Bishop
Proposed branch: lp:~stub/launchpad/oauth-db
Merge into: lp:launchpad
Diff against target: 436 lines (+55/-169)
7 files modified
lib/canonical/launchpad/database/oauth.py (+44/-36)
lib/canonical/launchpad/database/tests/test_oauth.py (+2/-8)
lib/canonical/launchpad/doc/oauth.txt (+8/-26)
lib/canonical/launchpad/interfaces/oauth.py (+1/-16)
lib/canonical/launchpad/scripts/garbo.py (+0/-38)
lib/canonical/launchpad/scripts/tests/test_garbo.py (+0/-41)
lib/canonical/launchpad/zcml/oauth.zcml (+0/-4)
To merge this branch: bzr merge lp:~stub/launchpad/oauth-db
Reviewer Review Type Date Requested Status
Canonical Launchpad Engineering Pending
Review via email: mp+27025@code.launchpad.net

Description of the change

The OAuthNonce table is our most heavily updated table by an order of magnitude and responsible for the majority of our replication load. It generally sits at 40 updates per second and spikes at over two hundred for short periods.

We maintain the nonce seen history per the OAuth spec to help prevent replay attacks. There is no need for this information to be stored in a relational database at all - it is overkill. Memcache seems a better storage mechanism.

If memory pressure causes memcached to prematurely evict nonce history, users will not notice - what we lose is the replay protection. Monitoring is being put in place so we will see when evictions start happening and we can add RAM to our memcached instances or investigate alternative storages.

To post a comment you must log in.
lp:~stub/launchpad/oauth-db updated
5590. By Stuart Bishop

Better expiry to avoid memcached pressure

5591. By Stuart Bishop

Betterer expiry

5592. By Stuart Bishop

Shorten keys

Unmerged revisions

5592. By Stuart Bishop

Shorten keys

5591. By Stuart Bishop

Betterer expiry

5590. By Stuart Bishop

Better expiry to avoid memcached pressure

5589. By Stuart Bishop

Drop OAuthNonce DB class, replacing its use with memcached

5588. By Stuart Bishop

Log OAuthNonces in memcached rather than PostgreSQL

5587. By Stuart Bishop

Merge from lp:~launchpad-pqm/launchpad/devel

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'lib/canonical/launchpad/database/oauth.py'
2--- lib/canonical/launchpad/database/oauth.py 2010-04-19 09:39:29 +0000
3+++ lib/canonical/launchpad/database/oauth.py 2010-06-08 12:11:25 +0000
4@@ -6,16 +6,16 @@
5 'OAuthAccessToken',
6 'OAuthConsumer',
7 'OAuthConsumerSet',
8- 'OAuthNonce',
9 'OAuthRequestToken',
10 'OAuthRequestTokenSet']
11
12-import pytz
13+import time
14 from datetime import datetime, timedelta
15
16 from zope.component import getUtility
17 from zope.interface import implements
18
19+from pytz import UTC
20 from sqlobject import BoolCol, ForeignKey, StringCol
21 from storm.expr import And
22
23@@ -32,8 +32,9 @@
24 from lp.registry.interfaces.projectgroup import IProjectGroup
25 from lp.registry.interfaces.distributionsourcepackage import (
26 IDistributionSourcePackage)
27+from lp.services.memcache.interfaces import IMemcacheClient
28 from canonical.launchpad.interfaces import (
29- IOAuthAccessToken, IOAuthConsumer, IOAuthConsumerSet, IOAuthNonce,
30+ IOAuthAccessToken, IOAuthConsumer, IOAuthConsumerSet,
31 IOAuthRequestToken, IOAuthRequestTokenSet, NonceAlreadyUsed,
32 TimestampOrderingError, ClockSkew)
33 from canonical.launchpad.webapp.interfaces import (
34@@ -46,7 +47,7 @@
35 # timestamp "MUST be equal or greater than the timestamp used in previous
36 # requests," but this is likely to cause problems if the client does request
37 # pipelining, so we use a time window (relative to the timestamp of the
38-# existing OAuthNonce) to check if the timestamp can is acceptable. As
39+# existing nonce) to check if the timestamp can is acceptable. As
40 # suggested by Robert, we use a window which is at least twice the size of our
41 # hard time out. This is a safe bet since no requests should take more than
42 # one hard time out.
43@@ -87,7 +88,7 @@
44 def newRequestToken(self):
45 """See `IOAuthConsumer`."""
46 key, secret = create_token_key_and_secret(table=OAuthRequestToken)
47- date_expires = (datetime.now(pytz.timezone('UTC'))
48+ date_expires = (datetime.now(UTC)
49 + timedelta(hours=REQUEST_TOKEN_VALIDITY))
50 return OAuthRequestToken(
51 consumer=self, key=key, secret=secret, date_expires=date_expires)
52@@ -161,35 +162,52 @@
53
54 def checkNonceAndTimestamp(self, nonce, timestamp):
55 """See `IOAuthAccessToken`."""
56- timestamp = float(timestamp)
57- date = datetime.fromtimestamp(timestamp, pytz.UTC)
58+ date = float(timestamp)
59 # Determine if the timestamp is too far off from now.
60- skew = timedelta(seconds=TIMESTAMP_SKEW_WINDOW)
61- now = datetime.now(pytz.UTC)
62+ skew = TIMESTAMP_SKEW_WINDOW
63+ now = time.time()
64 if date < (now-skew) or date > (now+skew):
65 raise ClockSkew('Timestamp appears to come from bad system clock')
66+
67+ # Have we seen this nonce before with this token?
68+ # Pull the nonce details, if they exists, from memcached.
69+ cache = getUtility(IMemcacheClient)
70+ nonce_seen_key = 'oauth:t_%s:n_%s' % (
71+ self.key.encode('US-ASCII'), nonce.encode('US-ASCII'))
72+ nonce_seen = cache.get(nonce_seen_key)
73+
74 # Determine if the nonce was already used for this timestamp.
75- store = OAuthNonce.getStore()
76- oauth_nonce = store.find(OAuthNonce,
77- And(OAuthNonce.access_token==self,
78- OAuthNonce.nonce==nonce,
79- OAuthNonce.request_timestamp==date)
80- ).one()
81- if oauth_nonce is not None:
82+ if nonce_seen == date:
83 raise NonceAlreadyUsed('This nonce has been used already.')
84+
85 # Determine if the timestamp is too old compared to most recent
86 # request.
87- limit = date + timedelta(seconds=TIMESTAMP_ACCEPTANCE_WINDOW)
88- match = store.find(OAuthNonce,
89- And(OAuthNonce.access_token==self,
90- OAuthNonce.request_timestamp>limit)
91- ).any()
92- if match is not None:
93+ token_seen_key = 'oauth:t_%s:seen' % self.key.encode('US-ASCII')
94+ token_seen = cache.get(token_seen_key)
95+
96+ limit = date + TIMESTAMP_ACCEPTANCE_WINDOW
97+ if token_seen > limit:
98 raise TimestampOrderingError(
99 'Timestamp too old compared to most recent request')
100- # Looks OK. Give a Nonce object back.
101- return OAuthNonce(
102- access_token=self, nonce=nonce, request_timestamp=date)
103+
104+ # Looks OK.
105+
106+ # No need to remember things beyond
107+ # TIMESTAMP_ACCEPTANCE_WINDOW * 2 seconds
108+ # Where n==TIMESTAMP_ACCEPTANCE_WINDOW, n * 2 is the worst case.
109+ # We will accept a timestamp claiming to be from n seconds in
110+ # the future. If, in n*2 seconds time we receive the same
111+ # timestamp, it would still be withing the acceptable skew
112+ # and we need to generate an 'already seen' exception.
113+ expires = TIMESTAMP_SKEW_WINDOW * 2 + TIMESTAMP_ACCEPTANCE_WINDOW
114+
115+ # Remember when we saw this nonce to avoid replay attacks.
116+ cache.set(nonce_seen_key, date, expires)
117+
118+ # Remember when we last checked a nonce for this access
119+ # token so we don't accept nonces in the past but still within
120+ # acceptable skew.
121+ cache.set(token_seen_key, max(date, token_seen), expires)
122
123
124 class OAuthRequestToken(OAuthBase):
125@@ -240,7 +258,7 @@
126 """See `IOAuthRequestToken`."""
127 assert not self.is_reviewed, (
128 "Request tokens can be reviewed only once.")
129- self.date_reviewed = datetime.now(pytz.timezone('UTC'))
130+ self.date_reviewed = datetime.now(UTC)
131 self.person = user
132 self.permission = permission
133 if IProduct.providedBy(context):
134@@ -286,16 +304,6 @@
135 return OAuthRequestToken.selectOneBy(key=key)
136
137
138-class OAuthNonce(OAuthBase):
139- """See `IOAuthNonce`."""
140- implements(IOAuthNonce)
141-
142- access_token = ForeignKey(
143- dbName='access_token', foreignKey='OAuthAccessToken', notNull=True)
144- request_timestamp = UtcDateTimeCol(default=UTC_NOW, notNull=True)
145- nonce = StringCol(notNull=True)
146-
147-
148 def create_token_key_and_secret(table):
149 """Create a key and secret for an OAuth token.
150
151
152=== modified file 'lib/canonical/launchpad/database/tests/test_oauth.py'
153--- lib/canonical/launchpad/database/tests/test_oauth.py 2010-03-24 17:37:26 +0000
154+++ lib/canonical/launchpad/database/tests/test_oauth.py 2010-06-08 12:11:25 +0000
155@@ -13,7 +13,7 @@
156 from zope.component import getUtility
157
158 from canonical.launchpad.database.oauth import (
159- OAuthAccessToken, OAuthConsumer, OAuthNonce, OAuthRequestToken)
160+ OAuthAccessToken, OAuthConsumer, OAuthRequestToken)
161 from canonical.testing.layers import DatabaseFunctionalLayer
162 from canonical.launchpad.webapp.interfaces import MAIN_STORE, MASTER_FLAVOR
163
164@@ -45,14 +45,8 @@
165 class_ = OAuthConsumer
166
167
168-class OAuthNonceTestCase(BaseOAuthTestCase):
169- class_ = OAuthNonce
170-
171-
172 def test_suite():
173 return unittest.TestSuite((
174 unittest.makeSuite(OAuthAccessTokenTestCase),
175 unittest.makeSuite(OAuthRequestTokenTestCase),
176- unittest.makeSuite(OAuthNonceTestCase),
177- unittest.makeSuite(OAuthConsumerTestCase),
178- ))
179+ unittest.makeSuite(OAuthConsumerTestCase)))
180
181=== modified file 'lib/canonical/launchpad/doc/oauth.txt'
182--- lib/canonical/launchpad/doc/oauth.txt 2010-04-16 15:06:55 +0000
183+++ lib/canonical/launchpad/doc/oauth.txt 2010-06-08 12:11:25 +0000
184@@ -15,7 +15,7 @@
185 ... AccessLevel, OAuthPermission)
186 >>> from canonical.launchpad.interfaces import (
187 ... IOAuthAccessToken, IOAuthConsumer, IOAuthConsumerSet,
188- ... IOAuthNonce, IOAuthRequestToken, IPersonSet)
189+ ... IOAuthRequestToken, IPersonSet)
190 >>> consumer_set = getUtility(IOAuthConsumerSet)
191 >>> verifyObject(IOAuthConsumerSet, consumer_set)
192 True
193@@ -307,26 +307,16 @@
194
195 >>> import time
196 >>> now = time.time() - 1
197- >>> nonce1 = access_token.checkNonceAndTimestamp('boo', now)
198- >>> verifyObject(IOAuthNonce, nonce1)
199- True
200+ >>> access_token.checkNonceAndTimestamp('boo', now)
201
202 - We can use an existing nonce with a new time.
203
204 >>> now += 1
205- >>> nonce2 = access_token.checkNonceAndTimestamp('boo', now)
206- >>> IOAuthNonce.providedBy(nonce2)
207- True
208- >>> nonce1 is nonce2
209- False
210+ >>> access_token.checkNonceAndTimestamp('boo', now)
211
212 - We can use a new nonce with the same time.
213
214- >>> nonce3 = access_token.checkNonceAndTimestamp('surprise!', now)
215- >>> IOAuthNonce.providedBy(nonce3)
216- True
217- >>> nonce1 is nonce3 or nonce2 is nonce3
218- False
219+ >>> access_token.checkNonceAndTimestamp('surprise!', now)
220
221 - But we cannot use an existing nonce used for the same time.
222
223@@ -368,12 +358,8 @@
224 ... TIMESTAMP_ACCEPTANCE_WINDOW)
225 >>> TIMESTAMP_ACCEPTANCE_WINDOW
226 60
227- >>> nonce4 = access_token.checkNonceAndTimestamp('boo', now-30)
228- >>> IOAuthNonce.providedBy(nonce4)
229- True
230- >>> nonce5 = access_token.checkNonceAndTimestamp('boo', now-60)
231- >>> IOAuthNonce.providedBy(nonce5)
232- True
233+ >>> access_token.checkNonceAndTimestamp('boo', now-30)
234+ >>> access_token.checkNonceAndTimestamp('boo', now-60)
235
236 - Once outside of the window (defined by the *latest* timestamp, even if it
237 is not the most recent), we get a TimestampOrderingError.
238@@ -405,9 +391,7 @@
239 ... TIMESTAMP_SKEW_WINDOW)
240 >>> TIMESTAMP_SKEW_WINDOW
241 3600
242- >>> nonce6 = access_token.checkNonceAndTimestamp('boo', now + 55*60)
243- >>> IOAuthNonce.providedBy(nonce6)
244- True
245+ >>> access_token.checkNonceAndTimestamp('boo', now + 55*60)
246
247 - We cannot access it 65 minutes in the future.
248
249@@ -419,9 +403,7 @@
250 - It's also worth noting that now the TIMESTAMP_ACCEPTANCE_WINDOW is based
251 off of the time 55 minutes in the future.
252
253- >>> nonce7 = access_token.checkNonceAndTimestamp('boo', now + 54*60 + 30)
254- >>> IOAuthNonce.providedBy(nonce7)
255- True
256+ >>> access_token.checkNonceAndTimestamp('boo', now + 54*60 + 30)
257 >>> access_token.checkNonceAndTimestamp('boo', now + 60)
258 Traceback (most recent call last):
259 ...
260
261=== modified file 'lib/canonical/launchpad/interfaces/oauth.py'
262--- lib/canonical/launchpad/interfaces/oauth.py 2010-04-19 14:47:49 +0000
263+++ lib/canonical/launchpad/interfaces/oauth.py 2010-06-08 12:11:25 +0000
264@@ -13,7 +13,6 @@
265 'IOAuthAccessToken',
266 'IOAuthConsumer',
267 'IOAuthConsumerSet',
268- 'IOAuthNonce',
269 'IOAuthRequestToken',
270 'IOAuthRequestTokenSet',
271 'IOAuthSignedRequest',
272@@ -174,7 +173,7 @@
273 +/- `TIMESTAMP_SKEW_WINDOW` of now.
274
275 If the nonce has never been used together with this token and
276- timestamp before, we store it in the database with the given timestamp
277+ timestamp before, we store it with the given timestamp
278 and associated with this token.
279 """
280
281@@ -236,20 +235,6 @@
282 """
283
284
285-class IOAuthNonce(Interface):
286- """The unique (nonce,timestamp) for requests using a given access token.
287-
288- The nonce value (which is unique for all requests with that timestamp)
289- is generated by the consumer and included, together with the timestamp,
290- in each request made. It's used to prevent replay attacks.
291- """
292-
293- request_timestamp = Datetime(
294- title=_('Date issued'), required=True, readonly=True)
295- access_token = Object(schema=IOAuthAccessToken, title=_('The token'))
296- nonce = TextLine(title=_('Nonce'), required=True, readonly=True)
297-
298-
299 class IOAuthSignedRequest(Interface):
300 """Marker interface for a request signed with OAuth credentials."""
301
302
303=== modified file 'lib/canonical/launchpad/scripts/garbo.py'
304--- lib/canonical/launchpad/scripts/garbo.py 2010-05-27 16:49:09 +0000
305+++ lib/canonical/launchpad/scripts/garbo.py 2010-06-08 12:11:25 +0000
306@@ -23,7 +23,6 @@
307 from canonical.launchpad.database.emailaddress import EmailAddress
308 from lp.hardwaredb.model.hwdb import HWSubmission
309 from canonical.launchpad.database.librarian import LibraryFileAlias
310-from canonical.launchpad.database.oauth import OAuthNonce
311 from canonical.launchpad.database.openidconsumer import OpenIDConsumerNonce
312 from canonical.launchpad.interfaces import IMasterStore
313 from canonical.launchpad.interfaces.emailaddress import EmailAddressStatus
314@@ -51,42 +50,6 @@
315 ONE_DAY_IN_SECONDS = 24*60*60
316
317
318-class OAuthNoncePruner(TunableLoop):
319- """An ITunableLoop to prune old OAuthNonce records.
320-
321- We remove all OAuthNonce records older than 1 day.
322- """
323- maximum_chunk_size = 6*60*60 # 6 hours in seconds.
324-
325- def __init__(self, log, abort_time=None):
326- super(OAuthNoncePruner, self).__init__(log, abort_time)
327- self.store = IMasterStore(OAuthNonce)
328- self.oldest_age = self.store.execute("""
329- SELECT COALESCE(EXTRACT(EPOCH FROM
330- CURRENT_TIMESTAMP AT TIME ZONE 'UTC'
331- - MIN(request_timestamp)), 0)
332- FROM OAuthNonce
333- """).get_one()[0]
334-
335- def isDone(self):
336- return self.oldest_age <= ONE_DAY_IN_SECONDS
337-
338- def __call__(self, chunk_size):
339- self.oldest_age = max(
340- ONE_DAY_IN_SECONDS, self.oldest_age - chunk_size)
341-
342- self.log.debug(
343- "Removed OAuthNonce rows older than %d seconds"
344- % self.oldest_age)
345-
346- self.store.find(
347- OAuthNonce,
348- OAuthNonce.request_timestamp < SQL(
349- "CURRENT_TIMESTAMP AT TIME ZONE 'UTC' - interval '%d seconds'"
350- % self.oldest_age)).remove()
351- transaction.commit()
352-
353-
354 class OpenIDConsumerNoncePruner(TunableLoop):
355 """An ITunableLoop to prune old OpenIDConsumerNonce records.
356
357@@ -739,7 +702,6 @@
358 class HourlyDatabaseGarbageCollector(BaseDatabaseGarbageCollector):
359 script_name = 'garbo-hourly'
360 tunable_loops = [
361- OAuthNoncePruner,
362 OpenIDConsumerNoncePruner,
363 OpenIDConsumerAssociationPruner,
364 RevisionCachePruner,
365
366=== modified file 'lib/canonical/launchpad/scripts/tests/test_garbo.py'
367--- lib/canonical/launchpad/scripts/tests/test_garbo.py 2010-05-26 01:48:12 +0000
368+++ lib/canonical/launchpad/scripts/tests/test_garbo.py 2010-06-08 12:11:25 +0000
369@@ -20,7 +20,6 @@
370 from canonical.config import config
371 from canonical.database.constants import THIRTY_DAYS_AGO, UTC_NOW
372 from canonical.launchpad.database.message import Message
373-from canonical.launchpad.database.oauth import OAuthNonce
374 from canonical.launchpad.database.openidconsumer import OpenIDConsumerNonce
375 from canonical.launchpad.interfaces import IMasterStore
376 from canonical.launchpad.interfaces.emailaddress import EmailAddressStatus
377@@ -90,46 +89,6 @@
378 collector.main()
379 return collector
380
381- def test_OAuthNoncePruner(self):
382- now = datetime.utcnow().replace(tzinfo=UTC)
383- timestamps = [
384- now - timedelta(days=2), # Garbage
385- now - timedelta(days=1) - timedelta(seconds=60), # Garbage
386- now - timedelta(days=1) + timedelta(seconds=60), # Not garbage
387- now, # Not garbage
388- ]
389- LaunchpadZopelessLayer.switchDbUser('testadmin')
390- store = IMasterStore(OAuthNonce)
391-
392- # Make sure we start with 0 nonces.
393- self.failUnlessEqual(store.find(OAuthNonce).count(), 0)
394-
395- for timestamp in timestamps:
396- OAuthNonce(
397- access_tokenID=1,
398- request_timestamp = timestamp,
399- nonce = str(timestamp))
400- transaction.commit()
401-
402- # Make sure we have 4 nonces now.
403- self.failUnlessEqual(store.find(OAuthNonce).count(), 4)
404-
405- self.runHourly(maximum_chunk_size=60) # 1 minute maximum chunk size
406-
407- store = IMasterStore(OAuthNonce)
408-
409- # Now back to two, having removed the two garbage entries.
410- self.failUnlessEqual(store.find(OAuthNonce).count(), 2)
411-
412- # And none of them are older than a day.
413- # Hmm... why is it I'm putting tz aware datetimes in and getting
414- # naive datetimes back? Bug in the SQLObject compatibility layer?
415- # Test is still fine as we know the timezone.
416- self.failUnless(
417- store.find(
418- Min(OAuthNonce.request_timestamp)).one().replace(tzinfo=UTC)
419- >= now - timedelta(days=1))
420-
421 def test_OpenIDConsumerNoncePruner(self):
422 now = int(time.mktime(time.gmtime()))
423 MINUTES = 60
424
425=== modified file 'lib/canonical/launchpad/zcml/oauth.zcml'
426--- lib/canonical/launchpad/zcml/oauth.zcml 2009-07-13 18:15:02 +0000
427+++ lib/canonical/launchpad/zcml/oauth.zcml 2010-06-08 12:11:25 +0000
428@@ -44,8 +44,4 @@
429 permission="launchpad.Edit"
430 set_schema="canonical.launchpad.interfaces.IOAuthAccessToken"/>
431 </class>
432-
433- <class class="canonical.launchpad.database.OAuthNonce">
434- <allow interface="canonical.launchpad.interfaces.IOAuthNonce"/>
435- </class>
436 </configure>