Issue 119090 - Default Encryption Fails for Down-Level Implementations
Default Encryption Fails for Down-Level Implementations
Status: RESOLVED FIXED
Product: General
Classification: Code
Component: security
3.4.1
PC All
: P3 normal (vote)
: ---
Assigned To: jsc
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-03-18 23:48 UTC by orcmid
Modified: 2013-02-21 23:37 UTC (History)
6 users (show)

See Also:
Issue Type: DEFECT
Latest Confirmation on: ---
Developer Difficulty: ---
jsc: 3.4_release_blocker+


Attachments
This is a typical failure when an ODF 1.2 document is encrypted by default with an encryption method not recognized by downlevel products (37.73 KB, image/png)
2012-03-18 23:48 UTC, orcmid
no flags Details
Change saveopt.cxx so that both UseSHA1InODF2 and UseBlowfishInODF2 are true by default (1.60 KB, patch)
2012-03-20 01:39 UTC, orcmid
orcmid: review?
Details | Diff

Note You need to log in before you can comment on or make changes to this issue.
Description orcmid 2012-03-18 23:48:09 UTC
Created attachment 77341 [details]
This is a typical failure when an ODF 1.2 document is encrypted by default with an encryption method not recognized by downlevel products

The default encryption used by AOO3.4-dev r1293550 for ODF 1.2 (extended) documents fails in all down-level implementations except for the Oracle 3.4-dev release and LibreOffice releases in the LO 3.5.x line.

The following implementations are unable to open the encryption that is done when the document is saved as ODF 1.2 (extended), the default installation Load/Save option:

LO 3.4.3 and LO 3.3.2
OpenOffice.org 3.3.0 (the last stable release)
Lotus Symphony 3.0.1

All fail with a complaint of the type

"Read-Error.  Format error discovered in the file in sub-document styles.xml  at 1.0(row,col) "

It appears that the down-level implementations fail to recognize that the document is encrypted and rather than fail at decryption, the document is processed as unencrypted, which is bound to fail parsing of the sub-documents.

When the Tools | Options | Load/Save option is altered to save documents as ODF 1.0/1.1, all of the tested products were able to open the resulting encryption without difficulty (given the correct password of course):

AOO-dev 3.4 r1293550
LO 3.5.0rc3
LO 3.4.3
LO 3.3.2
OOo-dev 3.4.0 (from Oracle)
OOo 3.3.0
Lotus Symphony 3.0.1
Comment 1 orcmid 2012-03-18 23:50:46 UTC
This might also be related to problems that the reporter of https://issues.apache.org/ooo/show_bug.cgi?id=119089 is experiencing, although the symptoms do not appear to be the same.
Comment 2 orcmid 2012-03-19 00:02:29 UTC
The OASIS Standard ODF 1.2 Part 3 specifies the following conformance requirements with regard to choice of encryption methods (section 4.8.1):

"Package producers that support encryption shall support the value Blowfish CFB. Package consumers that support encryption shall support the values Blowfish CFB and urn:oasis:names:tc:opendocument:xmlns:manifest:1.0#blowfish."

Although other algorithms are allowed for ODF 1.2, these are the only ones that are required to be supported for ODF 1.2 conformance.

Furthermore, the one document encryption method defined for ODF 1.0/1.1 is fully allowable in ODF 1.2, in exactly the form provided in ODF 1.0/1.1.

RECOMMENDATION

Have the automatic choice of encryption methods and parameters be the same as when the document is saved in ODF 1.0/1.1 format.  Do this even when Load/Save is set to ODF 1.2 or ODF 1.2 (extended).  

Still *ACCEPT* (but do not produce) any of the additional encryption methods and parameters including (1) SHA256 start-key digests, SHA256-1k confirmation checksums, and at least AES256-CBC among the other AES and Triple-DES algorithms allowed in accordance with section 5.2 of [xmlenc-core].

*AFTER* AOO 3.4, consider whether or not to allow user-controlled choice of a different encryption during a Save with Password operation. At that time, it can be determined how to advise users about the optional use and limitation to consumers that support the same optional parameters.  This would be something useful to coordinate with LibreOffice support for optional ODF 1.2 encryption cases as well.
Comment 3 Oliver-Rainer Wittmann 2012-03-19 08:23:22 UTC
no release blocker from my point of view
Comment 4 Andre 2012-03-19 08:35:25 UTC
I do not see this as a blocker either.
All of the behavior described above seems to be according to specification.

Although, I do not think that having optional encryption methods in an office suite is a good idea.
Comment 5 orcmid 2012-03-19 15:03:59 UTC
Why is it not a blocker to release code that will, by default and without notice, change the encryption method used, so that the document will be unreadable by anyone who has the key but is using a version that does not recognize that optional encryption instead of the only one that is required to be supported for both consumption and production?

The recommendation is to accept the optional ones that are allowed in conforming implementations (since some will be in the wild as LO 3.5.x goes into use) but to only produce the one form that is assured to roundtrip among all known ODF 1.0/1.1/1.2 consumers that support encryption?
Comment 6 Rob Weir 2012-03-19 16:23:01 UTC
I think this is a security versus interop trade-off.

It is interesting to note that the author of the Blowfish algorithm actually recommends not to use it anymore:

http://www.computerworld.com.au/article/46254/bruce_almighty_schneier_preaches_security_linux_faithful/?pp=3

He wrote TwoFish to replace Blowfish. Twofish was submitted in the competition to replace the older DES standard.  AES was picked as the winner of that competition, not Twofish.  So from a security standpoint, our default of AES is the best choice.

But it does have the interop downside that Dennis has mentioned.  But I think this can be handled by user education.  If users want to share encrypted files with older editors, they can:

1) Save As ODF 1.0/1.1

or

2) Set a configuration entry to make Blowfish the default.
Comment 7 orcmid 2012-03-19 16:31:26 UTC
Perhaps my report is not clear.  The current AOO 3.4 previews use AES256-cbc
encryption automatically.  Users are not given any choice.

No AES256-cbc encryption is handled by any distributions other than OOo-dev 3.4
(not a stable release) and recent releases in the LibreOffice 3.5.x line.  The
AES256-cbc encryption is not recognized and cannot be decrypted on earlier
releases of any OpenOffice-lineage distributions, including OOo 3.3.0,
LibreOffice through 3.4.3 (at least), and Lotus Symphony 3.0.1.  The failure is
reported with a misleading error message that suggests the documents styles.xml
is damaged, not that it can't be decrypted.

On the other hand, all known releases that support encryption will accept the
Blowfish CFB encryption exactly as they are produced for ODF 1.0, 1.1, and ODF
1.2 producers using default settings.

The recommended fix is to arrange that AOO 3.4 only use the ODF 1.0/1.1/1.2
default settings (and hence Blowfish CFB and SHA1 start-key-generation) in what
is produced automatically.  It is further recomended that it accept other
allowed encryptions (at least AES256-cbc) that are optionally usable when
opening encrypted documents.  Note that ODF 1.2 *requires* that Blowfish CFB be supported.

Following the recommendation, AOO 3.4 would accept encryptions currently produced by anyone and will produce the one encryption that *every* encryption-supporting implementation accepts without problems.  It would also correctly accept all encryptions produced by LO, whether using AES256-cbc or something else on the optional list.  There is no user education required, and there will be no support issues except by those seeking a way to use AES256-cbc on purpose.  That can be handled in a subsequent AOO release when the smoke clears on this problem.

If the release goes ahead using AES256-cbc, there will be a support nightmare
involving those people unable to unpgrade all of the installations where it is
intended that the encryption be decryptable.  In addition, where the decryption
fails, it will appear that AOO 3.4 has produced a corrupted document.  That
will not turn out well.

There is no reason to release something that causes known interoperability
breakdowns into the wild as the heralded first Apache OpenOffice release.
Comment 8 orcmid 2012-03-19 16:44:12 UTC
@Rob

I think there is some unspoken assumption that using AES256-cbc is somehow more secure than using Blowfish CFB.  There is no basis for that.  Attackers use the weakest points they can find.

