comments in-line below

On 2015-02-11 15:59, David Mandelberg wrote:
> From: David Mandelberg <dmand...@bbn.com>
> 
> Use the schema introduced in the previous commit when inserting ROAs
> into the database.
> 
> WARNING: This is one of a series of commits to use the updated schema.
> It is not the last in the series, so the code is still broken.
> 
> addresses [#6] and [#7]
> ---
>  lib/rpki/cms/roa_general.c | 174 ++++++++++++++++++--------------------
>  lib/rpki/cms/roa_utils.h   |  33 ++++++--
>  lib/rpki/scmf.h            |   4 +-
>  lib/rpki/sqcon.c           |   4 +-
>  lib/rpki/sqhl.c            | 206 
> ++++++++++++++++++++++++++++++++++++++++++---
>  5 files changed, 306 insertions(+), 115 deletions(-)
> 
> diff --git a/lib/rpki/cms/roa_general.c b/lib/rpki/cms/roa_general.c
> index 9c71f92..82d9870 100644
> --- a/lib/rpki/cms/roa_general.c
> +++ b/lib/rpki/cms/roa_general.c
> @@ -442,125 +442,113 @@ uint32_t roaAS_ID(
>      return (uint32_t)iAS_ID;
>  }
>  
> -/*
> - * void roaFree(struct ROA *r) { if (NULL != r) { delete_casn(&(r->self));
> - * free((void *)r); } } 
> - */
> -static int convertAddr(
> -    int family,
> -    struct ROAIPAddress *ipaddressp,
> -    char *outbuf,
> -    int outbufLth)
> -{
> -    memset(outbuf, 0, outbufLth);
> -    uchar abuf[36];
> -    int addrLth = read_casn(&ipaddressp->address, abuf);
> -    if (family == AF_INET)
> -    {
> -        uint32_t addrVal = 0;
> -        uint32_t xx;
> -        int ii;
> -        if (outbufLth < INET_ADDRSTRLEN + 6)
> -            return ERR_SCM_INVALSZ;
> -        for (ii = 1; ii < addrLth; ii++)
> -            addrVal = (addrVal << 8) + abuf[ii];
> -        while (ii++ <= (int)sizeof(unsigned int))
> -            addrVal <<= 8;
> -        xx = htonl(addrVal);
> -        if (!inet_ntop(family, &xx, outbuf, outbufLth))
> -            return ERR_SCM_INVALIPL;
> -        // prefix length is ((addrLth - 1) * 8) - unused bits
> -        sprintf(&outbuf[strlen(outbuf)], "/%d", ((addrLth - 1) * 8) - 
> abuf[0]);
> -    }
> -    else if (family == AF_INET6)
> -    {
> -        if (outbufLth < INET6_ADDRSTRLEN + 6)
> -            return ERR_SCM_INVALSZ;
> -        uchar addrVal[16];
> -        memset(addrVal, 0, 16);
> -        memcpy(addrVal, &abuf[1], addrLth - 1);
> -        if (!inet_ntop(family, &addrVal, outbuf, outbufLth))
> -            return ERR_SCM_INVALIPL;
> -        sprintf(&outbuf[strlen(outbuf)], "/%d", ((addrLth - 1) * 8) - 
> abuf[0]);
> -    }
> -    else
> -        return ERR_SCM_INVALIPB;
> -
> -    int lth = strlen(outbuf);
> -    if (vsize_casn(&ipaddressp->maxLength))
> -    {
> -        long j;
> -        read_casn_num(&ipaddressp->maxLength, &j);
> -        sprintf(&outbuf[lth], "(%d)", (int)j);
> -        lth = strlen(outbuf);
> -    }
> -    return lth;
> -}
> -
> -int roaGetIPAddresses(
> +ssize_t roaGetPrefixes(
>      struct CMS *rp,
> -    char **str)
> +    struct roa_prefix * * prefixes)

Spacing out the pointer asterisks doesn't match the style in the rest
of the file.

>  {
>      struct ROAIPAddrBlocks *addrBlocksp =
>          &rp->content.signedData.encapContentInfo.eContent.roa.ipAddrBlocks;
>      struct ROAIPAddressFamily *famp;
> -    int replysiz = 0,
> -        lth;
> -    char *replyp = NULL;
> -    char tmpbuf[INET6_ADDRSTRLEN + 8];  // to be on safe side
> -    *str = NULL;                // in case of failure
> +
> +    // Actual length of *prefixes
> +    size_t prefixes_length = 0;
> +
> +    // Allocated length of *prefixes
> +    size_t prefixes_allocated = 16;

undocumented magic number

> +
> +    *prefixes = calloc(prefixes_allocated, sizeof(struct roa_prefix));

Why calloc() instead of malloc()?  Does the memory need to be zeroed?
(I'm guessing no because you use realloc() later to grow this, which
doesn't zero its memory.)

> +    if (*prefixes == NULL)
> +    {
> +        return ERR_SCM_NOMEM;
> +    }
> +
> +    // Buffer for a single IP prefix, stored as a BIT STRING
> +    uint8_t prefix_buf[17];

What is the significance of the 17 here?  I can guess based on
context, but undocumented magic numbers should be avoided.

> +
>      for (famp =
>           (struct ROAIPAddressFamily *)member_casn(&addrBlocksp->self, 0); 
> famp;
>           famp = (struct ROAIPAddressFamily *)next_of(&famp->self))
>      {
>          uchar famtyp[4];
>          if (read_casn(&famp->addressFamily, famtyp) < 0)
> +        {
> +            free(*prefixes);
> +            *prefixes = NULL;
>              return -1;
> -        int family;
> -        if (famtyp[1] == 1)
> -            family = AF_INET;
> -        else if (famtyp[1] == 2)
> -            family = AF_INET6;
> +        }
> +
> +        uint_fast8_t prefix_family_length;
> +        switch (famtyp[1])
> +        {
> +            case 1:
> +                prefix_family_length = 4;
> +                break;
> +
> +            case 2:
> +                prefix_family_length = 16;
> +                break;
> +
> +            default:
> +                free(*prefixes);
> +                *prefixes = NULL;
> +                return -1;
> +        }

famp->addressFamily (AFI) is a two-octet number followed by an
optional one-octet SAFI.  This switch assumes the most significant
octet is 0, and if it isn't then this will behave incorrectly.

This should instead do something like:

    uint_fast16_t afi = ((uint_fast16_t)famtyp[0] << 8) + famtyp[1];
    switch (afi)
    {
        ...
    }

> +
>          struct ROAIPAddress *ipaddressp;
>          for (ipaddressp =
>               (struct ROAIPAddress *)member_casn(&famp->addresses.self, 0);
>               ipaddressp;
>               ipaddressp = (struct ROAIPAddress *)next_of(&ipaddressp->self))
>          {
> -            lth =
> -                convertAddr(family, ipaddressp, tmpbuf, INET6_ADDRSTRLEN + 
> 8);
> -            if (lth < 0)
> +            if (prefixes_length >= prefixes_allocated)
>              {
> -                if (replyp)
> -                    free(replyp);
> -                return lth;
> -            }
> -            if (!replysiz)
> -            {                   // lth + 1 allows for null
> -                if (!(replyp = (char *)calloc(1, lth + 1)))
> +                prefixes_allocated *= 2;
> +
> +                struct roa_prefix * new_prefixes = realloc(
> +                    *prefixes,
> +                    prefixes_allocated * sizeof(struct roa_prefix));

Rather than dynamically grow the array as needed, does ASN.1 provide a
way to get the number of prefixes before processing?

Also:

  * Rather than load all of the prefixes into memory and then dump
    them to the database, wouldn't it be better to somehow iterate
    over the prefixes and write them one at a time so that there's no
    risk of running out of memory?  (disk storage would still be a
    concern)

  * Should there be a sanity check on the number of prefixes in the
    ROA?  E.g., if there are more than $x$ prefixes then assume the
    ROA is malicious and the CA is trying to DoS RPs.

  * Similarly, should there be a sanity check on the total number of
    prefixes across all ROAs for an AS?

  * Should the code try to aggregate prefixes?  E.g., if the ROA
    contains the four prefixes 10.0.{0,1,2,3}/24-24 then aggregate
    these into 10.0.0/22-24.  If the prefixes are ordered in a
    particular way, a ROA would still be able to consume $N/2$ entries
    worth of storage (where $N$ is the number of prefixes in the ROA)
    before aggregation can kick in, and I think this would require
    some complex database magic.

  * RFC question:  Why isn't there a limit to the number of prefixes
    in a ROA?  If a ROA is too long for an implementation to handle,
    who is at fault, the implementation or the CA?  How does the CA
    know when a ROA will be too big for an implementation to handle?

> +                if (new_prefixes == NULL)
> +                {
> +                    free(*prefixes);
> +                    *prefixes = NULL;
>                      return ERR_SCM_NOMEM;
> -                strcpy(replyp, tmpbuf);
> -                replysiz = lth; // size without null
> +                }
> +                *prefixes = new_prefixes;
> +            }
> +
> +            (*prefixes)[prefixes_length].prefix_family_length =
> +                prefix_family_length;
> +
> +            int prefix_buflen =
> +                read_casn(&ipaddressp->address, prefix_buf);

Are read_casn() and friends safe against malicious ASN.1 data?  For
example, what if the ROA has an absurdly long BIT STRING for
IPAddress?  Will this call stomp all over the stack?

Also: Shouldn't you check to see that prefix_buflen is less than or
equal to the family size in bytes?

> +            memset((*prefixes)[prefixes_length].prefix, 0,
> +                prefix_family_length);
> +            memcpy((*prefixes)[prefixes_length].prefix,
> +                prefix_buf + 1, prefix_buflen - 1);

What is the +1 for?  (I couldn't find any good documentation for
read_casn(), so I'll assume this code is correct and that the first
byte is special somehow.)

> +
> +            (*prefixes)[prefixes_length].prefix_length =
> +                ((prefix_buflen - 1) * 8) - prefix_buf[0];

Does prefix_buf[0] contain the number of leftover (unused) bits in the
last octet of the bit string?

> +
> +            if (vsize_casn(&ipaddressp->maxLength))

It looks like vsize_casn() can return a positive value or a negative
error code, but this function only checks to see if the return value
is non-zero (i.e. it appears to assume it can't return an error code).

> +            {
> +                long prefix_max_length;
> +                read_casn_num(&ipaddressp->maxLength,
> +                    &prefix_max_length);
> +                (*prefixes)[prefixes_length].prefix_max_length =
> +                    prefix_max_length;
>              }
>              else
>              {
> -                char *tmpp = (char *)realloc(replyp, replysiz + lth + 3);
> -                if (!tmpp)
> -                {
> -                    free(replyp);
> -                    return ERR_SCM_NOMEM;;
> -                }
> -                replyp = tmpp;
> -                char *b = &replyp[replysiz];
> -                *b++ = ',';
> -                *b++ = ' ';
> -                strcpy(b, tmpbuf);
> -                replysiz += lth + 2;    // without null
> +                // Default max length is equal to the prefix length
> +                (*prefixes)[prefixes_length].prefix_max_length =
> +                    (*prefixes)[prefixes_length].prefix_length;
>              }
> +
> +            ++prefixes_length;

prefixes_length has type size_t (unsigned), but it is returned and the
return value is ssize_t (signed).  If prefixes_length grows bigger
than SSIZE_MAX, then we'll return an implementation-defined value (or
raise an implementation-defined signal).

However, I don't think it's necessary to test if prefixes_length will
go over SSIZE_MAX -- we'll have bigger problems if prefixes_length
ever gets that big.

>          }
>      }
> -    *str = replyp;
> -    return 0;
> +
> +    return prefixes_length;
>  }
>  
>  int roaGenerateFilter(
> diff --git a/lib/rpki/cms/roa_utils.h b/lib/rpki/cms/roa_utils.h
> index 6c4ddc9..740f367 100644
> --- a/lib/rpki/cms/roa_utils.h
> +++ b/lib/rpki/cms/roa_utils.h
> @@ -165,13 +165,36 @@ int roaGenerateFilter2(
>      struct CMS *r,
>      char **str);
>  
> -/*
> - * Fills the IP addresses assigned by a ROA into a multiline string, where
> - * each line has the form address/prefix_length[/max_prefix_len] 
> +/**
> + * @brief Represent a prefix from a ROA.
> + */
> +struct roa_prefix
> +{
> +    /**
> +     * @brief Length of #prefix, either 4 for IPv4 or 16 for IPv6.
> +     */
> +    uint8_t prefix_family_length;
> +
> +    uint8_t prefix[16];
> +
> +    uint8_t prefix_length;
> +
> +    uint8_t prefix_max_length;
> +};
> +
> +/**
> + * @brief Extract the prefixes from a ROA.
> + *
> + * @param[in] r The ROA.
> + * @param[out] prefixes On success, *prefixes will be set to point to
> + *     an array of roa prefixes. This value must be free()ed by the
> + *     caller. On failure, *prefixes will be set to NULL.

Why promise the caller that *prefixes will be set to NULL on error?
This puts an unnecessary constraint on the implementation.

> + * @return On success, the number of prefixes. On failure, a negative
> + *     number.
>   */
> -int roaGetIPAddresses(
> +ssize_t roaGetPrefixes(
>      struct CMS *r,
> -    char **str);
> +    struct roa_prefix * * prefixes);

A few things:

  * Spacing out the pointer asterisks doesn't match the style in the
    rest of the file.

  * Should 'const' be added to the 'struct roa_prefix **prefixes'
    argument specification?  (Is it OK for the function to modify the
    'struct roa_prefix' entries?)

  * POSIX says ssize_t only has to cover the range [-1, {SSIZE_MAX}],
    so returning ERR_SCM_NOMEM ($-2$) seems a bit iffy (though I think
    the C spec guarantees that the minimum value goes down to at least
    negative {SSIZE_MAX}).

    I would prefer an API like this:

        /**
         * @brief Extract the prefixes from a ROA.
         *
         * @param[in] r The ROA.
         * @param[out] prefixes On success, *prefixes will be set to
         *     point to an array of roa prefixes. This value must be
         *     free()ed by the caller. On failure, the value at @a
         *     prefixes is unspecified.  This must not be NULL.
         * @param[out] prefixes_len On success, *prefixes_len will be
         *     set to the length of the array at @a prefixes.  On
         *     failure, the value at @a prefixes_len is unspecified.
         *     This must not be NULL.
         * @return Zero on success, non-0 otherwise.
         */
        int roaGetPrefixes(
            struct CMS *r,
            struct roa_prefix **prefixes,
            size_t *prefixes_len);

>  
>  /*
>   * This utility function extracts the SKI from a ROA and formats it in the
> diff --git a/lib/rpki/scmf.h b/lib/rpki/scmf.h
> index 45aed30..2a2bea5 100644
> --- a/lib/rpki/scmf.h
> +++ b/lib/rpki/scmf.h
> @@ -131,14 +131,14 @@ extern void freesrchscm(
>      scmsrcha * srch);
>  extern void *unhexify(
>      int strnglen,
> -    char *strng);
> +    char const * strng);

A few things:

  * Spacing out the pointer asterisks doesn't match the style in the
    rest of the file.

  * Do most of the files put const after the type?  Typically const is
    put before the type.

  * move these constifications to a separate commit

>  extern char *geterrorscm(
>      scmcon * conp);
>  extern char *gettablescm(
>      scmcon * conp);
>  extern char *hexify(
>      int bytelen,
> -    void *bytes,
> +    void const * bytes,
>      int useox);
>  extern int getrowsscm(
>      scmcon * conp);
> diff --git a/lib/rpki/sqcon.c b/lib/rpki/sqcon.c
> index 2a08ba8..c835c78 100644
> --- a/lib/rpki/sqcon.c
> +++ b/lib/rpki/sqcon.c
> @@ -1496,7 +1496,7 @@ int setflagsscm(
>  
>  char *hexify(
>      int bytelen,
> -    void *ptr,
> +    void const * ptr,

A few things:

  * Spacing out the pointer asterisks doesn't match the style in the
    rest of the file.

  * Do most of the files put const after the type?  Typically const is
    put before the type.

  * move these constifications to a separate commit

>      int useox)
>  {
>      unsigned char *inptr;
> @@ -1551,7 +1551,7 @@ char *hexify(
>  
>  void *unhexify(
>      int strnglen,
> -    char *strng)
> +    char const * strng)
>  {
>      unsigned char *oot;
>      unsigned int x;
> diff --git a/lib/rpki/sqhl.c b/lib/rpki/sqhl.c
> index 5dc019a..b34b8fa 100644
> --- a/lib/rpki/sqhl.c
> +++ b/lib/rpki/sqhl.c
> @@ -39,6 +39,7 @@
>  
>  static scmtab *theCertTable = NULL;
>  static scmtab *theROATable = NULL;
> +static scmtab *theROAPrefixTable = NULL;
>  static scmtab *theCRLTable = NULL;
>  static scmtab *theManifestTable = NULL;
>  static scmtab *theGBRTable = NULL;
> @@ -89,6 +90,12 @@ static void initTables(
>              LOG(LOG_ERR, "Error finding roa table");
>              exit(-1);
>          }
> +        theROAPrefixTable = findtablescm(scmp, "ROA_PREFIX");
> +        if (theROAPrefixTable == NULL)
> +        {
> +            LOG(LOG_ERR, "Error finding roa_prefix table");
> +            exit(-1);
> +        }
>          theManifestTable = findtablescm(scmp, "MANIFEST");
>          if (theManifestTable == NULL)
>          {
> @@ -2965,13 +2972,14 @@ static int add_roa_internal(
>      unsigned int dirid,
>      char *ski,
>      uint32_t asid,
> -    char *ip_addrs,
> +    size_t prefixes_length,
> +    struct roa_prefix const * prefixes,

Please add some Doxygen documentation for these new parameters.
(Don't worry about documenting the other parameters if you don't feel
like it, but if you do, add that documentation in a separate commit.)

>      char *sig,
>      unsigned int flags)
>  {
>      unsigned int roa_id = 0;
>      scmkva aone;
> -    scmkv cols[8];
> +    scmkv cols[7];

Hard-coding the length is risky -- I think it's safer to do something
like this:

    scmkv cols[] = {
        {"filename", outfile},
        {"dir_id", did},
        {"ski", ski},
        ...
    };

This would require some code rearranging.  If you do this, please put
it in a separate commit.

>      char flagn[24];
>      char asn[24];
>      char lid[24];
> @@ -2979,15 +2987,31 @@ static int add_roa_internal(
>      int idx = 0;
>      int sta;
>  
> +    // Buffer to hold a potentially large INSERT statement. This is
> +    // used to insert multiple rows per statement into
> +    // rpki_roa_prefix.
> +    size_t const multiinsert_len = 32 * 1024;
> +    char * multiinsert = malloc(multiinsert_len);
> +    if (multiinsert == NULL)
> +    {
> +        return ERR_SCM_NOMEM;
> +    }
> +
>      initTables(scmp);
>      conp->mystat.tabname = "ROA";
>      // first check for a duplicate signature
>      sta = dupsigscm(scmp, conp, theROATable, sig);
>      if (sta < 0)
> +    {
> +        free(multiinsert);
>          return (sta);
> +    }
>      sta = getmaxidscm(scmp, conp, "local_id", theROATable, &roa_id);
>      if (sta < 0)
> +    {
> +        free(multiinsert);
>          return (sta);
> +    }
>      roa_id++;
>      // fill in insertion structure
>      cols[idx].column = "filename";
> @@ -2999,8 +3023,6 @@ static int add_roa_internal(
>      cols[idx++].value = ski;
>      cols[idx].column = "sig";
>      cols[idx++].value = sig;
> -    cols[idx].column = "ip_addrs";
> -    cols[idx++].value = ip_addrs;
>      (void)snprintf(asn, sizeof(asn), "%" PRIu32, asid);
>      cols[idx].column = "asn";
>      cols[idx++].value = asn;
> @@ -3011,11 +3033,164 @@ static int add_roa_internal(
>      cols[idx].column = "local_id";
>      cols[idx++].value = lid;
>      aone.vec = &cols[0];
> -    aone.ntot = 8;
> +    aone.ntot = sizeof(cols)/sizeof(cols[0]);
>      aone.nused = idx;
>      aone.vald = 0;
>      // add the ROA
>      sta = insertscm(conp, theROATable, &aone);
> +    if (sta < 0)
> +    {
> +        free(multiinsert);
> +        return sta;
> +    }
> +
> +    // Prefix for the insert statement that inserts multiple rows
> +    // into rpki_roa_prefix.
> +    static char const multiinsert_pre[] =
> +        "insert into rpki_roa_prefix "
> +        "(roa_local_id, prefix, prefix_length, prefix_max_length) "
> +        "values ";

Please capitalize INSERT INTO and VALUES to match the style elsewhere.

> +    static size_t const multiinsert_pre_len =
> +        sizeof(multiinsert_pre) - 1;
> +
> +    // Index into multiinsert where the next character should be
> +    // written.
> +    size_t multiinsert_idx = 0;
> +
> +    // String to represent a prefix as VARBINARY in SQL.
> +    char * prefix;
> +
> +    int snprintf_ret;
> +
> +    size_t i;
> +    for (i = 0; i < prefixes_length; ++i)
> +    {
> +        if (multiinsert_idx == 0)
> +        {
> +            memcpy(multiinsert, multiinsert_pre, multiinsert_pre_len);
> +            multiinsert_idx += multiinsert_pre_len;
> +        }
> +
> +        prefix = hexify(
> +            prefixes[i].prefix_family_length, prefixes[i].prefix,
> +            HEXIFY_X);

Do you need to zero out the unused prefix bits in the last octet
before calling hexify()?

> +        if (prefix == NULL)
> +        {
> +            sta = ERR_SCM_NOMEM;
> +            goto done;
> +        }
> +
> +        snprintf_ret = snprintf(
> +            multiinsert + multiinsert_idx,
> +            multiinsert_len - multiinsert_idx,
> +            "(%u, %s, %" PRIu8 ", %" PRIu8 "),",
> +            roa_id,
> +            prefix,
> +            prefixes[i].prefix_length,
> +            prefixes[i].prefix_max_length);
> +
> +        free(prefix);
> +        prefix = NULL;
> +
> +        if (snprintf_ret < 0)
> +        {
> +            sta = ERR_SCM_INTERNAL;
> +            goto done;
> +        }
> +        else if ((size_t)snprintf_ret >=
> +            multiinsert_len - multiinsert_idx)
> +        {
> +            // The above write was truncated.
> +
> +            if (multiinsert_idx == multiinsert_pre_len)
> +            {
> +                // The write was truncated even though it's the first
> +                // prefix in this statement. That's an internal error
> +                // because multiinsert_len is too small.
> +                sta = ERR_SCM_INTERNAL;
> +                goto done;
> +            }
> +
> +            // Decrement i to ensure this prefix gets inserted
> +            // eventually.
> +            --i;
> +
> +            // Overwrite the ',' from the previous prefix and
> +            // terminate the statement.
> +            --multiinsert_idx;
> +            multiinsert[multiinsert_idx] = '\0';
> +        }
> +        else if (i >= prefixes_length - 1)
> +        {
> +            // The write was not truncated, but this is the last
> +            // prefix, so overwrite the ',' from this prefix and
> +            // terminate the statement.
> +            multiinsert_idx += snprintf_ret - 1;
> +            multiinsert[multiinsert_idx] = '\0';
> +        }
> +        else
> +        {
> +            // The above write was not truncated and this is not the
> +            // last prefix, so attempt to add more prefixes before
> +            // executing the statement.
> +            multiinsert_idx += snprintf_ret;
> +            continue;
> +        }
> +
> +        // Perform the insert.

Doesn't the SQL statement need to be terminated with a semicolon
first?

-Richard

> +        sta = statementscm_no_data(conp, multiinsert);
> +        if (sta < 0)
> +        {
> +            LOG(LOG_ERR,
> +                "Error inserting ROA prefixes. SQL query: %s",
> +                multiinsert);
> +            goto done;
> +        }
> +
> +        // Start the statement at the beginning again with the next
> +        // prefix.
> +        multiinsert_idx = 0;
> +    }
> +
> +done:
> +
> +    if (sta < 0)
> +    {
> +        // There was an error, so delete the ROA we just inserted.
> +        int delete_status;
> +
> +        delete_status = deletescm(conp, theROATable, &aone);
> +        if (delete_status < 0)
> +        {
> +            LOG(LOG_ERR,
> +                "Error deleting row from rpki_roa: %s (%d)",
> +                err2string(delete_status), delete_status);
> +        }
> +
> +        // Then delete the prefixes, if they weren't already deleted
> +        // by the foreign key constraints.
> +        scmkva roa_prefixes_aone;
> +        scmkv roa_prefixes_cols[1];
> +        roa_prefixes_cols[0].column = "roa_local_id";
> +        roa_prefixes_cols[0].value = lid;
> +        roa_prefixes_aone.vec = &cols[0];
> +        roa_prefixes_aone.ntot =
> +            sizeof(roa_prefixes_cols)/sizeof(roa_prefixes_cols[0]);
> +        roa_prefixes_aone.nused = roa_prefixes_aone.ntot;
> +        roa_prefixes_aone.vald = 0;
> +        delete_status =
> +            deletescm(conp, theROAPrefixTable, &roa_prefixes_aone);
> +        if (delete_status < 0)
> +        {
> +            LOG(LOG_ERR,
> +                "Error deleting from rpki_roa_prefix with "
> +                "roa_local_id %s: %s (%d)",
> +                lid, err2string(delete_status), delete_status);
> +        }
> +    }
> +
> +    free(multiinsert);
> +
>      return (sta);
>  }
>  
> @@ -3037,8 +3212,9 @@ int add_roa(
>      struct CMS roa;             // note: roaFromFile constructs this
>      char ski[60],
>         *sig = NULL,
> -        certfilename[PATH_MAX],
> -        *ip_addrs = NULL;
> +        certfilename[PATH_MAX];
> +    size_t prefixes_length = 0;
> +    struct roa_prefix * prefixes = NULL;
>      unsigned char *bsig = NULL;
>      int sta,
>          chainOK,
> @@ -3086,24 +3262,28 @@ int add_roa(
>          if ((sta = verify_roa(conp, &roa, ski, &chainOK)) != 0)
>              break;
>  
> -        // ip_addrs
> -        sta = roaGetIPAddresses(&roa, &ip_addrs);
> -        if (sta != 0)
> +        // prefixes
> +        ssize_t prefixes_ret = roaGetPrefixes(&roa, &prefixes);
> +        if (prefixes_ret < 0)
> +        {
>              break;
> +        }
> +        prefixes_length = prefixes_ret;
> +
>          sta = addStateToFlags(&flags, chainOK, outfile, outfull, scmp, conp);
>          if (sta != 0)
>              break;
>  
>          // add to database
> -        sta = add_roa_internal(scmp, conp, outfile, id, ski, asid, ip_addrs,
> -                               sig, flags);
> +        sta = add_roa_internal(scmp, conp, outfile, id, ski, asid,
> +            prefixes_length, prefixes, sig, flags);
>          if (sta < 0)
>              break;
>  
>      } while (0);
>  
>      // clean up
> -    free(ip_addrs);
> +    free(prefixes);
>      if (sta != 0 && cert_added)
>          (void)delete_object(scmp, conp, certfilename, outdir, outfull,
>                              (unsigned int)0);
> 


------------------------------------------------------------------------------
_______________________________________________
rpstir-devel mailing list
rpstir-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/rpstir-devel

Reply via email to