Skip to content
Merged
Show file tree
Hide file tree
Changes from 22 commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
1d10ea2
Remove requirement for signature
hrasyid Jul 12, 2013
ad42193
Modify test base to follow my folder structure
hrasyid Jul 12, 2013
af8e4f9
Add test case for encrypted Rar
hrasyid Jul 12, 2013
22d181e
recognize encrypted flag in archive header
hrasyid Jul 13, 2013
c0b630c
Decrypt rar - first attempt, test still fails
hrasyid Jul 13, 2013
bb2dcce
Remove comments from reference code
hrasyid Jul 13, 2013
e78ead7
Temporarily do not treat warning as error
hrasyid Jul 13, 2013
e1511bf
Decrypt stream (in addition to header)
hrasyid Jul 14, 2013
13de04d
Fix bug when no rijndael is used
hrasyid Jul 14, 2013
78cecda
Update to reflect folder structure
hrasyid Jul 14, 2013
34190f6
Code tidyness
hrasyid Jul 14, 2013
ab19ded
Fix bug when ignoring offset
hrasyid Jul 14, 2013
d15c878
Use more meaningful error message
hrasyid Jul 14, 2013
37c4665
Make password a parameter
hrasyid Jul 14, 2013
1e7d6dc
Test both Rar with header+ file encrypted and only file encrypted
hrasyid Jul 14, 2013
6c2b447
Fix filename
hrasyid Jul 14, 2013
203a7be
Merge pull request #1 from hrasyid/dev_rardecryption
hrasyid Jul 14, 2013
a73c831
Refactor initialization codes into one class
hrasyid Jul 15, 2013
3370fad
Refactor all cryptography codes to RarRijndael
hrasyid Jul 15, 2013
1ab89ba
Cleanup
hrasyid Jul 15, 2013
c7f6b50
Use faster implementation of SHA1
hrasyid Jul 15, 2013
f9194b6
Merge branch 'master' of https://github.com/hrasyid/sharpcompress
hrasyid Jul 15, 2013
a29f7d4
Revert "Remove requirement for signature"
hrasyid Jul 16, 2013
cc902bc
Style issue and access modifiers
hrasyid Jul 16, 2013
430263b
More style adjustment
hrasyid Jul 16, 2013
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -6,4 +6,5 @@ bin/
*.user
TestArchives/Scratch/
TestArchives/Scratch2/
TestResults/
*.nupkg
32 changes: 32 additions & 0 deletions SharpCompress.Test/Rar/RarReaderTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,38 @@ public void Rar_Reader()
Read("Rar.rar", CompressionType.Rar);
}

[TestMethod]
public void Rar_EncryptedFileAndHeader_Reader()
{
ReadRar("Rar.encrypted_filesAndHeader.rar", "test");

}

[TestMethod]
public void Rar_EncryptedFileOnly_Reader()
{
ReadRar("Rar.encrypted_filesOnly.rar", "test");

}

private void ReadRar(string testArchive, string password)
{
ResetScratch();
using (Stream stream = File.OpenRead(Path.Combine(TEST_ARCHIVES_PATH, testArchive)))
using (var reader = RarReader.Open(stream, password))
{
while (reader.MoveToNextEntry())
{
if (!reader.Entry.IsDirectory)
{
Assert.AreEqual(reader.Entry.CompressionType, CompressionType.Rar);
reader.WriteEntryToDirectory(SCRATCH_FILES_PATH, ExtractOptions.ExtractFullPath | ExtractOptions.Overwrite);
}
}
}
VerifyFiles();
}

