Bug#902758: stretch-pu: package subversion/1.9.5-1+deb9u2
Package: release.debian.org
Severity: normal
Tags: stretch
User: release.debian.org@packages.debian.org
Usertags: pu
It recently came up in discussion with upstream that Stretch only had
1.9.5 and although that had seen an update for a CVE, there hadn't been
any for shattered -- big oversight on my part.
I have uploaded 1.9.5-1+deb9u2 to address the SHA-1 collision/shattered
issues with subversion. These are the same patches that were included
in the official upstream release of 1.9.6 to address the issue.
The delta isn't small, but it does include new test coverage and there
have been no further changes in the 1.9.x release upstream related to
this.
-- System Information:
Debian Release: buster/sid
APT prefers unstable-debug
APT policy: (500, 'unstable-debug'), (500, 'unstable'), (1, 'experimental-debug'), (1, 'experimental')
Architecture: amd64 (x86_64)
Foreign Architectures: i386
Kernel: Linux 4.16.0-2-amd64 (SMP w/4 CPU cores)
Locale: LANG=en_US.UTF-8, LC_CTYPE=en_US.UTF-8 (charmap=UTF-8), LANGUAGE=en_US.UTF-8 (charmap=UTF-8)
Shell: /bin/sh linked to /bin/dash
Init: systemd (via /run/systemd/system)
LSM: AppArmor: enabled
diffstat for subversion_1.9.5-1+deb9u1 subversion_1.9.5-1+deb9u2
debian/patches/apr_file_trunc_r1759116 | 141 ++++++++
debian/patches/no-dir-rep-sharing_r1794527 | 157 +++++++++
debian/patches/no-dir-rep-sharing_r1796725 | 29 +
debian/patches/shattered_r1795993 | 491 +++++++++++++++++++++++++++++
debian/patches/shattered_r1796470 | 127 +++++++
subversion-1.9.5/debian/changelog | 12
subversion-1.9.5/debian/patches/series | 5
7 files changed, 962 insertions(+)
diff -u subversion-1.9.5/debian/changelog subversion-1.9.5/debian/changelog
--- subversion-1.9.5/debian/changelog
+++ subversion-1.9.5/debian/changelog
@@ -1,3 +1,15 @@
+subversion (1.9.5-1+deb9u2) stretch; urgency=medium
+
+ * Backport r1759116, working around an issue in APR's trunc API. This is a
+ prerequisite for the SHA1/shattered fixes.
+ * Backport r1794527 and r1796725 to prevent the possibility of rep-sharing
+ between a directory rep and a file/prop rep.
+ * Backport r1795993 and r1796470 to reject commits which would introduce
+ hash collisions with existing data, thus addressing the SHA1/shattered
+ issue.
+
+ -- James McCoy <jamessan@debian.org> Sat, 30 Jun 2018 09:44:22 -0400
+
subversion (1.9.5-1+deb9u1) stretch-security; urgency=high
* patches/CVE-2017-9800: Arbitrary code execution on clients through
diff -u subversion-1.9.5/debian/patches/series subversion-1.9.5/debian/patches/series
--- subversion-1.9.5/debian/patches/series
+++ subversion-1.9.5/debian/patches/series
@@ -16,0 +17,5 @@
+apr_file_trunc_r1759116
+no-dir-rep-sharing_r1794527
+no-dir-rep-sharing_r1796725
+shattered_r1795993
+shattered_r1796470
only in patch2:
unchanged:
--- subversion-1.9.5.orig/debian/patches/apr_file_trunc_r1759116
+++ subversion-1.9.5/debian/patches/apr_file_trunc_r1759116
@@ -0,0 +1,141 @@
+------------------------------------------------------------------------
+r1759116 | stefan2 | 2016-09-03 13:47:56 -0400 (Sat, 03 Sep 2016) | 16 lines
+
+Add a workaround for yet another issue with APR's apr_file_trunc.
+
+The previous workaround is ineffective if the last file access had been
+a read. Now, we force it into to "write mode" internally to have the
+existing workaround kick in.
+
+Luckily, this only affects 'svnadmin pack' for FSFS format 7 and FSX.
+The other functions using trunc should have no problem with the added
+overhead.
+
+* subversion/libsvn_subr/io.c
+ (svn_io_file_trunc): Admend the existing workaround with a dummy-write.
+
+* subversion/tests/libsvn_subr/io-test.c
+ (test_apr_trunc_workaround): New test demonstrating the problem.
+ (test_funcs): Register the new test.
+
+Index: trunk/subversion/libsvn_subr/io.c
+===================================================================
+--- trunk/subversion/libsvn_subr/io.c (revision 1759115)
++++ trunk/subversion/libsvn_subr/io.c (revision 1759116)
+@@ -4064,6 +4064,26 @@
+ svn_error_t *
+ svn_io_file_trunc(apr_file_t *file, apr_off_t offset, apr_pool_t *pool)
+ {
++ /* Workaround for yet another APR issue with trunc.
++
++ If the APR file internally is in read mode, the current buffer pointer
++ will not be clipped to the valid data range. get_file_offset may then
++ return an invalid position *after* new data was written to it.
++
++ To prevent this, write 1 dummy byte just after the OFFSET at which we
++ will trunc it. That will force the APR file into write mode
++ internally and the flush() work-around below becomes affective. */
++ apr_off_t position = 0;
++
++ /* A frequent usage is OFFSET==0, in which case we don't need to preserve
++ any file content or file pointer. */
++ if (offset)
++ {
++ SVN_ERR(svn_io_file_seek(file, APR_CUR, &position, pool));
++ SVN_ERR(svn_io_file_seek(file, APR_SET, &offset, pool));
++ }
++ SVN_ERR(svn_io_file_putc(0, file, pool));
++
+ /* This is a work-around. APR would flush the write buffer
+ _after_ truncating the file causing now invalid buffered
+ data to be written behind OFFSET. */
+@@ -4072,10 +4092,17 @@
+ N_("Can't flush stream"),
+ pool));
+
+- return do_io_file_wrapper_cleanup(file, apr_file_trunc(file, offset),
+- N_("Can't truncate file '%s'"),
+- N_("Can't truncate stream"),
+- pool);
++ SVN_ERR(do_io_file_wrapper_cleanup(file, apr_file_trunc(file, offset),
++ N_("Can't truncate file '%s'"),
++ N_("Can't truncate stream"),
++ pool));
++
++ /* Restore original file pointer, if necessary.
++ It's currently at OFFSET. */
++ if (position < offset)
++ SVN_ERR(svn_io_file_seek(file, APR_SET, &position, pool));
++
++ return SVN_NO_ERROR;
+ }
+
+
+Index: trunk/subversion/tests/libsvn_subr/io-test.c
+===================================================================
+--- trunk/subversion/tests/libsvn_subr/io-test.c (revision 1759115)
++++ trunk/subversion/tests/libsvn_subr/io-test.c (revision 1759116)
+@@ -782,6 +782,51 @@
+
+ return SVN_NO_ERROR;
+ }
++
++static svn_error_t *
++test_apr_trunc_workaround(apr_pool_t *pool)
++{
++ const char *tmp_dir;
++ const char *tmp_file;
++ apr_file_t *f;
++ apr_size_t len;
++ apr_off_t offset;
++ char dummy;
++
++ /* create a temp folder & schedule it for automatic cleanup */
++ SVN_ERR(svn_dirent_get_absolute(&tmp_dir, "test_apr_trunc_workaround",
++ pool));
++ SVN_ERR(svn_io_remove_dir2(tmp_dir, TRUE, NULL, NULL, pool));
++ SVN_ERR(svn_io_make_dir_recursively(tmp_dir, pool));
++ svn_test_add_dir_cleanup(tmp_dir);
++
++ /* create an r/w file */
++ tmp_file = svn_dirent_join(tmp_dir, "file", pool);
++ SVN_ERR(svn_io_file_open(&f, tmp_file,
++ APR_READ | APR_WRITE | APR_BUFFERED | APR_CREATE |
++ APR_TRUNCATE,
++ APR_OS_DEFAULT, pool));
++
++ /* write some content and put it internally into read mode */
++ len = 10;
++ SVN_ERR(svn_io_file_write(f, "0123456789", &len, pool));
++
++ offset = 0;
++ SVN_ERR(svn_io_file_seek(f, APR_SET, &offset, pool));
++ SVN_ERR(svn_io_file_getc(&dummy, f, pool));
++
++ /* clear the file and write some new content */
++ SVN_ERR(svn_io_file_trunc(f, 0, pool));
++ len = 3;
++ SVN_ERR(svn_io_file_write(f, "abc", &len, pool));
++
++ /* we should now be positioned at the end of the new content */
++ offset = 0;
++ SVN_ERR(svn_io_file_seek(f, APR_CUR, &offset, pool));
++ SVN_TEST_ASSERT(offset == (int)len);
++
++ return SVN_NO_ERROR;
++}
+
+ /* The test table. */
+
+ static int max_threads = 3;
+@@ -806,6 +851,8 @@
+ "test ignore-enoent"),
+ SVN_TEST_PASS2(test_install_stream_to_longpath,
+ "test svn_stream__install_stream to long path"),
++ SVN_TEST_PASS2(test_apr_trunc_workaround,
++ "test workaround for APR in svn_io_file_trunc"),
+ SVN_TEST_NULL
+ };
+
+
+------------------------------------------------------------------------
only in patch2:
unchanged:
--- subversion-1.9.5.orig/debian/patches/no-dir-rep-sharing_r1794527
+++ subversion-1.9.5/debian/patches/no-dir-rep-sharing_r1794527
@@ -0,0 +1,157 @@
+------------------------------------------------------------------------
+r1794527 | stsp | 2017-05-09 06:35:20 -0400 (Tue, 09 May 2017) | 17 lines
+
+Reintegrate the 1.9.x-r1785053 branch.
+
+ * r1785053
+ Never attempt to share directory representations in FSFS.
+ Justification:
+ This fixes inconsistent behavior. We never add dir reps to the rep-
+ cache but would happily share any file or prop rep that happens to
+ match the respective directory rep.
+ Branch:
+ ^/subversion/branches/1.9.x-r1785053
+ Notes:
+ Will create a text conflict with the strict-rep-sharing patches.
+ Depending on which change gets merged first, the respective other
+ needs to be updated.
+ Votes:
+ +1: stefan2, rhuijben, stsp
+
+
+Index: 1.9.x/subversion/libsvn_fs_fs/transaction.c
+===================================================================
+--- 1.9.x/subversion/libsvn_fs_fs/transaction.c (revision 1794526)
++++ 1.9.x/subversion/libsvn_fs_fs/transaction.c (revision 1794527)
+@@ -2519,11 +2519,16 @@
+ /* Write out the COLLECTION as a text representation to file FILE using
+ WRITER. In the process, record position, the total size of the dump and
+ MD5 as well as SHA1 in REP. Add the representation of type ITEM_TYPE to
+- the indexes if necessary. If rep sharing has been enabled and REPS_HASH
+- is not NULL, it will be used in addition to the on-disk cache to find
+- earlier reps with the same content. When such existing reps can be
+- found, we will truncate the one just written from the file and return
+- the existing rep. Perform temporary allocations in SCRATCH_POOL. */
++ the indexes if necessary.
++
++ If ALLOW_REP_SHARING is FALSE, rep-sharing will not be used, regardless
++ of any other option and rep-sharing settings. If rep sharing has been
++ enabled and REPS_HASH is not NULL, it will be used in addition to the
++ on-disk cache to find earlier reps with the same content. If such
++ existing reps can be found, we will truncate the one just written from
++ the file and return the existing rep.
++
++ Perform temporary allocations in SCRATCH_POOL. */
+ static svn_error_t *
+ write_container_rep(representation_t *rep,
+ apr_file_t *file,
+@@ -2531,6 +2536,7 @@
+ collection_writer_t writer,
+ svn_fs_t *fs,
+ apr_hash_t *reps_hash,
++ svn_boolean_t allow_rep_sharing,
+ apr_uint32_t item_type,
+ apr_pool_t *scratch_pool)
+ {
+@@ -2537,7 +2543,7 @@
+ svn_stream_t *stream;
+ struct write_container_baton *whb;
+ svn_checksum_ctx_t *fnv1a_checksum_ctx;
+- representation_t *old_rep;
++ representation_t *old_rep = NULL;
+ apr_off_t offset = 0;
+
+ SVN_ERR(svn_fs_fs__get_file_offset(&offset, file, scratch_pool));
+@@ -2564,8 +2570,9 @@
+
+ /* Check and see if we already have a representation somewhere that's
+ identical to the one we just wrote out. */
+- SVN_ERR(get_shared_rep(&old_rep, fs, rep, reps_hash, scratch_pool,
+- scratch_pool));
++ if (allow_rep_sharing)
++ SVN_ERR(get_shared_rep(&old_rep, fs, rep, reps_hash, scratch_pool,
++ scratch_pool));
+
+ if (old_rep)
+ {
+@@ -2608,12 +2615,15 @@
+ /* Write out the COLLECTION pertaining to the NODEREV in FS as a deltified
+ text representation to file FILE using WRITER. In the process, record the
+ total size and the md5 digest in REP and add the representation of type
+- ITEM_TYPE to the indexes if necessary. If rep sharing has been enabled and
+- REPS_HASH is not NULL, it will be used in addition to the on-disk cache to
+- find earlier reps with the same content. When such existing reps can be
+- found, we will truncate the one just written from the file and return the
+- existing rep.
++ ITEM_TYPE to the indexes if necessary.
+
++ If ALLOW_REP_SHARING is FALSE, rep-sharing will not be used, regardless
++ of any other option and rep-sharing settings. If rep sharing has been
++ enabled and REPS_HASH is not NULL, it will be used in addition to the
++ on-disk cache to find earlier reps with the same content. If such
++ existing reps can be found, we will truncate the one just written from
++ the file and return the existing rep.
++
+ If ITEM_TYPE is IS_PROPS equals SVN_FS_FS__ITEM_TYPE_*_PROPS, assume
+ that we want to a props representation as the base for our delta.
+ Perform temporary allocations in SCRATCH_POOL.
+@@ -2626,6 +2636,7 @@
+ svn_fs_t *fs,
+ node_revision_t *noderev,
+ apr_hash_t *reps_hash,
++ svn_boolean_t allow_rep_sharing,
+ apr_uint32_t item_type,
+ apr_pool_t *scratch_pool)
+ {
+@@ -2635,7 +2646,7 @@
+ svn_stream_t *file_stream;
+ svn_stream_t *stream;
+ representation_t *base_rep;
+- representation_t *old_rep;
++ representation_t *old_rep = NULL;
+ svn_checksum_ctx_t *fnv1a_checksum_ctx;
+ svn_stream_t *source;
+ svn_fs_fs__rep_header_t header = { 0 };
+@@ -2703,8 +2714,9 @@
+
+ /* Check and see if we already have a representation somewhere that's
+ identical to the one we just wrote out. */
+- SVN_ERR(get_shared_rep(&old_rep, fs, rep, reps_hash, scratch_pool,
+- scratch_pool));
++ if (allow_rep_sharing)
++ SVN_ERR(get_shared_rep(&old_rep, fs, rep, reps_hash, scratch_pool,
++ scratch_pool));
+
+ if (old_rep)
+ {
+@@ -2920,13 +2932,14 @@
+ SVN_ERR(write_container_delta_rep(noderev->data_rep, file,
+ entries,
+ write_directory_to_stream,
+- fs, noderev, NULL,
++ fs, noderev, NULL, FALSE,
+ SVN_FS_FS__ITEM_TYPE_DIR_REP,
+ pool));
+ else
+ SVN_ERR(write_container_rep(noderev->data_rep, file, entries,
+ write_directory_to_stream, fs, NULL,
+- SVN_FS_FS__ITEM_TYPE_DIR_REP, pool));
++ FALSE, SVN_FS_FS__ITEM_TYPE_DIR_REP,
++ pool));
+
+ reset_txn_in_rep(noderev->data_rep);
+ }
+@@ -2971,11 +2984,11 @@
+ if (ffd->deltify_properties)
+ SVN_ERR(write_container_delta_rep(noderev->prop_rep, file, proplist,
+ write_hash_to_stream, fs, noderev,
+- reps_hash, item_type, pool));
++ reps_hash, TRUE, item_type, pool));
+ else
+ SVN_ERR(write_container_rep(noderev->prop_rep, file, proplist,
+ write_hash_to_stream, fs, reps_hash,
+- item_type, pool));
++ item_type, TRUE, pool));
+
+ reset_txn_in_rep(noderev->prop_rep);
+ }
only in patch2:
unchanged:
--- subversion-1.9.5.orig/debian/patches/no-dir-rep-sharing_r1796725
+++ subversion-1.9.5/debian/patches/no-dir-rep-sharing_r1796725
@@ -0,0 +1,29 @@
+------------------------------------------------------------------------
+r1796725 | svn-role | 2017-05-30 00:00:07 -0400 (Tue, 30 May 2017) | 12 lines
+
+Merge r1796158 from trunk:
+
+ * r1796158
+ Fix FSFS f7 metadata recording issue caused by the r1785053 backport.
+ Justification:
+ An obvious fix (one caller got the order of parameters mixed up).
+ The bug is benign as non-deltifying property representations get
+ marked as "file data reps". This happens to be a valid state due
+ to potential rep sharing, but defeats that is just lucky.
+ Votes:
+ +1: stefan2, danielsh, rhuijben
+
+
+Index: 1.9.x/subversion/libsvn_fs_fs/transaction.c
+===================================================================
+--- 1.9.x/subversion/libsvn_fs_fs/transaction.c (revision 1796724)
++++ 1.9.x/subversion/libsvn_fs_fs/transaction.c (revision 1796725)
+@@ -3062,7 +3062,7 @@
+ else
+ SVN_ERR(write_container_rep(noderev->prop_rep, file, proplist,
+ write_hash_to_stream, fs, reps_hash,
+- item_type, TRUE, pool));
++ TRUE, item_type, pool));
+
+ reset_txn_in_rep(noderev->prop_rep);
+ }
only in patch2:
unchanged:
--- subversion-1.9.5.orig/debian/patches/shattered_r1795993
+++ subversion-1.9.5/debian/patches/shattered_r1795993
@@ -0,0 +1,491 @@
+------------------------------------------------------------------------
+r1795993 | svn-role | 2017-05-24 00:00:11 -0400 (Wed, 24 May 2017) | 18 lines
+
+Merge the 1.9.x-strict-rep-sharing branch:
+
+ * r1785737, r1785738, r1785734, r1786447, r1785754, r1786445, r1786446, r1786515, r1794530, r1794536, r1794611
+ Make FSFS consistency no longer depend on hash algorithms.
+ Justification:
+ This eliminates any existing or future FSFS vulnerability due to
+ attacks on MD5 or SHA1.
+ Branch:
+ ^/subversion/branches/1.9.x-strict-rep-sharing
+ Notes:
+ Depends on r1759116 for correctness with older APR.
+ While the backport code is very close to the /trunk changes, it is
+ easier to review them as r1786580, r1786581 and r1786619 on the branch.
+ Will create a text conflict with the r1785053 backport. Depending on
+ which change gets merged first, the respective other must be updated.
+ Votes:
+ +1: stsp, stefan2, rhuijben
+
+
+Index: 1.9.x/subversion/libsvn_fs_fs/cached_data.h
+===================================================================
+--- 1.9.x/subversion/libsvn_fs_fs/cached_data.h (revision 1795992)
++++ 1.9.x/subversion/libsvn_fs_fs/cached_data.h (revision 1795993)
+@@ -81,6 +81,18 @@
+ svn_boolean_t cache_fulltext,
+ apr_pool_t *pool);
+
++/* Set *CONTENTS_P to be a readable svn_stream_t that receives the text
++ representation REP as seen in filesystem FS. Read the latest element
++ of the delta chain from FILE at offset OFFSET.
++ Use POOL for allocations. */
++svn_error_t *
++svn_fs_fs__get_contents_from_file(svn_stream_t **contents_p,
++ svn_fs_t *fs,
++ representation_t *rep,
++ apr_file_t *file,
++ apr_off_t offset,
++ apr_pool_t *pool);
++
+ /* Attempt to fetch the text representation of node-revision NODEREV as
+ seen in filesystem FS and pass it along with the BATON to the PROCESSOR.
+ Set *SUCCESS only of the data could be provided and the processing
+Index: 1.9.x/subversion/libsvn_fs_fs/cached_data.c
+===================================================================
+--- 1.9.x/subversion/libsvn_fs_fs/cached_data.c (revision 1795992)
++++ 1.9.x/subversion/libsvn_fs_fs/cached_data.c (revision 1795993)
+@@ -2029,8 +2029,13 @@
+ SVN_ERR(skip_contents(rb, rb->fulltext_delivered));
+ }
+
+- /* Get the next block of data. */
+- SVN_ERR(get_contents_from_windows(rb, buf, len));
++ /* Get the next block of data.
++ * Keep in mind that the representation might be empty and leave us
++ * already positioned at the end of the rep. */
++ if (rb->off == rb->len)
++ *len = 0;
++ else
++ SVN_ERR(get_contents_from_windows(rb, buf, len));
+
+ if (rb->current_fulltext)
+ svn_stringbuf_appendbytes(rb->current_fulltext, buf, *len);
+@@ -2123,6 +2128,96 @@
+ return SVN_NO_ERROR;
+ }
+
++svn_error_t *
++svn_fs_fs__get_contents_from_file(svn_stream_t **contents_p,
++ svn_fs_t *fs,
++ representation_t *rep,
++ apr_file_t *file,
++ apr_off_t offset,
++ apr_pool_t *pool)
++{
++ struct rep_read_baton *rb;
++ pair_cache_key_t fulltext_cache_key = { SVN_INVALID_REVNUM, 0 };
++ rep_state_t *rs = apr_pcalloc(pool, sizeof(*rs));
++ svn_fs_fs__rep_header_t *rh;
++
++ /* Initialize the reader baton. Some members may added lazily
++ * while reading from the stream. */
++ SVN_ERR(rep_read_get_baton(&rb, fs, rep, fulltext_cache_key, pool));
++
++ /* Continue constructing RS. Leave caches as NULL. */
++ rs->size = rep->size;
++ rs->revision = SVN_INVALID_REVNUM;
++ rs->item_index = 0;
++ rs->ver = -1;
++ rs->start = -1;
++
++ /* Provide just enough file access info to allow for a basic read from
++ * FILE but leave all index / footer info with empty values b/c FILE
++ * probably is not a complete revision file. */
++ rs->sfile = apr_pcalloc(pool, sizeof(*rs->sfile));
++ rs->sfile->revision = rep->revision;
++ rs->sfile->pool = pool;
++ rs->sfile->fs = fs;
++ rs->sfile->rfile = apr_pcalloc(pool, sizeof(*rs->sfile->rfile));
++ rs->sfile->rfile->start_revision = SVN_INVALID_REVNUM;
++ rs->sfile->rfile->file = file;
++ rs->sfile->rfile->stream = svn_stream_from_aprfile2(file, TRUE, pool);
++
++ /* Read the rep header. */
++ SVN_ERR(aligned_seek(fs, file, NULL, offset, pool));
++ SVN_ERR(svn_fs_fs__read_rep_header(&rh, rs->sfile->rfile->stream,
++ pool, pool));
++ SVN_ERR(get_file_offset(&rs->start, rs, pool));
++ rs->header_size = rh->header_size;
++
++ /* Log the access. */
++ SVN_ERR(dbg_log_access(fs, SVN_INVALID_REVNUM, 0, rh,
++ SVN_FS_FS__ITEM_TYPE_ANY_REP, pool));
++
++ /* Build the representation list (delta chain). */
++ if (rh->type == svn_fs_fs__rep_plain)
++ {
++ rb->rs_list = apr_array_make(pool, 0, sizeof(rep_state_t *));
++ rb->src_state = rs;
++ }
++ else if (rh->type == svn_fs_fs__rep_self_delta)
++ {
++ rb->rs_list = apr_array_make(pool, 1, sizeof(rep_state_t *));
++ APR_ARRAY_PUSH(rb->rs_list, rep_state_t *) = rs;
++ rb->src_state = NULL;
++ }
++ else
++ {
++ representation_t next_rep = { 0 };
++
++ /* skip "SVNx" diff marker */
++ rs->current = 4;
++
++ /* REP's base rep is inside a proper revision.
++ * It can be reconstructed in the usual way. */
++ next_rep.revision = rh->base_revision;
++ next_rep.item_index = rh->base_item_index;
++ next_rep.size = rh->base_length;
++ svn_fs_fs__id_txn_reset(&next_rep.txn_id);
++
++ SVN_ERR(build_rep_list(&rb->rs_list, &rb->base_window,
++ &rb->src_state, &rb->len, rb->fs, &next_rep,
++ rb->filehandle_pool));
++
++ /* Insert the access to REP as the first element of the delta chain. */
++ svn_sort__array_insert(rb->rs_list, &rs, 0);
++ }
++
++ /* Now, the baton is complete and we can assemble the stream around it. */
++ *contents_p = svn_stream_create(rb, pool);
++ svn_stream_set_read2(*contents_p, NULL /* only full read support */,
++ rep_read_contents);
++ svn_stream_set_close(*contents_p, rep_read_contents_close);
++
++ return SVN_NO_ERROR;
++}
++
+ /* Baton for cache_access_wrapper. Wraps the original parameters of
+ * svn_fs_fs__try_process_file_content().
+ */
+Index: 1.9.x/subversion/libsvn_fs_fs/transaction.c
+===================================================================
+--- 1.9.x/subversion/libsvn_fs_fs/transaction.c (revision 1795992)
++++ 1.9.x/subversion/libsvn_fs_fs/transaction.c (revision 1795993)
+@@ -2128,6 +2128,11 @@
+ there may be new duplicate representations within the same uncommitted
+ revision, those can be passed in REPS_HASH (maps a sha1 digest onto
+ representation_t*), otherwise pass in NULL for REPS_HASH.
++
++ The content of both representations will be compared, taking REP's content
++ from FILE at OFFSET. Only if they actually match, will *OLD_REP not be
++ NULL.
++
+ Use RESULT_POOL for *OLD_REP allocations and SCRATCH_POOL for temporaries.
+ The lifetime of *OLD_REP is limited by both, RESULT_POOL and REP lifetime.
+ */
+@@ -2135,6 +2140,8 @@
+ get_shared_rep(representation_t **old_rep,
+ svn_fs_t *fs,
+ representation_t *rep,
++ apr_file_t *file,
++ apr_off_t offset,
+ apr_hash_t *reps_hash,
+ apr_pool_t *result_pool,
+ apr_pool_t *scratch_pool)
+@@ -2142,6 +2149,10 @@
+ svn_error_t *err;
+ fs_fs_data_t *ffd = fs->fsap_data;
+
++ svn_checksum_t checksum;
++ checksum.digest = rep->sha1_digest;
++ checksum.kind = svn_checksum_sha1;
++
+ /* Return NULL, if rep sharing has been disabled. */
+ *old_rep = NULL;
+ if (!ffd->rep_sharing_allowed)
+@@ -2158,9 +2169,6 @@
+ /* If we haven't found anything yet, try harder and consult our DB. */
+ if (*old_rep == NULL)
+ {
+- svn_checksum_t checksum;
+- checksum.digest = rep->sha1_digest;
+- checksum.kind = svn_checksum_sha1;
+ err = svn_fs_fs__get_rep_reference(old_rep, fs, &checksum, result_pool);
+ /* ### Other error codes that we shouldn't mask out? */
+ if (err == SVN_NO_ERROR)
+@@ -2235,6 +2243,72 @@
+ (*old_rep)->uniquifier = rep->uniquifier;
+ }
+
++ /* If we (very likely) found a matching representation, compare the actual
++ * contents such that we can be sure that no rep-cache.db corruption or
++ * hash collision produced a false positive. */
++ if (*old_rep)
++ {
++ apr_off_t old_position;
++ svn_stream_t *contents;
++ svn_stream_t *old_contents;
++ svn_boolean_t same;
++
++ /* The existing representation may itself be part of the current
++ * transaction. In that case, it may be in different stages of
++ * the commit finalization process.
++ *
++ * OLD_REP_NORM is the same as that OLD_REP but it is assigned
++ * explicitly to REP's transaction if OLD_REP does not point
++ * to an already committed revision. This then prevents the
++ * revision lookup and the txn data will be accessed.
++ */
++ representation_t old_rep_norm = **old_rep;
++ if ( !SVN_IS_VALID_REVNUM(old_rep_norm.revision)
++ || old_rep_norm.revision > ffd->youngest_rev_cache)
++ old_rep_norm.txn_id = rep->txn_id;
++
++ /* Make sure we can later restore FILE's current position. */
++ SVN_ERR(svn_fs_fs__get_file_offset(&old_position, file, scratch_pool));
++
++ /* Compare the two representations.
++ * Note that the stream comparison might also produce MD5 checksum
++ * errors or other failures in case of SHA1 collisions. */
++ SVN_ERR(svn_fs_fs__get_contents_from_file(&contents, fs, rep, file,
++ offset, scratch_pool));
++ SVN_ERR(svn_fs_fs__get_contents(&old_contents, fs, &old_rep_norm,
++ FALSE, scratch_pool));
++ err = svn_stream_contents_same2(&same, contents, old_contents,
++ scratch_pool);
++
++ /* A mismatch should be extremely rare.
++ * If it does happen, reject the commit. */
++ if (!same || err)
++ {
++ /* SHA1 collision or worse. */
++ svn_stringbuf_t *old_rep_str
++ = svn_fs_fs__unparse_representation(*old_rep,
++ ffd->format, FALSE,
++ scratch_pool,
++ scratch_pool);
++ svn_stringbuf_t *rep_str
++ = svn_fs_fs__unparse_representation(rep,
++ ffd->format, FALSE,
++ scratch_pool,
++ scratch_pool);
++ const char *checksum__str
++ = svn_checksum_to_cstring_display(&checksum, scratch_pool);
++
++ return svn_error_createf(SVN_ERR_FS_GENERAL,
++ err, "SHA1 of reps '%s' and '%s' "
++ "matches (%s) but contents differ",
++ old_rep_str->data, rep_str->data,
++ checksum__str);
++ }
++
++ /* Restore FILE's read / write position. */
++ SVN_ERR(svn_io_file_seek(file, APR_SET, &old_position, scratch_pool));
++ }
++
+ return SVN_NO_ERROR;
+ }
+
+@@ -2293,8 +2367,8 @@
+
+ /* Check and see if we already have a representation somewhere that's
+ identical to the one we just wrote out. */
+- SVN_ERR(get_shared_rep(&old_rep, b->fs, rep, NULL, b->result_pool,
+- b->scratch_pool));
++ SVN_ERR(get_shared_rep(&old_rep, b->fs, rep, b->file, b->rep_offset, NULL,
++ b->result_pool, b->scratch_pool));
+
+ if (old_rep)
+ {
+@@ -2543,7 +2617,6 @@
+ svn_stream_t *stream;
+ struct write_container_baton *whb;
+ svn_checksum_ctx_t *fnv1a_checksum_ctx;
+- representation_t *old_rep = NULL;
+ apr_off_t offset = 0;
+
+ SVN_ERR(svn_fs_fs__get_file_offset(&offset, file, scratch_pool));
+@@ -2568,19 +2641,26 @@
+ /* Store the results. */
+ SVN_ERR(digests_final(rep, whb->md5_ctx, whb->sha1_ctx, scratch_pool));
+
++ /* Update size info. */
++ rep->expanded_size = whb->size;
++ rep->size = whb->size;
++
+ /* Check and see if we already have a representation somewhere that's
+ identical to the one we just wrote out. */
+ if (allow_rep_sharing)
+- SVN_ERR(get_shared_rep(&old_rep, fs, rep, reps_hash, scratch_pool,
+- scratch_pool));
+-
+- if (old_rep)
+ {
+- /* We need to erase from the protorev the data we just wrote. */
+- SVN_ERR(svn_io_file_trunc(file, offset, scratch_pool));
++ representation_t *old_rep;
++ SVN_ERR(get_shared_rep(&old_rep, fs, rep, file, offset, reps_hash,
++ scratch_pool, scratch_pool));
+
+- /* Use the old rep for this content. */
+- memcpy(rep, old_rep, sizeof (*rep));
++ if (old_rep)
++ {
++ /* We need to erase from the protorev the data we just wrote. */
++ SVN_ERR(svn_io_file_trunc(file, offset, scratch_pool));
++
++ /* Use the old rep for this content. */
++ memcpy(rep, old_rep, sizeof (*rep));
++ }
+ }
+ else
+ {
+@@ -2603,10 +2683,6 @@
+ scratch_pool));
+
+ SVN_ERR(store_p2l_index_entry(fs, &rep->txn_id, &entry, scratch_pool));
+-
+- /* update the representation */
+- rep->size = whb->size;
+- rep->expanded_size = whb->size;
+ }
+
+ return SVN_NO_ERROR;
+@@ -2646,7 +2722,6 @@
+ svn_stream_t *file_stream;
+ svn_stream_t *stream;
+ representation_t *base_rep;
+- representation_t *old_rep = NULL;
+ svn_checksum_ctx_t *fnv1a_checksum_ctx;
+ svn_stream_t *source;
+ svn_fs_fs__rep_header_t header = { 0 };
+@@ -2712,19 +2787,27 @@
+ /* Store the results. */
+ SVN_ERR(digests_final(rep, whb->md5_ctx, whb->sha1_ctx, scratch_pool));
+
++ /* Update size info. */
++ SVN_ERR(svn_fs_fs__get_file_offset(&rep_end, file, scratch_pool));
++ rep->size = rep_end - delta_start;
++ rep->expanded_size = whb->size;
++
+ /* Check and see if we already have a representation somewhere that's
+ identical to the one we just wrote out. */
+ if (allow_rep_sharing)
+- SVN_ERR(get_shared_rep(&old_rep, fs, rep, reps_hash, scratch_pool,
+- scratch_pool));
+-
+- if (old_rep)
+ {
+- /* We need to erase from the protorev the data we just wrote. */
+- SVN_ERR(svn_io_file_trunc(file, offset, scratch_pool));
++ representation_t *old_rep;
++ SVN_ERR(get_shared_rep(&old_rep, fs, rep, file, offset, reps_hash,
++ scratch_pool, scratch_pool));
+
+- /* Use the old rep for this content. */
+- memcpy(rep, old_rep, sizeof (*rep));
++ if (old_rep)
++ {
++ /* We need to erase from the protorev the data we just wrote. */
++ SVN_ERR(svn_io_file_trunc(file, offset, scratch_pool));
++
++ /* Use the old rep for this content. */
++ memcpy(rep, old_rep, sizeof (*rep));
++ }
+ }
+ else
+ {
+@@ -2731,7 +2814,6 @@
+ svn_fs_fs__p2l_entry_t entry;
+
+ /* Write out our cosmetic end marker. */
+- SVN_ERR(svn_fs_fs__get_file_offset(&rep_end, file, scratch_pool));
+ SVN_ERR(svn_stream_puts(file_stream, "ENDREP\n"));
+
+ SVN_ERR(allocate_item_index(&rep->item_index, fs, &rep->txn_id,
+@@ -2748,10 +2830,6 @@
+ scratch_pool));
+
+ SVN_ERR(store_p2l_index_entry(fs, &rep->txn_id, &entry, scratch_pool));
+-
+- /* update the representation */
+- rep->expanded_size = whb->size;
+- rep->size = rep_end - delta_start;
+ }
+
+ return SVN_NO_ERROR;
+Index: 1.9.x/subversion/tests/libsvn_fs/fs-test.c
+===================================================================
+--- 1.9.x/subversion/tests/libsvn_fs/fs-test.c (revision 1795992)
++++ 1.9.x/subversion/tests/libsvn_fs/fs-test.c (revision 1795993)
+@@ -7106,6 +7106,67 @@
+ return SVN_NO_ERROR;
+ }
+
++static svn_error_t *
++test_rep_sharing_strict_content_check(const svn_test_opts_t *opts,
++ apr_pool_t *pool)
++{
++ svn_fs_t *fs;
++ svn_fs_txn_t *txn;
++ svn_fs_root_t *txn_root;
++ svn_revnum_t new_rev;
++ const char *fs_path, *fs_path2;
++ apr_pool_t *subpool = svn_pool_create(pool);
++ svn_error_t *err;
++
++ /* Bail (with success) on known-untestable scenarios */
++ if (strcmp(opts->fs_type, SVN_FS_TYPE_BDB) == 0)
++ return svn_error_create(SVN_ERR_TEST_SKIPPED, NULL,
++ "BDB repositories don't support rep-sharing");
++
++ /* Create 2 repos with same structure & size but different contents */
++ fs_path = "test-rep-sharing-strict-content-check1";
++ fs_path2 = "test-rep-sharing-strict-content-check2";
++
++ SVN_ERR(svn_test__create_fs(&fs, fs_path, opts, subpool));
++
++ SVN_ERR(svn_fs_begin_txn2(&txn, fs, 0, 0, subpool));
++ SVN_ERR(svn_fs_txn_root(&txn_root, txn, subpool));
++ SVN_ERR(svn_fs_make_file(txn_root, "/foo", subpool));
++ SVN_ERR(svn_test__set_file_contents(txn_root, "foo", "quite bad", subpool));
++ SVN_ERR(test_commit_txn(&new_rev, txn, NULL, subpool));
++ SVN_TEST_INT_ASSERT(new_rev, 1);
++
++ SVN_ERR(svn_test__create_fs(&fs, fs_path2, opts, subpool));
++
++ SVN_ERR(svn_fs_begin_txn2(&txn, fs, 0, 0, subpool));
++ SVN_ERR(svn_fs_txn_root(&txn_root, txn, subpool));
++ SVN_ERR(svn_fs_make_file(txn_root, "foo", subpool));
++ SVN_ERR(svn_test__set_file_contents(txn_root, "foo", "very good", subpool));
++ SVN_ERR(test_commit_txn(&new_rev, txn, NULL, subpool));
++ SVN_TEST_INT_ASSERT(new_rev, 1);
++
++ /* Close both repositories. */
++ svn_pool_clear(subpool);
++
++ /* Doctor the first repo such that it uses the wrong rep-cache. */
++ SVN_ERR(svn_io_copy_file(svn_relpath_join(fs_path2, "rep-cache.db", pool),
++ svn_relpath_join(fs_path, "rep-cache.db", pool),
++ FALSE, pool));
++
++ /* Changing the file contents such that rep-sharing would kick in if
++ the file contents was not properly compared. */
++ SVN_ERR(svn_fs_open2(&fs, fs_path, NULL, subpool, subpool));
++
++ SVN_ERR(svn_fs_begin_txn2(&txn, fs, 1, 0, subpool));
++ SVN_ERR(svn_fs_txn_root(&txn_root, txn, subpool));
++ err = svn_test__set_file_contents(txn_root, "foo", "very good", subpool);
++ SVN_TEST_ASSERT_ERROR(err, SVN_ERR_FS_GENERAL);
++
++ svn_pool_destroy(subpool);
++
++ return SVN_NO_ERROR;
++}
++
+ /* ------------------------------------------------------------------------ */
+
+ /* The test table. */
+@@ -7245,6 +7306,9 @@
+ "freeze and commit"),
+ SVN_TEST_OPTS_PASS(commit_with_locked_rep_cache,
+ "test commit with locked rep-cache"),
++ SVN_TEST_OPTS_XFAIL_OTOH(test_rep_sharing_strict_content_check,
++ "test rep-sharing on content rather than SHA1",
++ SVN_TEST_PASS_IF_FS_TYPE_IS(SVN_FS_TYPE_FSFS)),
+ SVN_TEST_NULL
+ };
+
only in patch2:
unchanged:
--- subversion-1.9.5.orig/debian/patches/shattered_r1796470
+++ subversion-1.9.5/debian/patches/shattered_r1796470
@@ -0,0 +1,127 @@
+------------------------------------------------------------------------
+r1796470 | svn-role | 2017-05-28 00:00:08 -0400 (Sun, 28 May 2017) | 11 lines
+
+Merge the 1.9.x-fix-fsfs branch:
+
+ * r1796143
+ Fix the FSFS breakage caused by the latest SHA-1-related merges.
+ Justification:
+ FSFS is basically broken (~400 tests failing).
+ Branch:
+ ^/subversion/branches/1.9.x-fix-fsfs
+ Votes:
+ +1: stefan2, stsp, danielsh
+
+
+Index: 1.9.x/subversion/libsvn_fs_fs/transaction.c
+===================================================================
+--- 1.9.x/subversion/libsvn_fs_fs/transaction.c (revision 1796469)
++++ 1.9.x/subversion/libsvn_fs_fs/transaction.c (revision 1796470)
+@@ -2618,6 +2618,7 @@
+ struct write_container_baton *whb;
+ svn_checksum_ctx_t *fnv1a_checksum_ctx;
+ apr_off_t offset = 0;
++ svn_fs_fs__p2l_entry_t entry;
+
+ SVN_ERR(svn_fs_fs__get_file_offset(&offset, file, scratch_pool));
+
+@@ -2660,30 +2661,27 @@
+
+ /* Use the old rep for this content. */
+ memcpy(rep, old_rep, sizeof (*rep));
++ return SVN_NO_ERROR;
+ }
+ }
+- else
+- {
+- svn_fs_fs__p2l_entry_t entry;
+
+- /* Write out our cosmetic end marker. */
+- SVN_ERR(svn_stream_puts(whb->stream, "ENDREP\n"));
++ /* Write out our cosmetic end marker. */
++ SVN_ERR(svn_stream_puts(whb->stream, "ENDREP\n"));
+
+- SVN_ERR(allocate_item_index(&rep->item_index, fs, &rep->txn_id,
+- offset, scratch_pool));
++ SVN_ERR(allocate_item_index(&rep->item_index, fs, &rep->txn_id,
++ offset, scratch_pool));
+
+- entry.offset = offset;
+- SVN_ERR(svn_fs_fs__get_file_offset(&offset, file, scratch_pool));
+- entry.size = offset - entry.offset;
+- entry.type = item_type;
+- entry.item.revision = SVN_INVALID_REVNUM;
+- entry.item.number = rep->item_index;
+- SVN_ERR(fnv1a_checksum_finalize(&entry.fnv1_checksum,
+- fnv1a_checksum_ctx,
+- scratch_pool));
++ entry.offset = offset;
++ SVN_ERR(svn_fs_fs__get_file_offset(&offset, file, scratch_pool));
++ entry.size = offset - entry.offset;
++ entry.type = item_type;
++ entry.item.revision = SVN_INVALID_REVNUM;
++ entry.item.number = rep->item_index;
++ SVN_ERR(fnv1a_checksum_finalize(&entry.fnv1_checksum,
++ fnv1a_checksum_ctx,
++ scratch_pool));
+
+- SVN_ERR(store_p2l_index_entry(fs, &rep->txn_id, &entry, scratch_pool));
+- }
++ SVN_ERR(store_p2l_index_entry(fs, &rep->txn_id, &entry, scratch_pool));
+
+ return SVN_NO_ERROR;
+ }
+@@ -2725,6 +2723,7 @@
+ svn_checksum_ctx_t *fnv1a_checksum_ctx;
+ svn_stream_t *source;
+ svn_fs_fs__rep_header_t header = { 0 };
++ svn_fs_fs__p2l_entry_t entry;
+
+ apr_off_t rep_end = 0;
+ apr_off_t delta_start = 0;
+@@ -2807,30 +2806,27 @@
+
+ /* Use the old rep for this content. */
+ memcpy(rep, old_rep, sizeof (*rep));
++ return SVN_NO_ERROR;
+ }
+ }
+- else
+- {
+- svn_fs_fs__p2l_entry_t entry;
+
+- /* Write out our cosmetic end marker. */
+- SVN_ERR(svn_stream_puts(file_stream, "ENDREP\n"));
++ /* Write out our cosmetic end marker. */
++ SVN_ERR(svn_stream_puts(file_stream, "ENDREP\n"));
+
+- SVN_ERR(allocate_item_index(&rep->item_index, fs, &rep->txn_id,
+- offset, scratch_pool));
++ SVN_ERR(allocate_item_index(&rep->item_index, fs, &rep->txn_id,
++ offset, scratch_pool));
+
+- entry.offset = offset;
+- SVN_ERR(svn_fs_fs__get_file_offset(&offset, file, scratch_pool));
+- entry.size = offset - entry.offset;
+- entry.type = item_type;
+- entry.item.revision = SVN_INVALID_REVNUM;
+- entry.item.number = rep->item_index;
+- SVN_ERR(fnv1a_checksum_finalize(&entry.fnv1_checksum,
+- fnv1a_checksum_ctx,
+- scratch_pool));
++ entry.offset = offset;
++ SVN_ERR(svn_fs_fs__get_file_offset(&offset, file, scratch_pool));
++ entry.size = offset - entry.offset;
++ entry.type = item_type;
++ entry.item.revision = SVN_INVALID_REVNUM;
++ entry.item.number = rep->item_index;
++ SVN_ERR(fnv1a_checksum_finalize(&entry.fnv1_checksum,
++ fnv1a_checksum_ctx,
++ scratch_pool));
+
+- SVN_ERR(store_p2l_index_entry(fs, &rep->txn_id, &entry, scratch_pool));
+- }
++ SVN_ERR(store_p2l_index_entry(fs, &rep->txn_id, &entry, scratch_pool));
+
+ return SVN_NO_ERROR;
+ }
Reply to: