jasonhinkle / php-gpg

GPG / PGP port written in pure PHP with no binary dependencies
116 stars 28 forks source link

File is not completely encrypt #1

Closed chinkung closed 9 years ago

chinkung commented 11 years ago

Hello,

After I use your library to encrypt, the encryption process is ok but after tried to decrypt with GnuPG 1.4.10 I found this error

gpg: WARNING: message was not integrity protected gpg: [don't know]: invalid packet (ctb=74) gpg: no valid OpenPGP data found.

and the file is partial decrypted

The key I use is 4096 bits and text file size is around 210 KB and text file contains UTF-8 (Thai) characters

jasonhinkle commented 11 years ago

The integrity protection is a warning that can be ignored because the PHP library isn't supporting MDC. It shouldn't affect the ability to decrypt though.

The second issue about the invalid packet, though, I believe is a separate issue. There seem to be a few issues that can cause it including a corrupted keychain, an outdated version of GPG or transferring either key or encrypted files in ascii mode instead of binary. I haven't run into it though. If you find any more info that would be great if you would be able to share it.

chinkung commented 10 years ago

It seems like a huge content of file is the factor.

Please see attachment, when I reduce line to 200 lines, it seems decryption ok but if you encrypt whole file with 400 lines++, you can't decrypt it with invalid package data error

Test file: https://gist.github.com/chinkung/7363624cd58e9c660907

jasonhinkle commented 10 years ago

hmm, thanks for this info. i'm trying to work out a few problems with the library it seems php 5.5 has broken the functionality. If anybody thinks they might be able to help it would be appreciated. The GPG functionality is provided by my library here... https://github.com/jasonhinkle/php-gpg

chinkung commented 10 years ago

It seems broken since PHP 5.3.x as I tested it on 5.3.10, below is my PHP version

$ php -v PHP 5.3.10-1ubuntu3.8 with Suhosin-Patch (cli) (built: Sep 4 2013 20:05:42) Copyright (c) 1997-2012 The PHP Group Zend Engine v2.3.0, Copyright (c) 1998-2012 Zend Technologies

jasonhinkle commented 10 years ago

I just committed a bunch of various fixes. I ran your sample data with 400+ lines as a unit test. It encrypts the message and I was then able to decrypt successfully with gpg 1.5.3.

maybe you can give it a try with the updated code to see if that worked..?

DanielRuf commented 9 years ago

It still happens.

I created a text file (len = 65506) and it triggers the same error.

Found the problem.

If $len is a multiple of 256, we get a wrong header. I have to read the specification how we have to change this so it works.

https://github.com/jasonhinkle/php-gpg/blob/39e7669aa4cfb49cf7aefc38f97ecbedd190ef30/libs/GPG.php#L83

When I use this (just for testing) instead of https://github.com/jasonhinkle/php-gpg/blob/39e7669aa4cfb49cf7aefc38f97ecbedd190ef30/libs/GPG.php#L152 it works

if($len>0x10000)$len=0x10000; // or $len-1;
return $this->gpg_header(0xa4, $len) . $enc;

I think we have to use something like this:

The following code does not solve the problem.

$exp = floor(log($len,0x100));
if($exp>0)$tag+=1; // or +=$exp?
$h = chr($tag);
if($exp>0)$h.=chr($exp); // just once or multiple times? right value?

$h.=chr($len%0x100);
DanielRuf commented 9 years ago

Found this code from openpgp.js

 /**
   * Writes a packet header version 4 with the given tag_type and length to a
   * string
   * 
   * @param {Integer} tag_type Tag type
   * @param {Integer} length Length of the payload
   * @return {String} String of the header
   */
  writeHeader: function(tag_type, length) {
    /* we're only generating v4 packet headers here */
    var result = "";
    result += String.fromCharCode(0xC0 | tag_type);
    result += this.writeSimpleLength(length);
    return result;
  },

  /**
   * Writes a packet header Version 3 with the given tag_type and length to a
   * string
   * 
   * @param {Integer} tag_type Tag type
   * @param {Integer} length Length of the payload
   * @return {String} String of the header
   */
  writeOldHeader: function(tag_type, length) {
    var result = "";
    if (length < 256) {
      result += String.fromCharCode(0x80 | (tag_type << 2));
      result += String.fromCharCode(length);
    } else if (length < 65536) {
      result += String.fromCharCode(0x80 | (tag_type << 2) | 1);
      result += util.writeNumber(length, 2);
    } else {
      result += String.fromCharCode(0x80 | (tag_type << 2) | 2);
      result += util.writeNumber(length, 4);
    }
    return result;
  },

  /**
   * Encodes a given integer of length to the openpgp length specifier to a
   * string
   * 
   * @param {Integer} length The length to encode
   * @return {String} String with openpgp length representation
   */
  writeSimpleLength: function(length) {
    var result = "";
    if (length < 192) {
      result += String.fromCharCode(length);
    } else if (length > 191 && length < 8384) {
      /*
       * let a = (total data packet length) - 192 let bc = two octet
       * representation of a let d = b + 192
       */
      result += String.fromCharCode(((length - 192) >> 8) + 192);
      result += String.fromCharCode((length - 192) & 0xFF);
    } else {
      result += String.fromCharCode(255);
      result += util.writeNumber(length, 4);
    }
    return result;
  },
writeNumber: function (n, bytes) {
    var b = '';
    for (var i = 0; i < bytes; i++) {
      b += String.fromCharCode((n >> (8 * (bytes - i - 1))) & 0xFF);
    }

    return b;
  },
jasonhinkle commented 9 years ago

I'm not really able to give this a thorough look that it deserves. If it helps, the phpunit tests should run and that might be a great place to put in a test case for this?

DanielRuf commented 9 years ago

Right, I will prepare a working commit when I find the time this week and let you know.

DanielRuf commented 9 years ago

I got some working code faster than I thought =) Please test and verify if it works.