Merge pull request #351 from dotcloud/remove-cookie

Remove cookie (flask.session)

Author: dmp42

README.md

@@ -95,9 +95,6 @@ Available configuration options
 
 ### General options
 
-1. `secret_key`: 64 character string, this key should be unique and secret. It
-    is used by the Registry to sign secret things. If you leave this blank, the
-    Registry will generate a random string.
 1. `loglevel`: string, level of debugging. Any of python's
     [logging](http://docs.python.org/2/library/logging.html) module levels:
     `debug`, `info`, `warn`, `error` or `critical`
@@ -397,11 +394,6 @@ using this command:
 gunicorn -k gevent --max-requests 100 --graceful-timeout 3600 -t 3600 -b localhost:5000 -w 8 docker_registry.wsgi:application
 ```
 
-Note that when using multiple workers, the secret_key for the Flask session
-must be set explicitly in config.yml. Otherwise each worker will use its own
-random secret key, leading to unpredictable behavior.
-
-
 #### nginx
 
 [Here is an nginx configuration file example.](https://github.com/dotcloud/docker-registry/blob/master/contrib/nginx.conf), which applies to versions < 1.3.9 which are compiled with the [HttpChunkinModule](http://wiki.nginx.org/HttpChunkinModule). 

config/config_s3.yml

@@ -12,8 +12,7 @@ prod:
     s3_bucket: _env:AWS_BUCKET
     s3_encrypt: true
     s3_secure: true
-    secret_key: REPLACEME
     s3_encrypt: true
     s3_secure: true
     storage_path: /images
-    storage_redirect: False  # Redirect signed URL for image files
+    storage_redirect: False  # Redirect signed URL for image files

config/config_sample.yml

@@ -28,9 +28,6 @@ common:
     # See: https://developers.google.com/accounts/docs/OAuth2.
     oauth2: REPLACEME
 
-    # Let gunicorn set this environment variable or set a random string here
-    secret_key: _env:SECRET_KEY
-
     search_backend: "_env:SEARCH_BACKEND:"
     sqlalchemy_index_database:
         "_env:SQLALCHEMY_INDEX_DATABASE:sqlite:////tmp/docker-registry.db"

config/config_test.yml

@@ -4,7 +4,6 @@ test:
     loglevel: info
     storage: local
     storage_path: /tmp/test
-    secret_key: foobar42
     s3_access_key: _env:S3_ACCESS_KEY
     s3_secret_key: _env:S3_SECRET_KEY
     s3_bucket: _env:S3_BUCKET

config/gunicorn_config.py

@@ -1,16 +1,9 @@
 ## Gunicorn config file
 
 import os
-import random
-import string
 
 flavor = os.environ.get('SETTINGS_FLAVOR', 'dev')
 
-if flavor == 'dev':
-    os.environ['SECRET_KEY'] = ''.join(
-        [random.choice(string.ascii_lowercase + string.ascii_uppercase +
-         string.digits) for x in range(128)])
-
 reload = True
 bind = '0.0.0.0:{0}'.format(os.environ.get('PORT_WWW', 8000))
 graceful_timeout = 3600

docker_registry/app.py

@@ -13,8 +13,6 @@
 from . import toolkit
 from .lib import config
 
-from .lib.core.exceptions import ConfigError
-
 
 VERSION = '0.6.9'
 app = flask.Flask('docker-registry')
@@ -46,10 +44,6 @@ def after_request(response):
 
 
 def init():
-    # Configure the secret key
-    if not cfg.secret_key:
-        raise ConfigError('`secret_key\' is not set')
-    app.secret_key = cfg.secret_key
     # Configure the email exceptions
     info = cfg.email_exceptions
     if info:

docker_registry/images.py

@@ -50,7 +50,6 @@ def wrapper(*args, **kwargs):
             return flask.Response(status=304, headers=headers)
         kwargs['headers'] = headers
         # Prevent the Cookie to be sent when the object is cacheable
-        flask.session.modified = False
         return f(*args, **kwargs)
     return wrapper
 
@@ -117,9 +116,11 @@ def _get_image_json(image_id, headers=None):
         headers['X-Docker-Size'] = str(size)
     except OSError:
         pass
-    checksum_path = store.image_checksum_path(image_id)
-    if store.exists(checksum_path):
-        headers['X-Docker-Checksum'] = store.get_content(checksum_path)
+    try:
+        csums = load_checksums(image_id)
+        headers['X-Docker-Payload-Checksum'] = csums
+    except IOError:
+        pass
     return toolkit.response(data, headers=headers, raw=True)
 
 
@@ -254,20 +255,8 @@ def put_image_layer(image_id):
             csums.append(tarsum.compute())
             tmp.close()
 
