Azure / azure-storage-net

Microsoft Azure Storage Libraries for .NET
Apache License 2.0
446 stars 370 forks source link

Using client side encryption with RSA alters my file #339

Closed runes83 closed 6 years ago

runes83 commented 8 years ago

We have a signed PDF (PADES) that is uploaded to Azure storage using RSA key. The problem is that the file is altered when downloading, not much but enough to break the signature seal on the PDF (there is a difference in bytes also).

When i turn of the encryption it does not alter the file, I've also tried turning it off for uploading, and not for downloading and that also works (new files are not encrypted, old files could still be fetched).

   private void InitKeyFromCertificateFile()
        {
            var cert = new X509Certificate2(File.ReadAllBytes(_certificatePath), _certificatePassword,
                X509KeyStorageFlags.MachineKeySet | X509KeyStorageFlags.Exportable);
            _key = new RsaKey(cert.Thumbprint.ToUpperInvariant(), (RSACryptoServiceProvider) cert.PrivateKey);
        }

  private RsaKey RSAKey(bool forceReolad = false)
        {
            if (forceReolad)
            {
                _key.Dispose();
                _key = null;
            }

            if (_key == null || forceReolad)
            {
                if (!string.IsNullOrWhiteSpace(certificateThumprint))
                {
                    InitKeyFromCertificateStore();
                }
                else if (!string.IsNullOrWhiteSpace(_certificatePath) &&
                         !string.IsNullOrWhiteSpace(_certificatePassword))
                {
                    InitKeyFromCertificateFile();
                }
            }
            return _key;
        }
    private BlobRequestOptions BlobOptions
        {
            get
            {
var policy=new BlobEncryptionPolicy(RSAKey(true), null);
   return new BlobRequestOptions() {EncryptionPolicy = policy};
}
}

  public async Task UploadAttachment(AttachmentItem attachment)
        {
            if (ApplicationId == Guid.Empty)
                return;

            CloudBlockBlob blob = container.GetBlockBlobReference(attachment.Id.ToString());

            await
                blob.UploadFromByteArrayAsync(attachment.Bytes, 0, attachment.Bytes.Length, null, new BlobRequestOptions(), null)
                    .ConfigureAwait(false);
            blob.Metadata.Add("orgfilename", attachment.FileName);
            await blob.SetMetadataAsync().ConfigureAwait(false);
        }
``
erezvani1529 commented 8 years ago

Thanks @runes83 for reporting this issue.

Just to clarify your statement below:

I've also tried turning it off for uploading, and not for downloading and that also works (new files are not encrypted, old files could still be fetched).

Do you mean you can fetch the old encrypted files correctly?

Also it would be great if you could share the version of XSCL that you are using. We are investigating the issue and will update as soon as we have more details to share.

erezvani1529 commented 8 years ago

@runes83 Would you please share your download logic as well here?

runes83 commented 8 years ago

Sorry for later answert

  CloudBlockBlob blob = container.GetBlockBlobReference(id.ToString());
            if (blob == null)
                return null;
            if (!(await blob.ExistsAsync(BlobOptions, null)))
                return null;

            await blob.FetchAttributesAsync();
            var response = new AttachmentItem() {Id = id, Bytes = new byte[blob.Properties.Length]};
            await blob.DownloadToByteArrayAsync(response.Bytes, 0, null, BlobOptions, null).ConfigureAwait(false);

            response.FileName = blob.Metadata.ContainsKey("orgfilename") ? blob.Metadata["orgfilename"] : null;
asorrin-msft commented 8 years ago

We need to dig more into this, but my first guess is that you're hitting a known issue with the blob length. Unfortunately, the length on the blob properties is the length of the encrypted data, which is longer than the unencrypted data. So, your byte array will contain some extra zeros at the end. Will this break the signing logic?

For now, you could try using DownloadToStreamAsync() instead of DownloadToByteArrayAsync() (you'll need to create a MemoryStream, or something similar.) This has the additional advantage of being able to omit the FetchAttributesAsync() call (although you might incur an extra memcopy, to get the data out of the stream)

Also - one minor comment - during your upload logic, I would recommend adding the metadata before calling UploadFromByteArrayAsync(). This will allow you to skip the SetMetadata() call, and reduces the risk of corrupting the encryption metadata on the blob. (We use blob metadata to store information required to decrypt the blob. If you (say in the future, making another change to the code or something) accidentally overwrite the metadata, the data becomes unrecoverable. That's why we don't recommend calling SetMetadata() after an encrypted blob is uploaded, unless you're very very sure you know what you're doing.)

runes83 commented 8 years ago

Reenabling encryption would not be a problem for the unencrypted files? The client would look for metadata that contains info wether or not the blob should be encrypted/decrypted?

I'll try the download to stream method and report back.

asorrin-msft commented 8 years ago

Yes, the blob contains metadata if it's encrypted, so the client knows whether or not to decrypt - the procedure should be transparent. The exception to this is if you have the "RequireEncryption" flag set to true - if the client sees that, it will throw an exception if the blob is not encrypted.