In the case of ODF encryption, the weakest point is the use of password-based encryption.  It is no less attackable, regardless of the block cipher used.  The fact that ODF encryption provides digests that can be used to check whether a decryption is correct makes that attack even easier, along with the fact that most packages contain files for which the plaintext is readily known.

The next weak point is the fact that a single start-key is derived from the password and used for the block-cipher key derivation of all of the individual parts.  That makes that common start-key value also a point of attack, including by using start-key candidates purloined from elsewhere.  The move from SHA1 to SHA256 for the start-key-derivation raises the bar, but it is still a point of attack.  The provisions of ODF encryption that assist attack on the password also assist here.

I'm not arguing that an attack is known.  Only that the choice between AES256-cbc and Blowfish CFB is irrelevant with regard to the attack surface of document for which ODF encryption has been applied.  It is especially irrelevant with regard to the pain that an automatic change of the encryption will cause in terms of the down-level and cross-product unacceptability of the result.

There is no security trade-off here.  It is entirely an interoperability issue.
Comment 9 Rob Weir 2012-03-19 17:22:39 UTC
Dennis, IIRC, some of your attacks require access to something like 1 trillion different test documents encrypted with the same password.

So, I'm not persuaded that the attack points you mention are weaker than Blowfish itself. If they are not,then the move to AES still increases security.
Comment 10 Rob Weir 2012-03-19 18:52:24 UTC
Thinking on this a little more.

At some point we need to change to AES and at that point we will break compat with earlier editors.  We cannot avoid that.  We can only delay that.

But delay does have some value.  We can seed the install base with the ability to read AES files,and do that for a release or two before we enable AES as the default for writing.  So then in the future, when we make AES the default for writing, the older versions (at least 3.4+) have the ability to read them as well.

I have no idea whether changing the default is easy or hard, or whether any one volunteers to do this.  But it is one possible approach.

The user could then change the default via the configuration option.
Comment 11 orcmid 2012-03-19 23:34:04 UTC
@Rob:

I agree with seeding the progression of releases with versions that accept AES256-cbc (and the other 4, for that matter), while continuing to produce the common ODF 1.0/1.1/1.2 Blowfish CFB encryption.

I suspect it is easy to rever to Blowfish CFB, since it is there for when default Save mode is ODF 1.0/1.1.  So someplace during the Save with Password process, a test on Save version has to be made.  It is probably relatively easy to find that and jam the result.  I'll nose around but I am certain there are others who know better where the essential code is.

It may be more difficicut to come up with configuration options that would now cause AES256 (and any others) to be used instead, since the current ones are for reverting to ODF 1.0/1.1 default encryption and use of SHA1.  If that's difficulty, it can be safely deferred to a future release.
Comment 12 orcmid 2012-03-20 01:39:56 UTC
Created attachment 77345 [details]
Change saveopt.cxx so that both UseSHA1InODF2 and UseBlowfishInODF2 are true by default

This issue is resolved by making the default configuration settings for UseSHA1InODF2 and UseBlowfishInODF2 to true.  This will provide full interoperability by defautl.

Users who prefer to use AES256-cbc encryption and are not concerned about down-level interoperability can explicitly set the flags to false in their configuration.

Note 1: Documents that are encrypted by AES256 will still be accepted, they just won't be produced by default.

Note 2: There may need to be additional changes to ensure that the produced package manifest reverts to default values in this case, rather than providing explicit attributes that are not recognized by down-level implementations.  That can be checked once this change is in place.
Comment 13 jsc 2012-03-20 08:39:31 UTC
I think it is still no show stopper issue.

I think the better default is more secure and we can provide an extension that switch the config entry if necessary.
Comment 14 Oliver-Rainer Wittmann 2012-03-20 09:00:34 UTC
FYI - this is the reference to the issue by which the default encryption has been changed:
https://issues.apache.org/ooo/show_bug.cgi?id=117562
Comment 15 orcmid 2012-03-20 14:20:28 UTC
@Oliver.  Issue r117562 is based on an incorrect premise.  ODF 1.2 does not change the default encryption in any way.  I quoted the ODF 1.2 specification in an earlier comment.  Here it is again, with more emphasis (ODF 1.2 Part 3 section 4.8.1):