-    try:
-        checksum = store.get_content(store.image_checksum_path(image_id))
-    except IOError:
-        # We don't have a checksum stored yet, that's fine skipping the check.
-        # Not removing the mark though, image is not downloadable yet.
-        flask.session['checksum'] = csums
-        return toolkit.response()
-    # We check if the checksums provided matches one the one we computed
-    if checksum not in csums:
-        logger.debug('put_image_layer: Wrong checksum')
-        return toolkit.api_error('Checksum mismatch, ignoring the layer')
-
-    # Checksum is ok, we remove the marker
-    store.remove(mark_path)
+    # We store the computed checksums for a later check
+    save_checksums(image_id, csums)
     return toolkit.response()
 
 
@@ -280,20 +269,16 @@ def put_image_checksum(image_id):
         checksum = flask.request.headers.get('X-Docker-Checksum-Payload')
     if not checksum:
         return toolkit.api_error('Missing Image\'s checksum')
-    if not flask.session.get('checksum'):
-        return toolkit.api_error('Checksum not found in Cookie')
     if not store.exists(store.image_json_path(image_id)):
         return toolkit.api_error('Image not found', 404)
     mark_path = store.image_mark_path(image_id)
     if not store.exists(mark_path):
         return toolkit.api_error('Cannot set this image checksum', 409)
-    err = store_checksum(image_id, checksum)
-    if err:
-        return toolkit.api_error(err)
-    if checksum not in flask.session.get('checksum', []):
+    checksums = load_checksums(image_id)
+    if checksum not in checksums:
         logger.debug('put_image_checksum: Wrong checksum. '
                      'Provided: {0}; Expected: {1}'.format(
-                         checksum, flask.session.get('checksum')))
+                         checksum, checksums))
         return toolkit.api_error('Checksum mismatch')
     # Checksum is ok, we remove the marker
     store.remove(mark_path)
@@ -352,26 +337,37 @@ def get_image_ancestry(image_id, headers):
 
 
 def check_images_list(image_id):
-    full_repos_name = flask.session.get('repository')
-    if not full_repos_name:
-        # We only enforce this check when there is a repos name in the session
-        # otherwise it means that the auth is disabled.
+    if cfg.disable_token_auth is True or cfg.standalone is not False:
+        # We enforce the check only when auth is enabled so we have a token.
         return True
+    repository = toolkit.get_repository()
     try:
-        path = store.images_list_path(*full_repos_name.split('/'))
+        path = store.images_list_path(*repository)
         images_list = json.loads(store.get_content(path))
     except IOError:
         return False
     return (image_id in images_list)
 
 
-def store_checksum(image_id, checksum):
-    checksum_parts = checksum.split(':')
-    if len(checksum_parts) != 2:
-        return 'Invalid checksum format'
+def save_checksums(image_id, checksums):
+    for checksum in checksums:
+        checksum_parts = checksum.split(':')
+        if len(checksum_parts) != 2:
+            return 'Invalid checksum format'
     # We store the checksum
     checksum_path = store.image_checksum_path(image_id)
-    store.put_content(checksum_path, checksum)
+    store.put_content(checksum_path, json.dumps(checksums))
+
+
+def load_checksums(image_id):
+    checksum_path = store.image_checksum_path(image_id)
+    data = store.get_content(checksum_path)
+    try:
+        return json.loads(data)
+    except ValueError:
+        # NOTE(sam): For backward compatibility only, existing data may not be
+        # a valid json but a simple string.
+        return [data]
 
 
 @app.route('/v1/images/<image_id>/json', methods=['PUT'])
@@ -385,16 +381,6 @@ def put_image_json(image_id):
         return toolkit.api_error('Invalid JSON')
     if 'id' not in data:
         return toolkit.api_error('Missing key `id\' in JSON')
-    # Read the checksum
-    checksum = flask.request.headers.get('X-Docker-Checksum')
-    if checksum:
-        # Storing the checksum is optional at this stage
-        err = store_checksum(image_id, checksum)
-        if err:
-            return toolkit.api_error(err)
-    else:
-        # We cleanup any old checksum in case it's a retry after a fail
-        store.remove(store.image_checksum_path(image_id))
     if image_id != data['id']:
         return toolkit.api_error('JSON data contains invalid id')
     if check_images_list(image_id) is False:
@@ -412,6 +398,8 @@ def put_image_json(image_id):
     # If we reach that point, it means that this is a new image or a retry
     # on a failed push
     store.put_content(mark_path, 'true')
+    # We cleanup any old checksum in case it's a retry after a fail
+    store.remove(store.image_checksum_path(image_id))
     store.put_content(json_path, flask.request.data)
     layers.generate_ancestry(image_id, parent_id)
     return toolkit.response()

docker_registry/toolkit.py

@@ -212,12 +212,7 @@ def check_token(args):
         return False
     if validate_token(auth) is False:
         return False
-    # Token is valid, we create a session
-    session = flask.session
-    session['repository'] = auth.get('repository')
-    if is_ssl() is False:
-        # We enforce the IP check only when not using SSL
-        session['from'] = get_remote_ip()
+    # Token is valid
     return True
 
 
