Skip to content

Commit

Permalink
Merge remote-tracking branch 'origin/develop' into merge-master-to-ex…
Browse files Browse the repository at this point in the history
…periemental-client
  • Loading branch information
Jussi Kukkonen committed Apr 21, 2021
2 parents 5dde1e7 + 41f7e80 commit df1a888
Show file tree
Hide file tree
Showing 57 changed files with 278 additions and 546 deletions.
4 changes: 3 additions & 1 deletion MANIFEST.in
Expand Up @@ -2,10 +2,12 @@ include LICENSE*
include README.md
include tox.ini
include tests/repository_data/keystore/delegation_key
include tests/repository_data/keystore/root_key
include tests/repository_data/keystore/root_key*
include tests/repository_data/keystore/snapshot_key
include tests/repository_data/keystore/targets_key
include tests/repository_data/keystore/timestamp_key
include tests/ssl_certs/*.crt
include tests/ssl_certs/*.key

recursive-include docs *.txt
recursive-include docs *.md
Expand Down
51 changes: 51 additions & 0 deletions docs/adr/0008-accept-unrecognised-fields.md
@@ -0,0 +1,51 @@
# Accept metadata that includes unrecognized fields

- Status: accepted
- Date: 2021-04-08

Technical Story: https://github.com/theupdateframework/tuf/issues/1266

## Context and Problem Statement

The current reference implementation will ignore unrecognized fields in a
metadata file when loading it.
This leads to the side effect that if you read a metadata file with unrecognized
fields and immediately write it back to the disk, this file will be modified.

Furthermore, some TAPs like:
- [TAP 6](https://github.com/theupdateframework/taps/blob/master/tap6.md)
- [TAP 10](https://github.com/theupdateframework/taps/blob/master/tap10.md)
- [TAP 14](https://github.com/theupdateframework/taps/blob/master/tap14.md)
- [TAP 15](https://github.com/theupdateframework/taps/blob/master/tap15.md)
- [TAP 16](https://github.com/theupdateframework/taps/blob/master/tap16.md)

are relying on that unrecognized fields will be accepted to introduce new fields
to the specification without making the metadata invalid for older clients who
don't recognize the field.

## Decision Drivers
- The TUF specification implies support for unrecognized attribute-value fields,
see [Document formats](https://theupdateframework.github.io/specification/latest/#document-formats)
- If we perform the following operations on a metadata file with no
intermediate operations:
1. read the metadata file
2. write the metadata file back to the disk

then, the checksum (the content) of the file must not be changed.
- Flexibility to add new fields in the spec without adding breaking changes.

## Considered Options
- Ignore and drop unrecognized fields.
- Ignore, but store unrecognized fields as an additional attribute.

## Decision Outcome

Chosen option: "Ignore, but store unrecognized fields as an additional
attribute."
The motivation for this decision is that the TUF specification already implies
that we should accept unrecognized fields for backward compatibility and easier
future extensibility.

Additionally, it seems unacceptable to change a metadata file content just by
reading and writing it back.

3 changes: 3 additions & 0 deletions docs/adr/index.md
Expand Up @@ -11,6 +11,9 @@ This log lists the architectural decisions for tuf.
- [ADR-0004](0004-extent-of-OOP-in-metadata-model.md) - Add classes for complex metadata attributes
- [ADR-0005](0005-use-google-python-style-guide.md) - Use Google Python style guide with minimal refinements


- [ADR-0008](0008-accept-unrecognised-fields.md) - Accept metadata that includes unrecognized fields

<!-- adrlogstop -->

For new ADRs, please use [template.md](template.md) as basis.
Expand Down
2 changes: 1 addition & 1 deletion docs/tuf-spec.0.9.txt
@@ -1 +1 @@
The TUF specification file has been moved to https://github.com/theupdateframework/specification/blob/master/historical/tuf-spec.md
The TUF specification file has been moved to https://github.com/theupdateframework/specification/blob/master/historical/tuf-spec.0.9.txt
4 changes: 2 additions & 2 deletions pylintrc
Expand Up @@ -287,7 +287,7 @@ ignore-on-opaque-inference=yes
# List of class names for which member attributes should not be checked (useful
# for classes with dynamically set attributes). This supports the use of
# qualified names.
ignored-classes=optparse.Values,thread._local,_thread._local, six.moves
ignored-classes=optparse.Values,thread._local,_thread._local

# List of module names for which member attributes should not be checked
# (useful for modules/projects where namespaces are manipulated during runtime
Expand Down Expand Up @@ -334,7 +334,7 @@ init-import=no

# List of qualified module names which can have objects that can redefine
# builtins.
redefining-builtins-modules=six.moves,future.builtins
redefining-builtins-modules=future.builtins


[CLASSES]
Expand Down
3 changes: 1 addition & 2 deletions requirements-pinned.txt
@@ -1,11 +1,10 @@
certifi==2020.12.5 # via requests
cffi==1.14.5 # via cryptography, pynacl
chardet==4.0.0 # via requests
cryptography==3.4.6 # via securesystemslib
cryptography==3.4.7 # via securesystemslib
idna==2.10 # via requests
pycparser==2.20 # via cffi
pynacl==1.4.0 # via securesystemslib
requests==2.25.1
securesystemslib[crypto,pynacl]==0.20.0
six==1.15.0
urllib3==1.26.4 # via requests
1 change: 0 additions & 1 deletion requirements.txt
Expand Up @@ -43,4 +43,3 @@
#
securesystemslib[crypto, pynacl]
requests
six
3 changes: 1 addition & 2 deletions setup.py
Expand Up @@ -113,8 +113,7 @@
python_requires="~=3.6",
install_requires = [
'requests>=2.19.1',
'securesystemslib>=0.20.0',
'six>=1.11.0'
'securesystemslib>=0.20.0'
],
packages = find_packages(exclude=['tests']),
scripts = [
Expand Down
8 changes: 0 additions & 8 deletions tests/aggregate_tests.py
Expand Up @@ -26,14 +26,6 @@
'tuf/tests'. Use --random to run the tests in random order.
"""

# Help with Python 3 compatibility, where the print statement is a function, an
# implicit relative import is invalid, and the '/' operator performs true
# division. Example: print 'hello world' raises a 'SyntaxError' exception.
from __future__ import print_function
from __future__ import absolute_import
from __future__ import division
from __future__ import unicode_literals

import sys
import unittest

Expand Down
14 changes: 3 additions & 11 deletions tests/simple_https_server.py
Expand Up @@ -29,18 +29,10 @@
http://docs.python.org/library/simplehttpserver.html#module-SimpleHTTPServer
"""

# Help with Python 3 compatibility, where the print statement is a function, an
# implicit relative import is invalid, and the '/' operator performs true
# division. Example: print 'hello world' raises a 'SyntaxError' exception.
from __future__ import print_function
from __future__ import absolute_import
from __future__ import division
from __future__ import unicode_literals

import sys
import ssl
import os
import six
import http.server

keyfile = os.path.join('ssl_certs', 'ssl_cert.key')
certfile = os.path.join('ssl_certs', 'ssl_cert.crt')
Expand All @@ -49,8 +41,8 @@
if len(sys.argv) > 1 and os.path.exists(sys.argv[1]):
certfile = sys.argv[1]

httpd = six.moves.BaseHTTPServer.HTTPServer(('localhost', 0),
six.moves.SimpleHTTPServer.SimpleHTTPRequestHandler)
httpd = http.server.HTTPServer(('localhost', 0),
http.server.SimpleHTTPRequestHandler)

httpd.socket = ssl.wrap_socket(
httpd.socket, keyfile=keyfile, certfile=certfile, server_side=True)
Expand Down
17 changes: 4 additions & 13 deletions tests/simple_server.py
Expand Up @@ -25,19 +25,10 @@
http://docs.python.org/library/simplehttpserver.html#module-SimpleHTTPServer
"""

# Help with Python 3 compatibility, where the print statement is a function, an
# implicit relative import is invalid, and the '/' operator performs true
# division. Example: print 'hello world' raises a 'SyntaxError' exception.
from __future__ import print_function
from __future__ import absolute_import
from __future__ import division
from __future__ import unicode_literals

import sys
import random

import six
from six.moves.SimpleHTTPServer import SimpleHTTPRequestHandler
import socketserver
from http.server import SimpleHTTPRequestHandler


class QuietHTTPRequestHandler(SimpleHTTPRequestHandler):
Expand All @@ -64,9 +55,9 @@ def log_request(self, code='-', size='-'):

# Allow re-use so you can re-run tests as often as you want even if the
# tests re-use ports. Otherwise TCP TIME-WAIT prevents reuse for ~1 minute
six.moves.socketserver.TCPServer.allow_reuse_address = True
socketserver.TCPServer.allow_reuse_address = True

httpd = six.moves.socketserver.TCPServer(('localhost', 0), handler)
httpd = socketserver.TCPServer(('localhost', 0), handler)
port_message = 'bind succeeded, server port is: ' \
+ str(httpd.server_address[1])
print(port_message)
Expand Down
15 changes: 3 additions & 12 deletions tests/slow_retrieval_server.py
Expand Up @@ -21,24 +21,15 @@
interval 'DELAY'). The server is used in 'test_slow_retrieval_attack.py'.
"""

# Help with Python 3 compatibility, where the print statement is a function, an
# implicit relative import is invalid, and the '/' operator performs true
# division. Example: print 'hello world' raises a 'SyntaxError' exception.
from __future__ import print_function
from __future__ import absolute_import
from __future__ import division
from __future__ import unicode_literals

import os
import sys
import time

import six
import http.server



# HTTP request handler.
class Handler(six.moves.BaseHTTPServer.BaseHTTPRequestHandler):
class Handler(http.server.BaseHTTPRequestHandler):

# Overwrite do_GET.
def do_GET(self):
Expand Down Expand Up @@ -66,7 +57,7 @@ def do_GET(self):
if __name__ == '__main__':
server_address = ('localhost', 0)

httpd = six.moves.BaseHTTPServer.HTTPServer(server_address, Handler)
httpd = http.server.HTTPServer(server_address, Handler)
port_message = 'bind succeeded, server port is: ' \
+ str(httpd.server_address[1])
print(port_message)
Expand Down
29 changes: 24 additions & 5 deletions tests/test_api.py
Expand Up @@ -105,8 +105,7 @@ def test_generic_read(self):
path = os.path.join(self.repo_dir, 'metadata', metadata + '.json')
metadata_obj = Metadata.from_file(path)
with open(path, 'rb') as f:
metadata_str = f.read()
metadata_obj2 = JSONDeserializer().deserialize(metadata_str)
metadata_obj2 = Metadata.from_bytes(f.read())

# Assert that both methods instantiate the right inner class for
# each metadata type and ...
Expand All @@ -119,15 +118,17 @@ def test_generic_read(self):
self.assertDictEqual(
metadata_obj.to_dict(), metadata_obj2.to_dict())


# Assert that it chokes correctly on an unknown metadata type
bad_metadata_path = 'bad-metadata.json'
bad_metadata = {'signed': {'_type': 'bad-metadata'}}
bad_string = json.dumps(bad_metadata).encode('utf-8')
with open(bad_metadata_path, 'wb') as f:
f.write(json.dumps(bad_metadata).encode('utf-8'))
f.write(bad_string)

with self.assertRaises(DeserializationError):
Metadata.from_file(bad_metadata_path)
with self.assertRaises(DeserializationError):
Metadata.from_bytes(bad_string)

os.remove(bad_metadata_path)

Expand Down Expand Up @@ -219,7 +220,25 @@ def test_metadata_base(self):
md.signed.bump_expiration(timedelta(days=365))
self.assertEqual(md.signed.expires, datetime(2031, 1, 2, 0, 0))


# Test is_expired with reference_time provided
is_expired = md.signed.is_expired(md.signed.expires)
self.assertTrue(is_expired)
is_expired = md.signed.is_expired(md.signed.expires + timedelta(days=1))
self.assertTrue(is_expired)
is_expired = md.signed.is_expired(md.signed.expires - timedelta(days=1))
self.assertFalse(is_expired)

# Test is_expired without reference_time,
# manipulating md.signed.expires
expires = md.signed.expires
md.signed.expires = datetime.utcnow()
is_expired = md.signed.is_expired()
self.assertTrue(is_expired)
md.signed.expires = datetime.utcnow() + timedelta(days=1)
is_expired = md.signed.is_expired()
self.assertFalse(is_expired)
md.signed.expires = expires

def test_metadata_snapshot(self):
snapshot_path = os.path.join(
self.repo_dir, 'metadata', 'snapshot.json')
Expand Down
20 changes: 6 additions & 14 deletions tests/test_arbitrary_package_attack.py
Expand Up @@ -28,21 +28,14 @@
There is no difference between 'updates' and 'target' files.
"""

# Help with Python 3 compatibility, where the print statement is a function, an
# implicit relative import is invalid, and the '/' operator performs true
# division. Example: print 'hello world' raises a 'SyntaxError' exception.
from __future__ import print_function
from __future__ import absolute_import
from __future__ import division
from __future__ import unicode_literals

import os
import tempfile
import shutil
import json
import logging
import unittest
import sys
from urllib import request

import tuf
import tuf.formats
Expand All @@ -55,7 +48,6 @@
from tests import utils

import securesystemslib
import six

logger = logging.getLogger(__name__)

Expand Down Expand Up @@ -142,16 +134,16 @@ def setUp(self):


def tearDown(self):
# Modified_TestCase.tearDown() automatically deletes temporary files and
# directories that may have been created during each test case.
unittest_toolbox.Modified_TestCase.tearDown(self)
# updater.Updater() populates the roledb with the name "test_repository1"
tuf.roledb.clear_roledb(clear_all=True)
tuf.keydb.clear_keydb(clear_all=True)

# Logs stdout and stderr from the sever subprocess.
self.server_process_handler.flush_log()

# Remove temporary directory
unittest_toolbox.Modified_TestCase.tearDown(self)



def test_without_tuf(self):
Expand All @@ -174,7 +166,7 @@ def test_without_tuf(self):
url_file = os.path.join(url_prefix, 'targets', 'file1.txt')

# On Windows, the URL portion should not contain back slashes.
six.moves.urllib.request.urlretrieve(url_file.replace('\\', '/'), client_target_path)
request.urlretrieve(url_file.replace('\\', '/'), client_target_path)

self.assertTrue(os.path.exists(client_target_path))
length, hashes = securesystemslib.util.get_file_details(client_target_path)
Expand All @@ -188,7 +180,7 @@ def test_without_tuf(self):
malicious_fileinfo = tuf.formats.make_targets_fileinfo(length, hashes)

# On Windows, the URL portion should not contain back slashes.
six.moves.urllib.request.urlretrieve(url_file.replace('\\', '/'), client_target_path)
request.urlretrieve(url_file.replace('\\', '/'), client_target_path)

length, hashes = securesystemslib.util.get_file_details(client_target_path)
download_fileinfo = tuf.formats.make_targets_fileinfo(length, hashes)
Expand Down
13 changes: 3 additions & 10 deletions tests/test_download.py
Expand Up @@ -25,14 +25,6 @@
TODO: Adopt the environment variable management from test_proxy_use.py here.
"""

# Help with Python 3 compatibility, where the print statement is a function, an
# implicit relative import is invalid, and the '/' operator performs true
# division. Example: print 'hello world' raises a 'SyntaxError' exception.
from __future__ import print_function
from __future__ import absolute_import
from __future__ import division
from __future__ import unicode_literals

import hashlib
import logging
import os
Expand Down Expand Up @@ -93,13 +85,14 @@ def setUp(self):

# Stop server process and perform clean up.
def tearDown(self):
unittest_toolbox.Modified_TestCase.tearDown(self)

# Cleans the resources and flush the logged lines (if any).
self.server_process_handler.clean()

self.target_fileobj.close()

# Remove temp directory
unittest_toolbox.Modified_TestCase.tearDown(self)


# Test: Normal case.
def test_download_url_to_tempfileobj(self):
Expand Down

0 comments on commit df1a888

Please sign in to comment.