"Package producers that support encryption SHALL support the value Blowfish
CFB. Package consumers that support encryption SHALL support the values
Blowfish CFB and urn:oasis:names:tc:opendocument:xmlns:manifest:1.0#blowfish."

There is conformance language related to the use of the manifest:checksum, which is not about security but being able to determine whether a decryption is correct.  That language is in 4.8.3,

"Package producers that support encryption SHOULD use the urn:oasis:names:tc:opendocument:xmlns:manifest:1.0#sha256-1k algorithm, Package consumers that support encryption SHALL support the values SHA1/1K, urn:oasis:names:tc:opendocument:xmlns:manifest:1.0#sha1-1k and urn:oasis:names:tc:opendocument:xmlns:manifest:1.0#sha256-1k."

For the legacy case, the values "SHA1" and "SHA1/1K" are the only ones recognized in use and some implementations treat "SHA1" the same as "SHA1/1K".

When blowfish is used, the SHA1/1K should always be used for interoperability reasons.
Comment 16 orcmid 2012-03-23 01:31:32 UTC
(In reply to comment #9)
> Dennis, IIRC, some of your attacks require access to something like 1 trillion
> different test documents encrypted with the same password.
> So, I'm not persuaded that the attack points you mention are weaker than
> Blowfish itself. If they are not,then the move to AES still increases security.

Rob, I didn't want to reply to this, because it is a kind of red herring.  However, others may assume this is a serious fact.

I have never considered anything that requires finding multiple documents encrypted with the same password, nor creation of multiple test documents encrypted with the same password.  

I have only considered attacks on a single document signed by the password of someone who wants that document to be a secret known only to those with whom the password has been shared by some means.  (The common case is when a document is encrypted to keep it private and only one person knows the password.)

There is an opportunistic attack available if any of the parties have used that same password in other ways where the password or its SHA1 hash or its SHA256 hash has become known.   For example, in the case of ODF documents, if the same password has been used to set protections on unencrypted documents that is used to encrypt some documents, the protection keys formed with that reused password become available to attack the encrypted document.  In this case, one simply wants to collect documents by the same parties that are likely or known to have protections set.  The fact that protection key digests and encryption start-key digests are not salted, makes this trivial, although the opportunity can't be counted on.

Nevertheless, you may recall that I have proposed updates to the ODF 1.3 specification (and ODF 1.2 extended documents) that remedy this specific case and the exposure of passwords via their known unsalted digest values.  These documents are available to the public via http://tools.oasis-open.org/issues/browse/OFFICE-3709 and http://tools.oasis-open.org/issues/browse/OFFICE-3703

More practically, there are features of ODF documents that facilitate attack on the password directly.  Note that an attack on the password is independent of whether Blowfish or AES 256 are used for the actual encryption.  (In fact, it may be easier in the case of AES256-cbc, since Blowfish with 8-bit CFB may be slower to encrypt and decrypt with.)  The key used by the encryption itself is derived from the password and is intended to be unique for each encryption ever done with that password.  However, if the password is known, the encryption key, whether for Blowfish or AES, is determined exactly.

The first curious feature is the fact that encrypted ODF documents include a plain-sight digest value of the first 1k bytes of each unencrypted (but compressed) file inside the ODF Zip package.  This is used to determine whether or not the decryption is working correctly for that file (i.e., the password can be presumed to be successful).  This is just as useful to an attacker as it is to ODF consumers that have been given a password to try, of course.

The second feature is the fact that there are gratuitously-included known-plaintext files in every ODF package produced by the well-known OpenOffice lineage implementations. Some of these are relatively short and their sizes and compressed values are known in advance.  That makes these files easy to spot in an encrypted ODF package.  That makes them interesting as aids to discovery of the password (or its digest) as well.  (It may even be the case, because of the extensive use of boilerplate on the front of some ODF XML parts, that the first 1k bytes of their compressions become known plaintext too.)

Finally, these are persistent documents, not streams occuring in one-time data-exchange with limited time sensitivity.  That means all of this attacking can be done off-line, even crowd-sourced.  In particular, a precomputed dictionary of SHA1/SHA256 digests of likely passwords is promising.  Because the known plaintext content is benign (it is insignificant boilerplate in most cases), a criminal-enterprise cracking service need not be shown the ODF Package for which the password needs to be discovered.  Only the known plaintext part and the encryption of its compressed form needs to be shared, along with the non-secret encryption parameters.  The discovered password (or its digest) can then be used to decrypt the entire document.

None of this requires cracking of Blowfish or AES, but some wicked fast Blowfish and AES implementations are important for the attacker to be able to use as tools.

Now, just as there are ways to deflect attacks by salting digests and not using the same scheme for start-keys as anywhere else, it is also possible to eliminate the availability of gratuitous known plaintexts.  The first approach is not to produce them if they have no actual purpose for the document at hand.  And if that can't be avoided, there are ways of arranging that known-plaintext XML documents not have known compressed files, making the encryption of that compressed file not of an already known compressed plaintext.  This is accomplished by introducing chaff into the the XML documents before they are compressed and subseqently encrypted.  The chaff can be random and designed in a way where it does not interfere with the processing of the decrypted and decompressed file.  This means that even if it is known what the unchaffed-plaintext is likely to be, that is no help in figuring out how to crack the encrypted version.  Now attack on the password is not so good (although the plain-sight digest values are still useful).  [I am indebted to Caolin McNamara for suggesting that random but benign insertions be termed "chaff."]  Note that this technique doesn't work for all kinds of content, and if there is a recuring image, for example, found in both unencrypted and encrypted documents from the same source, the known plaintext condition is still satisfied.

 - [whew]
Comment 17 orcmid 2012-03-23 05:58:59 UTC
(In reply to comment #6)
> I think this is a security versus interop trade-off.
> It is interesting to note that the author of the Blowfish algorithm actually
> recommends not to use it anymore:
> http://www.computerworld.com.au/article/46254/bruce_almighty_schneier_preaches_security_linux_faithful/?pp=3
> He wrote TwoFish to replace Blowfish. Twofish was submitted in the competition
> to replace the older DES standard.  AES was picked as the winner of that
> competition, not Twofish.  So from a security standpoint, our default of AES is
> the best choice.

That 2007 interview with Schneier is interesting, but I think Schneier's appraisal is more nuanced, so I hit the books.

First, why would Schneier not recommend Blowfish any longer?  Two reasons: the block size is too small and it is not approved by the U.S. Government.  64 bits is not very good and even 128 isn't desirable any longer.  This is why AES256 tends to be chosen, even though it has been thought that AES128 may actually be the superior of the three sizes standardized. 

Why does block size matter?  Well, once some sort of chaining has to be added, there is increased risk of information leakage.  The more blocks it takes, the more likely the risk of information leakage and vulnerability to what is called a collision attack.  There are also important kinds of tampering available when chaining is employed.  (It is odd, in that context, that ODF basically produces 8-bit CFB blocks from what start out to be 64-bit blocks.  Every cycle, 56 bits are discarded. I suspect this is why CFB is no longer mentioned in Schneier texts.  Now, the information leakage that is worrisome is not quite so applicable to ODF documents, because the plaintext is a binary compression.  But it is nice not to have it.  (If of those vulnerable blocks has a known plaintext, the situation can go badly more quickly.)

Back to AES.  Here are some considered words about the supposed higher security of AES:

In "Practical Cryptography," Niels Ferguson and Bruce Schneier, 2003, pp.56, 58]:

  "We have one criticism of AES: we don't quite trust the security. ...
  "In the end, everyone will use AES because it is the U.S. Government standard.  We even advise people to use it, because it *is* the standard and using the standard avoids lots of discussions and problems.  Even if AES is ever broken, nobody can fault you for having chosen the standard cipher.  But the very aggressive design coupled with the clean algebraic structure just makes us feel very uneasy."

In "Cryptography Engineering," Neils Ferguson, Bruce Schneier, and Tadayoshi Kohno, 2010, p.59:
 "3.5.6 Which Block Cipher Should I Choose?
 "The recent cryptanalytic advances against AES make this a tough choice.  Despite these cryptographic advances, AES is still what we recommend.  It is fast.  All known attacks are theoretical, not practical.  Even though AES is now broken academically, these breaks do not imply a significant security degradation of real systems in practice.  It is also the official standard, sanctioned by the U.S. government.  And everybody is using it.  They used to say 'Nobody gets fired for buying IBM.'  Similarly, nobody will fire you for choosing AES.
 "AES has other advantages.  It is relatively easy to use and implement.  All cryptographic libraries support it, and customers like it, because it is 'the standard.' "

It is not about superior security, although the larger block size is an important consideration.
Comment 18 jsc 2012-03-23 06:03:23 UTC
I am really not sure if this longer background explanation should be in this issue. If you want to discuss it in detail please use the mailing list.
Comment 19 orcmid 2012-03-23 06:13:30 UTC
(In reply to comment #10)
> Thinking on this a little more.
> At some point we need to change to AES and at that point we will break compat
> with earlier editors.  We cannot avoid that.  We can only delay that.
> But delay does have some value.  We can seed the install base with the ability
> to read AES files,and do that for a release or two before we enable AES as the
> default for writing.  So then in the future, when we make AES the default for
> writing, the older versions (at least 3.4+) have the ability to read them as
> well.
> I have no idea whether changing the default is easy or hard, or whether any one
> volunteers to do this.  But it is one possible approach.
> The user could then change the default via the configuration option.

Since you were looking for a volunteer, I dug through the SVN and found the
place where the defaults can be changed with ease.  I submitted the patch that
makes it so.

I have seen no review of the patch (which I requested just to be on the safe
side).  

Now the question seems to be whether or not the change of the default is
desirable or not.  I claim that it is for interoperability reasons.  There is
no basis for assuming that switching from Blowfish CFB to AES256 CBC does
anything to reduce the actual vulnerabilities and the cost to interoperability
is quite high if there is no staging and means to gradual switch-over.

How do we resolve this?
Comment 20 orcmid 2012-03-23 06:15:53 UTC
(In reply to comment #18)
> I am really not sure if this longer background explanation should be in this
> issue. If you want to discuss it in detail please use the mailing list.

@jsc:  I am happy to make a wider discussion of the considerations so that there can be some sort of consensus on approach.  However, it is handy to have the relevant analysis of the security details in one place.  This works a lot better than the mailing list for that.
Comment 21 jsc 2012-03-23 06:53:59 UTC
I think it should be documented somewhere else (wiki?) for later reference and the discussion should take place on the mailing list and not in the issue. Once the issue is fixed and closed the info got lost or forgotten quite fast.
Comment 22 Rob Weir 2012-03-23 21:21:51 UTC
@orcmid, a reduction in verbosity would aid comprehensibility.  Anything worth saying is worth saying briefly.
Comment 23 T. J. Frazier 2012-03-24 14:39:25 UTC
*Configuration file*
From comment 6: "[Users can] 2) Set a configuration entry to make Blowfish the default."
Please provide evidence that such a *run-time* file exists. Meanwhile, I am assuming that this is a *build-time* file.

*Better security*
Orcmid points out that, considering the overall security situation, the benefit of the new encryption method is marginal. Worth doing, yes; big rush, no. Wait for the GUI code, in AOO 4.0. Offer as an option, possibly, but opt-in, not opt-out.

*Standards*
Yes, the new encryption is a standard. However, the standard we should be most concerned with (since it's what we're all about) is the ODF standard. As quoted by Orcmid, "... SHALL support Blowfish ...". This can easily be interpreted to mean, "SHALL support *writing* Blowfish", meaning we SHALL change our configuration to do that.

*User Impact*
This is the biggie. The use of the term "default" is misleading. There is currently no, repeat *no*, known way for users to choose encryption methods for ODF 1.2 Extended saves, short of doing their own builds.

What requires, even demands, that we go with an opt-in method (rather than opt-out, which the users currently can't!), is that this proposal breaks both downward and sideways compatibility for many editors, and in a most undesirable way. We might live with it, if other editors (e.g., 3.3) would report, "I can't read this encryption method." But they don't: they report a read error, which can strike terror into even sophisticated users. There is nothing we can do about that, except education (which takes time, which we should allow), and avoidance (we should do that, too).

With an opt-in method, such as a macro or extension, the user can be adequately warned of the potential problems, on the spot.

*Recommendations*
(1) Treat this as a blocker, adopt Orcmid's patch or similar, and write Blowfish in 3.4.
(2) Schedule GUI code (and help, and translations) for 4.0.
(3) Explore opt-in possibilities for 3.4, like a macro or extension.
Comment 24 Pedro Giffuni 2012-03-25 23:05:37 UTC
I am OK about enforcing higher security by default, however I am concerned on making the NSS mandatory to interoperate. NSS is Category B software and is rather outdated (security issues likely). If we make such functionality default we should use OpenSSL instead but as discussed in the list long ago, someone will have to provide patches for that.

I think the patch solves the issue without prohibiting  higher encrytion level so I think its a reasonable option for 3.4 Release.
Comment 25 Pedro Giffuni 2012-03-25 23:16:18 UTC
Comment on attachment 77345 [details]
Change saveopt.cxx so that both UseSHA1InODF2 and UseBlowfishInODF2 are true by default

Concerning the patch: please dont include cosmetical changes on the license with code.
Hmm: what does this mean for new documents? Having both encryption types enabled for writing doesnt sound right, but I will have to look at the code around it.
Comment 26 Pedro Giffuni 2012-03-25 23:43:57 UTC
(In reply to comment #25)
> Comment on attachment 77345 [details]
> Change saveopt.cxx so that both UseSHA1InODF2 and UseBlowfishInODF2 are true by
> default
> 
> Concerning the patch: please dont include cosmetical changes on the license
> with code.
> Hmm: what does this mean for new documents? Having both encryption types
> enabled for writing doesnt sound right, but I will have to look at the code
> around it.

Nevermind ... I understand now that both UseSHA1InODF12 and UseBlowfishInODF12
must be there as they are for different things.
I am not an expert of the code, but those values are also set here:
officecfg/registry/schema/org/openoffice/Office/Common.xcs
Comment 27 orcmid 2012-03-26 00:06:45 UTC
(In reply to comment #25)

> Concerning the patch: please dont include cosmetical changes on the license
> with code.

Sorry about the cosmetic changes.  I followed the instructions for creating and submitting a patch from my SVN Working Copy.  I think those changes are because the editor I used is set to remove trailing blanks on lines.  I didn't notice until I submitted the patch.

Of course, that part of the patch doesn't have to be applied.
Comment 28 Pedro Giffuni 2012-03-26 00:32:53 UTC
(In reply to comment #27)
> (In reply to comment #25)
> 
> > Concerning the patch: please dont include cosmetical changes on the license
> > with code.
> 
> Sorry about the cosmetic changes.  I followed the instructions for creating and
> submitting a patch from my SVN Working Copy.  I think those changes are because
> the editor I used is set to remove trailing blanks on lines.  I didn't notice
> until I submitted the patch.
> 
That's OK, just thought I'd mention it for future occasions :).

I think that if just changing the default values here is
sufficient:
officecfg/registry/schema/org/openoffice/Office/Common.xcs

then that would be a better place to do it as it seems less
invasive. I don't really have the time to test it though.

In general I do agree we should avoid surprises to our users
for this release.
Comment 29 orcmid 2012-03-26 00:48:47 UTC
(In reply to comment #28)
> (In reply to comment #27)
> > (In reply to comment #25)
> > 
> > > Concerning the patch: please dont include cosmetical changes on the license
> > > with code.
> > 
> > Sorry about the cosmetic changes.  I followed the instructions for creating and
> > submitting a patch from my SVN Working Copy.  I think those changes are because
> > the editor I used is set to remove trailing blanks on lines.  I didn't notice
> > until I submitted the patch.
> > 
> That's OK, just thought I'd mention it for future occasions :).
> I think that if just changing the default values here is
> sufficient:
> officecfg/registry/schema/org/openoffice/Office/Common.xcs
> then that would be a better place to do it as it seems less
> invasive. I don't really have the time to test it though.
> In general I do agree we should avoid surprises to our users
> for this release.

(In reply to comment #26)
> (In reply to comment #25)
> > Comment on attachment 77345 [details]
> > Change saveopt.cxx so that both UseSHA1InODF2 and UseBlowfishInODF2 are true by
> > default
> Nevermind ... I understand now that both UseSHA1InODF12 and UseBlowfishInODF12
> must be there as they are for different things.
> I am not an expert of the code, but those values are also set here:
> officecfg/registry/schema/org/openoffice/Office/Common.xcs

I thought I checked every occurrence of Blowfish in trunk/main. I obviously
missed these.  I suspect grep didn't think this was a text file.

Common.xcs is supposed to be a schema, so I am not sure what the impact of
those entries are.  If they are used to supply defaults, separate from the ones
I found (which are in the initialization of a class at the time of its
construction), then these should have their <value>false</value> sub-elements
changed to <value>true</value>. 

It would be a concern, here, if this schema actually prevents an user's change
to those configuration options.  

I agree, we need to get to the bottom of this, *either* *way*.  

PS: If these settings are preset, it is apparently in files like
registrymodifications.xcu.  

I found, on Windows XP SP3 under Document & Settings\orcmid\Application Data\
OOo-dev\3\user\registrymodifications.xcu settings that indeed set the SHA1 and
BlowfishInODF2 options to false.

According to the community forum, this is where users have to edit these
manually in order to change them.  What a hoot.

I think the default should be changed in both places - in the class initializer that I found to patch, and in the Common.xcs.  If the option is mentioned in the configuration file, it changes what is initialized in the class when read by the starting application.

We'll know its working when we see the change in registrymodifications.xcu

Then we can go on to see if it actually works for users to edit these entries.  I agree with TJ, this requires a tool or script for changing the settings.  That, and add control in Tools | Options post AOO 3.4.
Comment 30 orcmid 2012-03-26 00:55:48 UTC
(In reply to comment #29)
[ ... ]> I think the default should be changed in both places - in the class initializer
> that I found to patch, and in the Common.xcs.  If the option is mentioned in
> the configuration file, it changes what is initialized in the class when read
> by the starting application.
> We'll know its working when we see the change in registrymodifications.xcu
> Then we can go on to see if it actually works for users to edit these entries. 
> I agree with TJ, this requires a tool or script for changing the settings. 
> That, and add control in Tools | Options post AOO 3.4.

Sorry for the excessive clipping.  I got caught in a BZ-comment collision and didn't clean up enough of my resubmission.

BOTTOM LINE

It would be great to put both changes into the next rc build so that we can confirm (1) that the options do indeed change and (2) editing of the registrymodifications.xcu file succeeds in changing them between true and false.

Then we have free choice in which settings are used as default in the final release candidate and it is known what is needed to give folks a tool to change them in whichever direction they choose.
Comment 31 jsc 2012-03-26 08:33:55 UTC
based on the discussion we will move back to the old defaults. The config entries are changed.

The behaviour will be switched again for 4.0 but with a GUI to switch the config items much easier for end users.
Comment 32 orcmid 2012-04-12 05:13:43 UTC
I have confirmed the default encryption for a Save with Password using the Windows x86 en_US install of Apache OpenOffice 3.4 r1309668.

I have also confirmed that introduction of the appropriate options into the registrymodifications.xcu file in the installation-created AppData sub-tree can be used to select either of SHA1 with Blowfish usage and SHA256 with AES usage.

Initially on Windows installation of r1309668, and after first-run, no explicit option is present in registrymodifications.xcu.  Any time the explicit options are removed (including by deletion of registrymodifications.xcu), the default SHA1 and Blowfish are restored.

I will update the Release Notes.