@@ -256,11 +251,8 @@ def parse_content_signature(s):
 def requires_auth(f):
     @functools.wraps(f)
     def wrapper(*args, **kwargs):
-        session = flask.session
         if check_signature() is True or check_token(kwargs) is True:
-            if 'from' not in session or session['from'] == get_remote_ip():
-                return f(*args, **kwargs)
-        session.clear()
+            return f(*args, **kwargs)
         headers = {'WWW-Authenticate': 'Token'}
         return api_error('Requires authorization', 401, headers)
     return wrapper

test/base.py

@@ -22,7 +22,7 @@ def _open(*args, **kwargs):
             if 'headers' not in kwargs:
                 kwargs['headers'] = {}
             if 'User-Agent' not in kwargs['headers']:
-                ua = ('docker/0.0 go/go1.2.1 git-commit/3600720 '
+                ua = ('docker/0.10.1 go/go1.2.1 git-commit/3600720 '
                       'kernel/3.8.0-19-generic os/linux arch/amd64')
                 kwargs['headers']['User-Agent'] = ua
             return orig_open(*args, **kwargs)
@@ -32,8 +32,20 @@ def gen_random_string(self, length=16):
         return ''.join([random.choice(string.ascii_uppercase + string.digits)
                         for x in range(length)]).lower()
 
-    def upload_image(self, image_id, parent_id, layer,
-                     set_checksum_callback=None):
+    def set_image_checksum(self, image_id, checksum):
+        headers = {'X-Docker-Checksum-Payload': checksum}
+        url = '/v1/images/{0}/checksum'.format(image_id)
+        resp = self.http_client.put(url, headers=headers)
+        self.assertEqual(resp.status_code, 200, resp.data)
+        # Once the checksum test passed, the image is "locked"
+        resp = self.http_client.put(url, headers=headers)
+        self.assertEqual(resp.status_code, 409, resp.data)
+        # Cannot set the checksum on an non-existing image
+        url = '/v1/images/{0}/checksum'.format(self.gen_random_string())
+        resp = self.http_client.put(url, headers=headers)
+        self.assertEqual(resp.status_code, 404, resp.data)
+
+    def upload_image(self, image_id, parent_id, layer):
         json_obj = {
             'id': image_id
         }
@@ -43,9 +55,7 @@ def upload_image(self, image_id, parent_id, layer,
         h = hashlib.sha256(json_data + '\n')
         h.update(layer)
         layer_checksum = 'sha256:{0}'.format(h.hexdigest())
-        headers = {'X-Docker-Checksum': layer_checksum}
-        if set_checksum_callback:
-            headers = {}
+        headers = {'X-Docker-Payload-Checksum': layer_checksum}
         resp = self.http_client.put('/v1/images/{0}/json'.format(image_id),
                                     headers=headers,
                                     data=json_data)
@@ -58,10 +68,10 @@ def upload_image(self, image_id, parent_id, layer,
                                     input_stream=layer_file)
         layer_file.close()
         self.assertEqual(resp.status_code, 200, resp.data)
-        if set_checksum_callback:
-            set_checksum_callback(image_id, layer_checksum)
+        self.set_image_checksum(image_id, layer_checksum)
         # Push done, test reading the image
         resp = self.http_client.get('/v1/images/{0}/json'.format(image_id))
-        self.assertEqual(resp.headers.get('x-docker-size'), str(len(layer)))
         self.assertEqual(resp.status_code, 200, resp.data)
-        self.assertEqual(resp.headers['x-docker-checksum'], layer_checksum)
+        self.assertEqual(resp.headers.get('x-docker-size'), str(len(layer)))
+        self.assertEqual(resp.headers['x-docker-payload-checksum'],
+                         layer_checksum)

test/test_images.py

@@ -64,27 +64,6 @@ def test_notfound(self):
             self.gen_random_string()))
         self.assertEqual(resp.status_code, 404, resp.data)
 
-    def test_set_checksum_after(self):
-        def set_checksum_callback(image_id, checksum):
-            headers = {'X-Docker-Checksum': checksum}
-            url = '/v1/images/{0}/checksum'.format(image_id)
-            resp = self.http_client.put(url, headers=headers)
-            self.assertEqual(resp.status_code, 200, resp.data)
-            # Once the checksum test passed, the image is "locked"
-            resp = self.http_client.put(url, headers=headers)
-            self.assertEqual(resp.status_code, 409, resp.data)
-            # Cannot set the checksum on an non-existing image
-            url = '/v1/images/{0}/checksum'.format(self.gen_random_string())
-            resp = self.http_client.put(url, headers=headers)
-            self.assertEqual(resp.status_code, 404, resp.data)
-
-        image_id = self.gen_random_string()
-        layer_data = self.gen_random_string(1024)
-        self.upload_image(image_id,
-                          parent_id=None,
-                          layer=layer_data,
-                          set_checksum_callback=set_checksum_callback)
-
     def test_bytes_range(self):
         image_id = self.gen_random_string()
         layer_data = self.gen_random_string(1024)