[TestMethod]
public void Rar_Entry_Stream()
{
Expand Down
65 changes: 65 additions & 0 deletions SharpCompress.Test/Rar/Unit/RarHeaderFactoryTest.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
using System;
using System.IO;
using System.Net.Security;
using System.Text;
using System.Collections.Generic;
using System.Linq;
using Microsoft.VisualStudio.TestTools.UnitTesting;
using SharpCompress.Common;
using SharpCompress.Common.Rar.Headers;
using SharpCompress.IO;

namespace SharpCompress.Test.Rar.Unit
{
/// <summary>
/// Summary description for RarFactoryReaderTest
/// </summary>
[TestClass]
public class RarHeaderFactoryTest : TestBase
{
private RarHeaderFactory rarHeaderFactory;

[TestInitialize]
public void Initialize()
{
ResetScratch();
rarHeaderFactory = new RarHeaderFactory(StreamingMode.Seekable, Options.KeepStreamsOpen);
}


[TestMethod]
public void ReadHeaders_RecognizeEncryptedFlag()
{

ReadEncryptedFlag("Rar.Encrypted_filesAndHeader.rar", true);



}

private void ReadEncryptedFlag(string testArchive, bool isEncrypted)
{
using (var stream = GetReaderStream(testArchive))
foreach (var header in rarHeaderFactory.ReadHeaders(stream))
{
if (header.HeaderType == HeaderType.ArchiveHeader)
{
Assert.AreEqual(isEncrypted, rarHeaderFactory.IsEncrypted);
break;
}
}
}

[TestMethod]
public void ReadHeaders_RecognizeNoEncryptedFlag()
{
ReadEncryptedFlag("Rar.rar", false);
}

private FileStream GetReaderStream(string testArchive)
{
return new FileStream(Path.Combine(TEST_ARCHIVES_PATH, testArchive),
FileMode.Open);
}
}
}
3 changes: 2 additions & 1 deletion SharpCompress.Test/SharpCompress.Test.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@
</StartupObject>
</PropertyGroup>
<PropertyGroup>
<SignAssembly>true</SignAssembly>
<SignAssembly>false</SignAssembly>

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Revert this please.

</PropertyGroup>
<PropertyGroup>
<AssemblyOriginatorKeyFile>..\SharpCompress\SharpCompress.pfx</AssemblyOriginatorKeyFile>
Expand All @@ -56,6 +56,7 @@
<Compile Include="ArchiveTests.cs" />
<Compile Include="GZip\GZipWriterTests.cs" />
<Compile Include="GZip\GZipArchiveTests.cs" />
<Compile Include="Rar\Unit\RarHeaderFactoryTest.cs" />
<Compile Include="SevenZip\SevenZipArchiveTests.cs" />
<Compile Include="Streams\StreamTests.cs" />
<Compile Include="Tar\TarWriterTests.cs" />
Expand Down
8 changes: 4 additions & 4 deletions SharpCompress.Test/TestBase.cs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ namespace SharpCompress.Test
{
public class TestBase
{
protected const string TEST_BASE_PATH = @"C:\Git\sharpcompress";
protected const string TEST_BASE_PATH = @"D:\Codes\sharpcompress";
protected static readonly string TEST_ARCHIVES_PATH = Path.Combine(TEST_BASE_PATH, "TestArchives", "Archives");
protected static readonly string ORIGINAL_FILES_PATH = Path.Combine(TEST_BASE_PATH, "TestArchives", "Original");
protected static readonly string MISC_TEST_FILES_PATH = Path.Combine(TEST_BASE_PATH, "TestArchives", "MiscTest");
Expand Down Expand Up @@ -122,14 +122,14 @@ protected void CompareFilesByPath(string file1, string file2)
using (var file2Stream = File.OpenRead(file2))
{
Assert.AreEqual(file1Stream.Length, file2Stream.Length);

int byte1 = 0;
int byte2 = 0;
while (byte1 != -1)
for (int counter = 0; byte1 != -1; counter++ )
{
byte1 = file1Stream.ReadByte();
byte2 = file2Stream.ReadByte();
Assert.AreEqual(byte1, byte2);
if (byte1 != byte2) Assert.AreEqual(byte1, byte2, string.Format("Byte {0} differ between {1} and {2}",
counter, file1, file2));
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion SharpCompress/AssemblyInfo.cs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
[assembly: AssemblyProduct("SharpCompress")]
[assembly:
InternalsVisibleTo(
"SharpCompress.Test, PublicKey=002400000480000094000000060200000024000052534131000400000100010005d6ae1b0f6875393da83c920a5b9408f5191aaf4e8b3c2c476ad2a11f5041ecae84ce9298bc4c203637e2fd3a80ad5378a9fa8da1363e98cea45c73969198a4b64510927c910001491cebbadf597b22448ad103b0a4007e339faf8fe8665dcdb70d65b27ac05b1977c0655fad06b372b820ecbdccf10a0f214fee0986dfeded"
"SharpCompress.Test"

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Revert this please.

)]
#endif
#endif
5 changes: 5 additions & 0 deletions SharpCompress/Common/Rar/Headers/ArchiveHeader.cs
Original file line number Diff line number Diff line change
Expand Up @@ -27,5 +27,10 @@ internal ArchiveFlags ArchiveHeaderFlags
internal int PosAv { get; private set; }

internal byte EncryptionVersion { get; private set; }

public bool HasPassword
{
get { return ArchiveHeaderFlags.HasFlag(ArchiveFlags.PASSWORD); }
}
}
}
5 changes: 2 additions & 3 deletions SharpCompress/Common/Rar/Headers/RarHeader.cs
Original file line number Diff line number Diff line change
Expand Up @@ -35,11 +35,10 @@ internal static RarHeader Create(MarkingBinaryReader reader)
return null;
}
}

protected virtual void ReadFromReader(MarkingBinaryReader reader)
{
HeadCRC = reader.ReadInt16();
HeaderType = (HeaderType) (int) (reader.ReadByte() & 0xff);
HeaderType = (HeaderType)(int)(reader.ReadByte() & 0xff);
Flags = reader.ReadInt16();
HeaderSize = reader.ReadInt16();
if (FlagUtility.HasFlag(Flags, LONG_BLOCK))
Expand All @@ -58,7 +57,7 @@ internal T PromoteHeader<T>(MarkingBinaryReader reader)
header.ReadFromReader(reader);
header.ReadBytes += reader.CurrentReadByteCount;

int headerSizeDiff = header.HeaderSize - (int) header.ReadBytes;
int headerSizeDiff = header.HeaderSize - (int)header.ReadBytes;

if (headerSizeDiff > 0)
{
Expand Down
26 changes: 22 additions & 4 deletions SharpCompress/Common/Rar/Headers/RarHeaderFactory.cs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
using System;
using System.Collections.Generic;
using System.IO;
using SharpCompress.Compressor.Rar;
using SharpCompress.IO;

namespace SharpCompress.Common.Rar.Headers
Expand All @@ -9,22 +10,27 @@ internal class RarHeaderFactory
{
private int MAX_SFX_SIZE = 0x80000 - 16; //archive.cpp line 136

internal RarHeaderFactory(StreamingMode mode, Options options)
internal RarHeaderFactory(StreamingMode mode, Options options, string password = null)
{
StreamingMode = mode;
Options = options;
Password = password;
}

private Options Options { get; set; }
public string Password { get; set; }

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

private setter?


internal StreamingMode StreamingMode { get; private set; }

internal bool IsEncrypted { get; set; }

internal IEnumerable<RarHeader> ReadHeaders(Stream stream)
{
if (Options.HasFlag(Options.LookForHeader))
{
stream = CheckSFX(stream);
}

RarHeader header;
while ((header = ReadNextHeader(stream)) != null)
{
Expand Down Expand Up @@ -107,9 +113,19 @@ private RewindableStream GetRewindableStream(Stream stream)
return rewindableStream;
}


private RarHeader ReadNextHeader(Stream stream)
{
MarkingBinaryReader reader = new MarkingBinaryReader(stream);
MarkingBinaryReader reader = new MarkingBinaryReader(stream, Password);

if (IsEncrypted)
{
reader.Salt = null;
reader.SkipQueue();
byte[] salt = reader.ReadBytes(8);
reader.Salt = salt;

}
RarHeader header = RarHeader.Create(reader);
if (header == null)
{
Expand All @@ -119,7 +135,9 @@ private RarHeader ReadNextHeader(Stream stream)
{
case HeaderType.ArchiveHeader:
{
return header.PromoteHeader<ArchiveHeader>(reader);
var ah = header.PromoteHeader<ArchiveHeader>(reader);
IsEncrypted = ah.HasPassword;
return ah;
}
case HeaderType.MarkHeader:
{
Expand Down Expand Up @@ -164,7 +182,7 @@ private RarHeader ReadNextHeader(Stream stream)
{
ReadOnlySubStream ms
= new ReadOnlySubStream(reader.BaseStream, fh.CompressedSize);
fh.PackedStream = ms;
fh.PackedStream = new RarCryptoWrapper(ms, Password) { Salt = fh.Salt};

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't this be only wrapped if it is encrypted?

}
break;
default:
Expand Down
116 changes: 116 additions & 0 deletions SharpCompress/Common/Rar/RarCryptoWrapper.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,116 @@
using System;
using System.Collections.Generic;
using System.IO;

namespace SharpCompress.Common.Rar
{
internal class RarCryptoWrapper : Stream
{
private readonly Stream _actualStream;
private byte[] _salt;
private RarRijndael _rijndael;
private readonly string _password;
private readonly Queue<byte> _data = new Queue<byte>();

public RarCryptoWrapper(Stream actualStream, string password)
{
_actualStream = actualStream;
_password = password;
}

internal byte[] Salt
{
get { return _salt; }
set
{
_salt = value;
if (value != null) InitializeAes();
}
}

private void InitializeAes()
{
_rijndael = RarRijndael.InitializeFrom(_password, _salt);
}

public override void Flush()
{
throw new NotImplementedException();
}

public override long Seek(long offset, SeekOrigin origin)
{
throw new NotImplementedException();
}

public override void SetLength(long value)
{
throw new NotImplementedException();
}

public override int Read(byte[] buffer, int offset, int count)
{
if (Salt == null) return _actualStream.Read(buffer, offset, count);
return ReadAndDecrypt(buffer, offset, count);
}

public int ReadAndDecrypt(byte[] buffer, int offset, int count)
{
int queueSize = _data.Count;
int sizeToRead = count - queueSize;

if (sizeToRead > 0)
{
int alignedSize = sizeToRead + ((~sizeToRead + 1) & 0xf);
for (int i = 0; i < alignedSize/16; i++)
{
//long ax = System.currentTimeMillis();
byte[] cipherText = new byte[RarRijndael.CryptoBlockSize];
_actualStream.Read(cipherText, 0, RarRijndael.CryptoBlockSize);

var readBytes = _rijndael.ProcessBlock(cipherText);
foreach(var readByte in readBytes)
_data.Enqueue(readByte);

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would using a queue like this be a source of performance problems? Also, please nest everything with braces. Even if it's just a single line under an foreach for if statement.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The queue is necessary because we can only decrypt 16 bytes at a time, but the client of this code does not necessarily read 16 block at a time. As a solution for this, all 16 bytes will go to a queue, and if for instance the client only reads 2 bytes, the other 14 will stay in the queue and will be directly read (without decrypting) the next time.

Also, performance wise I don't think there's an issue, because generarlly, queues only involve in-memory access without any difficult calculation. I checked, the performance culprit in this class is the SHA1 calculation.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

About braces, don't additional braces make the code look more verbose? If you're worried that people might add another line and forget to use braces, I think with IDEs like Visual studio it won't happen because the indentation will make it look obvious

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It does add more lines yes. However, there are two reasons I do it. First, it just makes it perfectly clear on a quick skim of the code that a line is nested. Second, and probably more important, merging goes a lot smoother with the nesting. I've shot myself in the foot with merging because the tool couldn't see the differences with the single line versus the nested approach.

Having a couple of extra lines because of braces never hurt anyone :) Of course, you can write some nasty code because of too many levels of nesting in a method, but that's a different story.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok.. updated to use braces

On 16 July 2013 22:22, Adam Hathcock notifications@github.com wrote:

In SharpCompress/Common/Rar/RarCryptoWrapper.cs:

  •    {
    
  •        int queueSize = _data.Count;
    
  •        int sizeToRead = count - queueSize;
    
  •        if (sizeToRead > 0)
    
  •        {
    
  •            int alignedSize = sizeToRead + ((~sizeToRead + 1) & 0xf);
    
  •            for (int i = 0; i < alignedSize/16; i++)
    
  •            {
    
  •                //long ax = System.currentTimeMillis();
    
  •                byte[] cipherText = new byte[RarRijndael.CryptoBlockSize];
    
  •                _actualStream.Read(cipherText, 0, RarRijndael.CryptoBlockSize);
    
  •                var readBytes = _rijndael.ProcessBlock(cipherText);
    
  •                foreach(var readByte in readBytes)
    
  •                    _data.Enqueue(readByte);
    

It does add more lines yes. However, there are two reasons I do it. First,
it just makes it perfectly clear on a quick skim of the code that a line is
nested. Second, and probably more important, merging goes a lot smoother
with the nesting. I've shot myself in the foot with merging because the
tool couldn't see the differences with the single line versus the nested
approach.

Having a couple of extra lines because of braces never hurt anyone :) Of
course, you can write some nasty code because of too many levels of nesting
in a method, but that's a different story.


Reply to this email directly or view it on GitHubhttps://github.com//pull/1/files#r5216560
.


}

for (int i = 0; i < count; i++)
buffer[offset+i] = _data.Dequeue();
}
return count;
}

public override void Write(byte[] buffer, int offset, int count)
{
throw new NotImplementedException();
}

public override bool CanRead
{
get { throw new NotImplementedException(); }
}

public override bool CanSeek
{
get { throw new NotImplementedException(); }
}

public override bool CanWrite
{
get { throw new NotImplementedException(); }
}

public override long Length
{
get { throw new NotImplementedException(); }
}

public override long Position { get; set; }

protected override void Dispose(bool disposing)
{
if(_rijndael!= null) _rijndael.Dispose();
base.Dispose(disposing);
}
}
}
Loading