On 9/10/14 1:38 PM, Heikki Linnakangas wrote:
On 09/10/2014 02:26 PM, Marko Tiikkaja wrote:
So I wonder if I shouldn't try and instead keep the
code closer to what it is in HEAD right now; I could call
enlargeStringInfo() first, then hand out a pointer to b64_encode (or
b64_decode()) and finally increment StringInfoData.len by how much was
actually written.  That would keep the code changes a lot smaller, too.


Yeah, that sounds reasonable.

OK, I've attemped to do that in the attached. I'm pretty sure I didn't get all of the overflows right, so someone should probably take a really good look at it. (I'm not too confident the original code got them right either, but whatever).

Speaking of good looks, should I add it to the next commitfest as a new patch, or should we try and get someone to review it like this?

I'm also not sure why we need to keep a copy of the base64
encoding/decoding logic instead of exporting it in utils/adt/encode.c.

Good question...

I've left this question unanswered for now. We can fix it later, independently of this patch.


.marko
*** a/contrib/pgcrypto/pgp-armor.c
--- b/contrib/pgcrypto/pgp-armor.c
***************
*** 203,240 **** crc24(const uint8 *data, unsigned len)
        return crc & 0xffffffL;
  }
  
! int
! pgp_armor_encode(const uint8 *src, unsigned len, uint8 *dst)
  {
!       int                     n;
!       uint8      *pos = dst;
        unsigned        crc = crc24(src, len);
  
!       n = strlen(armor_header);
!       memcpy(pos, armor_header, n);
!       pos += n;
  
!       n = b64_encode(src, len, pos);
!       pos += n;
  
!       if (*(pos - 1) != '\n')
!               *pos++ = '\n';
  
!       *pos++ = '=';
!       pos[3] = _base64[crc & 0x3f];
!       crc >>= 6;
!       pos[2] = _base64[crc & 0x3f];
!       crc >>= 6;
!       pos[1] = _base64[crc & 0x3f];
!       crc >>= 6;
!       pos[0] = _base64[crc & 0x3f];
!       pos += 4;
  
!       n = strlen(armor_footer);
!       memcpy(pos, armor_footer, n);
!       pos += n;
! 
!       return pos - dst;
  }
  
  static const uint8 *
--- 203,239 ----
        return crc & 0xffffffL;
  }
  
! void
! pgp_armor_encode(const uint8 *src, int len, StringInfo dst)
  {
!       int                     res;
!       unsigned        b64len;
        unsigned        crc = crc24(src, len);
  
!       appendStringInfoString(dst, armor_header);
  
!       /* make sure we have enough room to b64_encode() */
!       b64len = b64_enc_len(len);
!       if (b64len > INT_MAX)
!               ereport(ERROR,
!                               (errcode(ERRCODE_OUT_OF_MEMORY),
!                                                errmsg("out of memory")));
!       enlargeStringInfo(dst, (int) b64len);
!       res = b64_encode(src, len, (uint8 *) dst->data + dst->len);
!       if (res > b64len)
!               elog(FATAL, "overflow - encode estimate too small");
!       dst->len += res;
  
!       if (*(dst->data + dst->len - 1) != '\n')
!               appendStringInfoChar(dst, '\n');
  
!       appendStringInfoChar(dst, '=');
!       appendStringInfoChar(dst, _base64[(crc >> 18) & 0x3f]);
!       appendStringInfoChar(dst, _base64[(crc >> 12) & 0x3f]);
!       appendStringInfoChar(dst, _base64[(crc >> 6) & 0x3f]);
!       appendStringInfoChar(dst, _base64[crc & 0x3f]);
  
!       appendStringInfoString(dst, armor_footer);
  }
  
  static const uint8 *
***************
*** 309,315 **** find_header(const uint8 *data, const uint8 *datend,
  }
  
  int
! pgp_armor_decode(const uint8 *src, unsigned len, uint8 *dst)
  {
        const uint8 *p = src;
        const uint8 *data_end = src + len;
--- 308,314 ----
  }
  
  int
