Re: [pdf-devel] Encryption module page draft

[David Vazquez]

address@hidden writes:

> Thanks :) Some comments about the patch follows.
>
> address@hidden Initializating the module
> address@hidden Initializating the module
> +
> +The encryption module must be initialized when Libgnupdf library was
> +loaded It is *NOT* safe-thread.
>
> What kind of global data will be maintained by the module? All the
> information related with the ciphers will be stored in the per-cipher
> data structure, isnt it?
>

Indeed. The module will not any data directly, it should be a wrapper
for `gcry_check_version' libgcrypt's function, which must be called
before anyother libgcrypt's function.

> address@hidden {Data Type} {pdf_crypt_algo_t}
> +Enumeration of encryption algorithms provided.
> address@hidden deftp
>
> Note that we are using the convention of naming the enumeration data
> types as 'enum pdf_XXX_e'. Also, we use to include the complete data
> type name in the @deftp entry, as:
>
>   @deftp {Data Type} {enum pdf_crypt_algo_e}
>   ...
>   @end deftp
>
> address@hidden {Data Type} {pdf_crypt_t}
> +A cipher. It contains data which is required between several calls to
> +functions.
> address@hidden deftp
>
> The crypt module will implement several abstractions, one of which is
> the 'cipher'. I would suggest to use a more explicit data type such as
> 'pdf_crypt_cipher_t' for ciphers.
>
> Also, the functions operating in ciphers should be named after
> 'pdf_crypt_cipher_xxx'.
>

I agree.

> address@hidden pdf_status_t pdf_crypt_new (pdf_crypt_algo_t @var{algorithm}, 
> pdf_crypt_t @var{*cipher})
>
> Please use 'address@hidden' instead of '@var{*cipher}'
>
Fixed.

> +The algorithm the cipher will use in order to encrypt or decrypt
> +data. Possible values are:
> address@hidden @code
> address@hidden PDF_CRYPT_IDENTITY
> address@hidden PDF_CRYPT_AESV2
> address@hidden PDF_CRYPT_V2
>
> Please document the possible values of the enumeration data type in
> the appropriate @deftp entry.
>
> address@hidden Encryption and decryption
> address@hidden Encryption and decryption
> +
> address@hidden pdf_status_t pdf_crypt_setkey (pdf_crypt_t @var{cipher}, 
> pdf_char_t  @var{*key}, pdf_size_t  @var{length})
> +
> +Set the key which will be used to encrypt and decrypt data. This
> +function should be called each time @code{pdf_crypt_encrypt} or
> address@hidden are called.
>
> Is that restriction really needed?
>

Not really, although you must reset the key for each object. It is
original behavior of libgcrypt.

Do I remove the restriction?

> address@hidden length
> +The length of the key in bytes. Note that some algorithms could require
> +a fixed length of key.
> address@hidden table
> address@hidden Returns
> +A PDF status value:
> address@hidden @code
> address@hidden PDF_OK
> address@hidden PDF_ERROR
> address@hidden table
> address@hidden table
> address@hidden deftypefun
>
> Since you can pass invalid parameters to this function
> (pdf_crypt_cipher_setkey) would be good to have more detailed return
> status values, such as PDF_EBADDATA for an invalid key (wrong key
> length, etc).
>
> Dont hesitate to define new values for pdf_status_t as needed.
>

Ok. I wrote down it. :)


> address@hidden pdf_status_t pdf_crypt_encrypt (pdf_crypt_t @var{cipher}, 
> pdf_char_t  @var{*out}, pdf_size_t  @var{outsize}, pdf_char_t  @var{*in}, 
> pdf_size_t  @var{length}) 
> +
> +Encrypt a buffer. The ciphered text will be put in @var{out}.
> +
> address@hidden deftypefun
>
> Please include information about parameters, return value, etc in
> every @deftypefun entry.
>
> Also note that if you call the first size parameter 'in_size' then
> would be reasonable to do the same with 'out_size' ('length' name
> may be confusing).
>
> BTW, how do we know the size of the de/ciphered text? As far as I
> understand it the *_crypt_size functions provides information about
> the minimum buffer space required to hold the de/ciphered text, but
> not the exact number of bytes.
>
I fix it: it is the number of bytes in order to hold the de/ciphered
text. I said the minimum buffer space because the user could want to
reserve a greater space.

> address@hidden Usage example
> address@hidden Usage example
>
> We are including "Usage example" entries for every function documented
> in the reference manual (into the @deftypefun block).
>
> If you want to provide additional examples involving several functions
> or non-simple examples then it is ok to include a subsection like
> "Examples of use".
>

Ok, thanks. I sent the patch as summary in order to fix general
issues. I will apply your suggestions and I will send you a patch for
more detailed documentation soon.

--