! pgp_armor_decode(const uint8 *src, int len, StringInfo dst)
  {
        const uint8 *p = src;
        const uint8 *data_end = src + len;
***************
*** 319,324 **** pgp_armor_decode(const uint8 *src, unsigned len, uint8 *dst)
--- 318,324 ----
        const uint8 *base64_end = NULL;
        uint8           buf[4];
        int                     hlen;
+       int                     blen;
        int                     res = PXE_PGP_CORRUPT_ARMOR;
  
        /* armor start */
***************
*** 360,382 **** pgp_armor_decode(const uint8 *src, unsigned len, uint8 *dst)
        crc = (((long) buf[0]) << 16) + (((long) buf[1]) << 8) + (long) buf[2];
  
        /* decode data */
!       res = b64_decode(base64_start, base64_end - base64_start, dst);
! 
!       /* check crc */
!       if (res >= 0 && crc24(dst, res) != crc)
!               res = PXE_PGP_CORRUPT_ARMOR;
  out:
        return res;
  }
- 
- unsigned
- pgp_armor_enc_len(unsigned len)
- {
-       return b64_enc_len(len) + strlen(armor_header) + strlen(armor_footer) + 
16;
- }
- 
- unsigned
- pgp_armor_dec_len(unsigned len)
- {
-       return b64_dec_len(len);
- }
--- 360,377 ----
        crc = (((long) buf[0]) << 16) + (((long) buf[1]) << 8) + (long) buf[2];
  
        /* decode data */
!       blen = (int) b64_dec_len(len);
!       enlargeStringInfo(dst, blen);
!       res = b64_decode(base64_start, base64_end - base64_start, (uint8 *) 
dst->data);
!       if (res > blen)
!               elog(FATAL, "overflow - decode estimate too small");
!       if (res >= 0)
!       {
!               if (crc24((uint8 *) dst->data, res) == crc)
!                       dst->len += res;
!               else
!                       res = PXE_PGP_CORRUPT_ARMOR;
!       }
  out:
        return res;
  }
*** a/contrib/pgcrypto/pgp-pgsql.c
--- b/contrib/pgcrypto/pgp-pgsql.c
***************
*** 31,36 ****
--- 31,37 ----
  
  #include "postgres.h"
  
+ #include "lib/stringinfo.h"
  #include "mb/pg_wchar.h"
  #include "utils/builtins.h"
  
***************
*** 820,842 **** pg_armor(PG_FUNCTION_ARGS)
  {
        bytea      *data;
        text       *res;
!       int                     data_len,
!                               res_len,
!                               guess_len;
  
        data = PG_GETARG_BYTEA_P(0);
        data_len = VARSIZE(data) - VARHDRSZ;
  
!       guess_len = pgp_armor_enc_len(data_len);
!       res = palloc(VARHDRSZ + guess_len);
  
!       res_len = pgp_armor_encode((uint8 *) VARDATA(data), data_len,
!                                                          (uint8 *) 
VARDATA(res));
!       if (res_len > guess_len)
!               ereport(ERROR,
!                               
(errcode(ERRCODE_EXTERNAL_ROUTINE_INVOCATION_EXCEPTION),
!                                errmsg("Overflow - encode estimate too 
small")));
!       SET_VARSIZE(res, VARHDRSZ + res_len);
  
        PG_FREE_IF_COPY(data, 0);
        PG_RETURN_TEXT_P(res);
--- 821,840 ----
  {
        bytea      *data;
        text       *res;
!       int                     data_len;
!       StringInfoData buf;
  
        data = PG_GETARG_BYTEA_P(0);
        data_len = VARSIZE(data) - VARHDRSZ;
  
!       initStringInfo(&buf);
  
!       pgp_armor_encode((uint8 *) VARDATA(data), data_len, &buf);
! 
!       res = palloc(VARHDRSZ + buf.len);
!       SET_VARSIZE(res, VARHDRSZ + buf.len);
!       memcpy(VARDATA(res), buf.data, buf.len);
!       pfree(buf.data);
  
        PG_FREE_IF_COPY(data, 0);
        PG_RETURN_TEXT_P(res);
***************
*** 847,873 **** pg_dearmor(PG_FUNCTION_ARGS)
  {
        text       *data;
        bytea      *res;
!       int                     data_len,
!                               res_len,
!                               guess_len;
  
        data = PG_GETARG_TEXT_P(0);
        data_len = VARSIZE(data) - VARHDRSZ;
  
!       guess_len = pgp_armor_dec_len(data_len);
!       res = palloc(VARHDRSZ + guess_len);
  
!       res_len = pgp_armor_decode((uint8 *) VARDATA(data), data_len,
!                                                          (uint8 *) 
VARDATA(res));
!       if (res_len < 0)
                ereport(ERROR,
                                
(errcode(ERRCODE_EXTERNAL_ROUTINE_INVOCATION_EXCEPTION),
!                                errmsg("%s", px_strerror(res_len))));
!       if (res_len > guess_len)
!               ereport(ERROR,
!                               
(errcode(ERRCODE_EXTERNAL_ROUTINE_INVOCATION_EXCEPTION),
!                                errmsg("Overflow - decode estimate too 
small")));
!       SET_VARSIZE(res, VARHDRSZ + res_len);
  
        PG_FREE_IF_COPY(data, 0);
        PG_RETURN_TEXT_P(res);
--- 845,868 ----
  {
        text       *data;
        bytea      *res;
!       int                     data_len;
!       int                     ret;
!       StringInfoData buf;
  
        data = PG_GETARG_TEXT_P(0);
        data_len = VARSIZE(data) - VARHDRSZ;
  
!       initStringInfo(&buf);
  
!       ret = pgp_armor_decode((uint8 *) VARDATA(data), data_len, &buf);
!       if (ret < 0)
                ereport(ERROR,
                                
(errcode(ERRCODE_EXTERNAL_ROUTINE_INVOCATION_EXCEPTION),
!                                errmsg("%s", px_strerror(ret))));
!       res = palloc(VARHDRSZ + buf.len);
!       SET_VARSIZE(res, VARHDRSZ + buf.len);
!       memcpy(VARDATA(res), buf.data, buf.len);
!       pfree(buf.data);
  
        PG_FREE_IF_COPY(data, 0);
        PG_RETURN_TEXT_P(res);
*** a/contrib/pgcrypto/pgp.h
--- b/contrib/pgcrypto/pgp.h
***************
*** 29,34 ****
--- 29,36 ----
   * contrib/pgcrypto/pgp.h
   */
  
+ #include "lib/stringinfo.h"
+ 
  #include "mbuf.h"
  #include "px.h"
  
***************
*** 274,283 **** void           pgp_cfb_free(PGP_CFB *ctx);
  int                   pgp_cfb_encrypt(PGP_CFB *ctx, const uint8 *data, int 
len, uint8 *dst);
  int                   pgp_cfb_decrypt(PGP_CFB *ctx, const uint8 *data, int 
len, uint8 *dst);
  
! int                   pgp_armor_encode(const uint8 *src, unsigned len, uint8 
*dst);
! int                   pgp_armor_decode(const uint8 *src, unsigned len, uint8 
*dst);
! unsigned      pgp_armor_enc_len(unsigned len);
! unsigned      pgp_armor_dec_len(unsigned len);
  
  int                   pgp_compress_filter(PushFilter **res, PGP_Context *ctx, 
PushFilter *dst);
  int                   pgp_decompress_filter(PullFilter **res, PGP_Context 
*ctx, PullFilter *src);
--- 276,283 ----
  int                   pgp_cfb_encrypt(PGP_CFB *ctx, const uint8 *data, int 
len, uint8 *dst);
  int                   pgp_cfb_decrypt(PGP_CFB *ctx, const uint8 *data, int 
len, uint8 *dst);
  
! void          pgp_armor_encode(const uint8 *src, int len, StringInfo dst);
! int                   pgp_armor_decode(const uint8 *src, int len, StringInfo 
dst);
  
  int                   pgp_compress_filter(PushFilter **res, PGP_Context *ctx, 
PushFilter *dst);
  int                   pgp_decompress_filter(PullFilter **res, PGP_Context 
*ctx, PullFilter *src);
-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to