Merge lp:~rene-hummen/hipl/midauth-hipd into lp:hipl

Proposed by René Hummen
Status: Merged
Approved by: René Hummen
Approved revision: 6095
Merged at revision: 6046
Proposed branch: lp:~rene-hummen/hipl/midauth-hipd
Merge into: lp:hipl
Diff against target: 1713 lines (+1157/-250)
21 files modified
.bzrignore (+1/-0)
Makefile.am (+20/-5)
firewall/midauth.h (+2/-1)
hipd/hipd.c (+2/-47)
hipd/hipd.h (+3/-0)
hipd/main.c (+84/-0)
hipd/output.c (+0/-10)
lib/core/builder.c (+8/-93)
lib/core/builder.h (+0/-10)
lib/core/protodefs.h (+0/-23)
lib/core/solve.c (+1/-57)
lib/core/solve.h (+1/-4)
modules/midauth/hipd/midauth.c (+229/-0)
modules/midauth/hipd/midauth.h (+34/-0)
modules/midauth/lib/midauth_builder.c (+181/-0)
modules/midauth/lib/midauth_builder.h (+79/-0)
modules/midauth/module_info.xml (+47/-0)
test/check_modules_midauth.c (+46/-0)
test/modules/midauth/hipd/midauth.c (+120/-0)
test/modules/midauth/lib/midauth_builder.c (+265/-0)
test/modules/midauth/test_suites.h (+34/-0)
To merge this branch: bzr merge lp:~rene-hummen/hipl/midauth-hipd
Reviewer Review Type Date Requested Status
Diego Biurrun Needs Fixing
Stefan Götz Pending
Christof Mroz Pending
Review via email: mp+70736@code.launchpad.net

This proposal supersedes a proposal from 2011-07-19.

Description of the change

I have added unit tests and applied the feedback of the previous merge proposal. I will merge the code tomorrow afternoon if I do not receive further feedback. Please indicate your approval of the code by then.

To post a comment you must log in.
Revision history for this message
Diego Biurrun (diego-biurrun) wrote : Posted in a previous version of this proposal

 review needs-fixing

review: Needs Fixing
Revision history for this message
Christof Mroz (christof-mroz) wrote : Posted in a previous version of this proposal

Aside from Diego's comment's, it looks good to me. Minor point:

> + static const size_t min_length = sizeof(*request)
> + - sizeof(request->tlv)
> + - sizeof(request->opaque);

This exact same computation is used in other functions, so it could be refactored into a global... But OTOH this is unlikely to change anyway.

More ideally, a separate branch could rewrite all HIPL structures to utilize zero-length arrays (a C99 feature AFAIK), which would lead to a simpler length calculations and more flexibility when working with preallocated packet buffers (by not fixing their length in the struct itself).
This was already briefly discussed on the list before though.

review: Approve
Revision history for this message
René Hummen (rene-hummen) wrote : Posted in a previous version of this proposal

On 23.07.2011, at 00:59, Christof Mroz wrote:
> Review: Approve
> Aside from Diego's comment's, it looks good to me. Minor point:
>
>> + static const size_t min_length = sizeof(*request)
>> + - sizeof(request->tlv)
>> + - sizeof(request->opaque);
>
> This exact same computation is used in other functions, so it could be refactored into a global... But OTOH this is unlikely to change anyway.

This code is just repeated once for request and response respectively. I'm leaving it as it is.

René

--
Dipl.-Inform. Rene Hummen, Ph.D. Student
Chair of Communication and Distributed Systems
RWTH Aachen University, Germany
tel: +49 241 80 20772
web: http://www.comsys.rwth-aachen.de/team/rene-hummen/

Revision history for this message
René Hummen (rene-hummen) wrote : Posted in a previous version of this proposal

Diego, I have addressed the issues that you mentioned. Can you please approve the merge or give further feedback.

Revision history for this message
Diego Biurrun (diego-biurrun) wrote : Posted in a previous version of this proposal

On Mon, Jul 25, 2011 at 09:06:20AM +0000, René Hummen wrote:
> Diego, I have addressed the issues that you mentioned.

Where? I have not seen any commits from you nor an updated merge
proposal...

Diego

Revision history for this message
Diego Biurrun (diego-biurrun) wrote : Posted in a previous version of this proposal
Download full text (3.5 KiB)

 review needs-fixing

On Tue, Jul 19, 2011 at 05:18:19PM +0000, René Hummen wrote:
> René Hummen has proposed merging lp:~rene-hummen/hipl/midauth-hipd into lp:hipl.
>
> --- modules/midauth/hipd/midauth.c 1970-01-01 00:00:00 +0000
> +++ modules/midauth/hipd/midauth.c 2011-07-19 17:18:04 +0000
> @@ -0,0 +1,257 @@
> +
> +#include <errno.h>
> +#include <string.h>

You should add stdint.h for the POSIX integer types you use.

> +/**
> + * Handle the CHALLENGE_REQUEST parameter.
> + *
> + * @param packet_type The packet type of the control message (RFC 5201, 5.3.)
> + * @param ha_state The host association state (RFC 5201, 4.4.1.)
> + * @param ctx Pointer to the packet context, containing all information for
> + * the packet handling (received message, source and destination
> + * address, the ports and the corresponding entry from the host
> + * association database).

nit: The one-space indentation for the description block looks weird.

more below

> +int hip_midauth_init(void)
> +{
> + int err = 0;
> +
> + HIP_IFEL(lmod_register_parameter_type(HIP_PARAM_CHALLENGE_REQUEST,
> + "HIP_PARAM_CHALLENGE_REQUEST"),
> + -1, "failed to register parameter type\n");
> + HIP_IFEL(lmod_register_parameter_type(HIP_PARAM_CHALLENGE_RESPONSE,
> + "HIP_PARAM_CHALLENGE_RESPONSE"),
> + -1, "failed to register parameter type\n");
> +
> + HIP_IFEL(hip_register_handle_function(HIP_UPDATE,
> + HIP_STATE_R2_SENT,
> + &hip_add_host_id_param_update,
> + 20750),
> + -1, "Error on registering MIDAUTH handle function.\n");
> + HIP_IFEL(hip_register_handle_function(HIP_UPDATE,
> + HIP_STATE_ESTABLISHED,
> + &hip_add_host_id_param_update,
> + 20750),
> + -1, "Error on registering MIDAUTH handle function.\n");
> +
> +out_err:
> + return err;
> +}

HIP_IFEL abuse

> --- modules/midauth/lib/midauth_builder.c 1970-01-01 00:00:00 +0000
> +++ modules/midauth/lib/midauth_builder.c 2011-07-19 17:18:04 +0000
> @@ -0,0 +1,199 @@
> +
> +#include <string.h>

stdint.h

> + static const size_t min_length = sizeof(*request)
> + - sizeof(request->tlv)
> + - sizeof(request->opaque);
> +
> +
> + static const size_t min_length = sizeof(response)
> + - sizeof(response.tlv)
> + - sizeof(response.opaque);

nit: These would IMO look slightly more readable with the '-' at the
end of the line.

> + /* note: the length cannot be calculated with calc_param_len() */
> + hip_set_param_contents_len(&response.tlv, min_length + opaque_len);
> + hip_set_param_type(&response.tlv, HIP_PARAM_CHALLENGE_RESPONSE);
> +
> + memcpy(response.J, val_J, PUZZLE_LENGTH);
> + response.K = request->K;
> + response.lifetime = req...

Read more...

review: Needs Fixing
Revision history for this message
Stefan Götz (stefan.goetz-deactivatedaccount) wrote : Posted in a previous version of this proposal
Download full text (11.2 KiB)

Hi Rene!

> 4) There are no unit-tests for the hipd-related functionality as of now.
> However, we have tested the code extensively in three demos and I will add the
> tests at a later point in time.

Sorry, but I'm skeptical :-) As per our current policy, I cannot approve this without unit tests.

> === modified file 'firewall/midauth.h'
> === modified file 'lib/core/solve.c'
> === modified file 'lib/core/solve.h'

Please don't forget to update the Copyright dates when you modify files with
Copyright headers. Scatterbrains like myself may want to try out the pre-commit
hook in https://code.launchpad.net/~stefan.goetz/hipl/commitguard that
automatically checks for that.

> === added file 'modules/midauth/Makefile.am'

This file lacks a copyright statement.

> === added file 'modules/midauth/hipd/midauth.c'
> +/**
> + * Handle the CHALLENGE_REQUEST parameter.
> + *
> + * @param packet_type The packet type of the control message (RFC 5201, 5.3.)
> + * @param ha_state The host association state (RFC 5201, 4.4.1.)
> + * @param ctx Pointer to the packet context, containing all information for
> + * the packet handling (received message, source and destination
> + * address, the ports and the corresponding entry from the host
> + * association database).
> + *
> + * @return zero if the challenge was processed correctly or no challenge was
> + * attached to the packet, negative value otherwise

So it is not an error for a CHALLENGE_REQUEST parameter to be present but an actual challenge to be missing at the same time? Sounds like a protocol violation to me, though I haven't checked the RFC.

> +static int handle_challenge_request_param(UNUSED const uint8_t packet_type,
> + UNUSED const uint32_t ha_state,
> + struct hip_packet_context *ctx)

[M] policy: please ensure full const correctness for the 'ctx' function argument

> +{
> + const struct hip_challenge_request *request = NULL;
> +
> + request = hip_get_param(ctx->input_msg, HIP_PARAM_CHALLENGE_REQUEST);
> + if (!request) {
> + return 0;
> + }
> + // each on-path middlebox may add a challenge on its own
> + do {

> + // process next challenge parameter, if available
> + request = (const struct hip_challenge_request *)
> + hip_get_next_param(ctx->input_msg, &request->tlv);
> + } while (request && hip_get_param_type(request) == HIP_PARAM_CHALLENGE_REQUEST);

[L] simplicity: the 'if' check for '!request' and its extra 'return' statement could be avoided if a 'while(){}' loop was used instead of the 'do {} while ()' loop. That would make the code more readable in my opinion.

> +/**
> + * Add a HOST_ID parameter corresponding to the local HIT of the association to
> + * an UPDATE packet.
> + *
> + * @param packet_type The packet type of the control message (RFC 5201, 5.3.)
> + * @param ha_state The host association state (RFC 5201, 4.4.1.)
> + * @param ctx Pointer to the packet context, containing all information for
> + * the packet handling (received message, source and destination
> + * address, the ...

review: Needs Fixing
Revision history for this message
Christof Mroz (christof-mroz) wrote : Posted in a previous version of this proposal

On Sat, 30 Jul 2011 17:01:37 +0200, Stefan Götz
<email address hidden> wrote:

>> +{
>> + const struct hip_challenge_request *request = NULL;
>> +
>> + request = hip_get_param(ctx->input_msg,
>> HIP_PARAM_CHALLENGE_REQUEST);
>> + if (!request) {
>> + return 0;
>> + }
>> + // each on-path middlebox may add a challenge on its own
>> + do {
>
>> + // process next challenge parameter, if available
>> + request = (const struct hip_challenge_request *)
>> + hip_get_next_param(ctx->input_msg, &request->tlv);
>> + } while (request && hip_get_param_type(request) ==
>> HIP_PARAM_CHALLENGE_REQUEST);
>
> [L] simplicity: the 'if' check for '!request' and its extra 'return'
> statement could be avoided if a 'while(){}' loop was used instead of the
> 'do {} while ()' loop. That would make the code more readable in my
> opinion.

Could you elaborate? I originally wrote it this way because of the
"asymmetry" between hip_get_param() and hip_get_next_param().

Revision history for this message
Stefan Götz (stefan.goetz-deactivatedaccount) wrote : Posted in a previous version of this proposal

> Could you elaborate? I originally wrote it this way because of the
> "asymmetry" between hip_get_param() and hip_get_next_param().

Something like:

request = hip_get_param(ctx->input_msg, HIP_PARAM_CHALLENGE_REQUEST);
while (request && hip_get_param_type(request) == HIP_PARAM_CHALLENGE_REQUEST) {
    [...]
    request = (const struct hip_challenge_request *)
              hip_get_next_param(ctx->input_msg, &request->tlv);
}

Perfectly valid, but less common would also be

for (request = hip_get_param(ctx->input_msg, HIP_PARAM_CHALLENGE_REQUEST); request && hip_get_param_type(request) == HIP_PARAM_CHALLENGE_REQUEST; request = (const struct hip_challenge_request *) hip_get_next_param(ctx->input_msg, &request->tlv)) {
    [...]
}

Stefan

Revision history for this message
Diego Biurrun (diego-biurrun) wrote : Posted in a previous version of this proposal

On Sat, Jul 30, 2011 at 03:01:37PM +0000, Stefan Götz wrote:
>
> > === added file 'modules/midauth/Makefile.am'
>
> This file lacks a copyright statement.

All subdirectory Makefile.am snippets do. It is part of the modularization
silliness. I have a patch pending that merges the snippets into the
top-level Makefile.am.

Diego

Revision history for this message
René Hummen (rene-hummen) wrote : Posted in a previous version of this proposal

Thanks for your extensive review!

On 30.07.2011, at 17:01, Stefan Götz wrote:
> Review: Needs Fixing
> Hi Rene!
>
>> 4) There are no unit-tests for the hipd-related functionality as of now.
>> However, we have tested the code extensively in three demos and I will add the
>> tests at a later point in time.
>
> Sorry, but I'm skeptical :-) As per our current policy, I cannot approve this without unit tests.

Well, you are right. I'll add them now.

>> === modified file 'firewall/midauth.h'
>> === modified file 'lib/core/solve.c'
>> === modified file 'lib/core/solve.h'
>
> Please don't forget to update the Copyright dates when you modify files with
> Copyright headers. Scatterbrains like myself may want to try out the pre-commit
> hook in https://code.launchpad.net/~stefan.goetz/hipl/commitguard that
> automatically checks for that.

I just started using your commit guard after finishing this branch. Future commit should be bullet proof.

>> +static int handle_challenge_request_param(UNUSED const uint8_t packet_type,
>> + UNUSED const uint32_t ha_state,
>> + struct hip_packet_context *ctx)
>
> [M] policy: please ensure full const correctness for the 'ctx' function argument

Handle functions currently require the non-constness. This should be fixed in trunk.

>> +int hip_midauth_puzzle_seed(const uint8_t opaque[],
>> + const uint8_t opaque_len,
>> + uint8_t puzzle_value[PUZZLE_LENGTH])
>
> [M] lack of const correctness

I don't know how to make this more const correct. The prototype uses the pointer notation with complete const-correctness instead of arrays though.

René

--
Dipl.-Inform. Rene Hummen, Ph.D. Student
Chair of Communication and Distributed Systems
RWTH Aachen University, Germany
tel: +49 241 80 20772
web: http://www.comsys.rwth-aachen.de/team/rene-hummen/

Revision history for this message
René Hummen (rene-hummen) wrote : Posted in a previous version of this proposal

On 30.07.2011, at 17:01, Stefan Götz wrote:

> Review: Needs Fixing
> Hi Rene!
>
>> 4) There are no unit-tests for the hipd-related functionality as of now.
>> However, we have tested the code extensively in three demos and I will add the
>> tests at a later point in time.
>
> Sorry, but I'm skeptical :-) As per our current policy, I cannot approve this without unit tests.

I have added unit tests for all but the function below:

static int add_host_id_param_update(UNUSED const uint8_t packet_type,
                                    UNUSED const uint32_t ha_state,
                                    struct hip_packet_context *ctx)

The problem here is that hip_init_host_ids() is needed to set up the host id db for host id lookup within add_host_id_param_update(). However, hip_init_host_ids() requires root access to the file system. I have appended the unit test that I have written already, but is not included in my latest changes due to the restrictions I just explained.

START_TEST(test_midauth_add_host_id_param_update_CORRECT)
{
    const uint8_t opaque[] = "\x01\x41\x01\x14\x05\x00\x48\x49\x0b"
                              "\x02\x42\x02\x15\x06\x08\x49\x50\x0c";
    struct hip_packet_context ctx = { 0 };

    ctx.input_msg = hip_msg_alloc();
    ctx.output_msg = hip_msg_alloc();

    hip_init_hostid_db();
    hip_init_host_ids();
    hip_get_default_hit(&ctx.input_msg->hitr);

    fail_unless(hip_build_param_challenge_request(ctx.input_msg, 0, 0, opaque,
                                                  MIDAUTH_DEFAULT_NONCE_LENGTH) ==
                0, NULL);
    fail_unless(add_host_id_param_update(0, 0, &ctx) == 0, NULL);
    fail_unless((hip_get_param(ctx.output_msg,
                                         HIP_PARAM_HOST_ID)) != NULL,
                    NULL);
}
END_TEST

Ciao,
René

--
Dipl.-Inform. Rene Hummen, Ph.D. Student
Chair of Communication and Distributed Systems
RWTH Aachen University, Germany
tel: +49 241 80 20772
web: http://www.comsys.rwth-aachen.de/team/rene-hummen/

Revision history for this message
Stefan Götz (stefan.goetz-deactivatedaccount) wrote : Posted in a previous version of this proposal
Download full text (4.3 KiB)

Hi Rene!

Thanks for all the not so trivial improvements! Almost happy now :)

> === modified file 'Makefile.am'
> --- Makefile.am 2011-07-11 12:50:33 +0000
> +++ Makefile.am 2011-08-04 17:15:48 +0000
> @@ -215,10 +220,16 @@
> test/lib/tool/checksum.c \
> test/lib/tool/pk.c
>
> +test_check_modules_midauth_SOURCES = test/check_modules_midauth.c \
> + test/modules/midauth/lib/midauth_builder.c \
> + test/modules/midauth/hipd/midauth.c \
> + $(hipd_hipd_sources)
> +
> # Include module Makefile.am's.
> # TODO: Make this inclusion dynamic
> include modules/heartbeat/Makefile.am
> include modules/heartbeat_update/Makefile.am
> +include modules/midauth/Makefile.am
> include modules/update/Makefile.am

[L] I think with Diego's recent change, the contents of the module Makefile.am should be moved into the top-level Makefile.am? Not sure.

> === added file 'modules/midauth/hipd/midauth.c'
> +/**
> + * Initialization function for the midauth module.
> + *
> + * @return zero on success, negative value otherwise
> + */
> +int hip_midauth_init(void)

[L] I don't like the code duplication in this function very much, even if it's just boiler plate code. Here are some optional suggestions for improvement.

> +{
> + int err = 0;

[L] Get rid of this and don't use HIP_IFEL at all because there is no cleanup.

> +
> + HIP_IFEL(lmod_register_parameter_type(HIP_PARAM_CHALLENGE_REQUEST,
> + "HIP_PARAM_CHALLENGE_REQUEST"),
> + -1, "failed to register parameter type\n");
> + HIP_IFEL(lmod_register_parameter_type(HIP_PARAM_CHALLENGE_RESPONSE,
> + "HIP_PARAM_CHALLENGE_RESPONSE"),
> + -1, "failed to register parameter type\n");

[L] Is the second parameter always the string version of the the first? If so, a macro with stringification would avoid that duplication.

> +
> + HIP_IFEL(hip_register_handle_function(HIP_R1,
> + HIP_STATE_I1_SENT,
> + &handle_challenge_request_param,
> + 32500),
> + -1, "Error on registering MIDAUTH handle function.\n");
> + HIP_IFEL(hip_register_handle_function(HIP_R1,
> + HIP_STATE_I2_SENT,
> + &handle_challenge_request_param,
> + 32500),
> + -1, "Error on registering MIDAUTH handle function.\n");
> + HIP_IFEL(hip_register_handle_function(HIP_R1,
> + HIP_STATE_CLOSING,
> + &handle_challenge_request_param,
> + 32500),
> + -1, "Error on registering MIDAUTH handle function.\n");
> + HIP_IFEL(hip_register_handle_function(HIP_R1,
> + HIP_STATE_CLOSED,
> + ...

Read more...

review: Needs Fixing
Revision history for this message
Stefan Götz (stefan.goetz-deactivatedaccount) wrote : Posted in a previous version of this proposal

Hi Rene!

> I have added unit tests for all but the function below:

Cool!

> The problem here is that hip_init_host_ids() is needed to set up the host id db for host id lookup within add_host_id_param_update(). However, hip_init_host_ids() requires root access to the file system. I have appended the unit test that I have written already, but is not included in my latest changes due to the restrictions I just explained.

How about adding the check 'if (!getuid()) { [UNIT TEST CODE] } else {
printf("The unit test %s requires root privileges.\n", __FUNCTION__); }' to the
unit test? Would that seem like a reasonable thing to do?

Stefan

Revision history for this message
René Hummen (rene-hummen) wrote : Posted in a previous version of this proposal
Download full text (3.3 KiB)

On 04.08.2011, at 20:41, Stefan Götz wrote:
> Review: Needs Fixing
> Hi Rene!
>
> Thanks for all the not so trivial improvements! Almost happy now :)
>
>> === modified file 'Makefile.am'
>> --- Makefile.am 2011-07-11 12:50:33 +0000
>> +++ Makefile.am 2011-08-04 17:15:48 +0000
>> @@ -215,10 +220,16 @@
>> test/lib/tool/checksum.c \
>> test/lib/tool/pk.c
>>
>> +test_check_modules_midauth_SOURCES = test/check_modules_midauth.c \
>> + test/modules/midauth/lib/midauth_builder.c \
>> + test/modules/midauth/hipd/midauth.c \
>> + $(hipd_hipd_sources)
>> +
>> # Include module Makefile.am's.
>> # TODO: Make this inclusion dynamic
>> include modules/heartbeat/Makefile.am
>> include modules/heartbeat_update/Makefile.am
>> +include modules/midauth/Makefile.am
>> include modules/update/Makefile.am
>
> [L] I think with Diego's recent change, the contents of the module Makefile.am should be moved into the top-level Makefile.am? Not sure.

Diego's changes have not yet been merged to trunk. Hence, I would prefer to modify the Makefile with his merge.

>> +
>> + HIP_IFEL(lmod_register_parameter_type(HIP_PARAM_CHALLENGE_REQUEST,
>> + "HIP_PARAM_CHALLENGE_REQUEST"),
>> + -1, "failed to register parameter type\n");
>> + HIP_IFEL(lmod_register_parameter_type(HIP_PARAM_CHALLENGE_RESPONSE,
>> + "HIP_PARAM_CHALLENGE_RESPONSE"),
>> + -1, "failed to register parameter type\n");
>
> [L] Is the second parameter always the string version of the the first? If so, a macro with stringification would avoid that duplication.

They need not be the same, but it makes sense to do so. The string is used for debugging output. Feel free to adjust and improve lmod_register_parameter_type() globally. I don't see that such a change is in the scope of this branch though.

>> + HIP_IFEL(hip_register_handle_function(HIP_R1,
>> + HIP_STATE_CLOSED,
>> + &handle_challenge_request_param,
>> + 32500),
>> + -1, "Error on registering MIDAUTH handle function.\n");
>
> [M] as I just see this: the magic numbers 32500 really must be replaced with something more meaningful!

You are right. The priority is for handle functions is somewhat random at the moment. This should be fixed for all handle functions in a separate branch.

> [L] How about:
>
> int states[4] = { HIP_STATE_I1_SENT, [...] };
> for (unsigned i = 0; i < ARRAY_SIZE(states); i++) {
> if (hip_register_handle_function(HIP_R1,
> states[i],
> &handle_challenge_request_param,
> 32500)) {
> HIP_ERROR("Error on registering MIDAUTH handle function.\n");
> return -1;
> }
> }
>
> and similar for all the registrations below?

Good idea. I changed the code accordingly.

--
Dipl.-Inform...

Read more...

Revision history for this message
René Hummen (rene-hummen) wrote :

On 08.08.2011, at 15:05, René Hummen wrote:
> René Hummen has proposed merging lp:~rene-hummen/hipl/midauth-hipd into lp:hipl.
>
> Requested reviews:
> Christof Mroz (christof-mroz)
> Diego Biurrun (diego-biurrun)
> Stefan Götz (stefan.goetz)
>
> For more details, see:
> https://code.launchpad.net/~rene-hummen/hipl/midauth-hipd/+merge/70736
>
> I have added unit tests and applied the feedback of the previous merge proposal. I will merge the code tomorrow afternoon if I do not receive further feedback. Please indicate your approval of the code by then.

> +<<<<<<< TREE
> +=======
> +test_check_modules_midauth_SOURCES = test/check_modules_midauth.c \
> + test/modules/midauth/lib/midauth_builder.c \
> + test/modules/midauth/hipd/midauth.c \
> + $(hipd_hipd_sources)
> +
> +# Include module Makefile.am's.
> +# TODO: Make this inclusion dynamic
> +include modules/heartbeat/Makefile.am
> +include modules/heartbeat_update/Makefile.am
> +include modules/midauth/Makefile.am
> +include modules/update/Makefile.am
> +
> +>>>>>>> MERGE-SOURCE

I'll resolve this conflict during the merge by only using the main makefile.

--
Dipl.-Inform. Rene Hummen, Ph.D. Student
Chair of Communication and Distributed Systems
RWTH Aachen University, Germany
tel: +49 241 80 20772
web: http://www.comsys.rwth-aachen.de/team/rene-hummen/

Revision history for this message
Diego Biurrun (diego-biurrun) wrote :

 review needs-fixing

On Mon, Aug 08, 2011 at 01:05:16PM +0000, René Hummen wrote:
> René Hummen has proposed merging lp:~rene-hummen/hipl/midauth-hipd into lp:hipl.
>
> --- Makefile.am 2011-08-05 11:00:52 +0000
> +++ Makefile.am 2011-08-08 13:01:27 +0000
> @@ -91,7 +93,7 @@
>
> -hipd_hipd_SOURCES = hipd/accessor.c \
> +hipd_hipd_sources = hipd/accessor.c \

Hmmmm...

> @@ -128,9 +130,12 @@
>
> if HIP_MIDAUTH
> -hipd_hipd_SOURCES += hipd/pisa.c
> +hipd_hipd_sources += hipd/pisa.c
> endif
>
> +hipd_hipd_SOURCES = $(hipd_hipd_sources) \
> + hipd/main.c

Umm, this looks like a giant hack. Why is the main function moved around
like this?

> @@ -220,6 +225,21 @@
>
> +<<<<<<< TREE
> +=======
> +test_check_modules_midauth_SOURCES = test/check_modules_midauth.c \
> + test/modules/midauth/lib/midauth_builder.c \
> + test/modules/midauth/hipd/midauth.c \
> + $(hipd_hipd_sources)
> +
> +# Include module Makefile.am's.
> +# TODO: Make this inclusion dynamic
> +include modules/heartbeat/Makefile.am
> +include modules/heartbeat_update/Makefile.am
> +include modules/midauth/Makefile.am
> +include modules/update/Makefile.am
> +
> +>>>>>>> MERGE-SOURCE

Umm, no. Sorry to be blunt here, but this is not acceptable. We need
to find a way to block merge requests with conflicts. Yes, it's a bug
in Bazaar to accept them in the first place, but if our tools are not
smart enough we will need to find a protocol to work around this.

Submitting merge requests with conflicts is a needless waste of reviewer
time. Now that I'm no longer a paid member of the project I'll exercise
my privilege of free time allocation and refuse to review such merge
requests outright. :)

Any branch that gets submitted for merging and not just as an RFC needs
to pass all mechanical checks that we have. That means compiling and
passing the test suite, ideally plus a few manual tests. There is no
point in using humans to check a branch before feeding it to the compiler.
Plus, reviewer time is the scarce resource that we need to preserve.

I also think we need to rethink this whole "modularization" thing that
is adding complication in return for no practical benefit before we
entrench it further via more "modules".

Diego

review: Needs Fixing
lp:~rene-hummen/hipl/midauth-hipd updated
6043. By Diego Biurrun

cosmetics: fix 'null' vs. 'NULL' typo

6044. By Diego Biurrun

builder: clean up return handling

Return values directly instead of setting a variable, jumping
to a goto label and returning the variable there.

6045. By Diego Biurrun

nlink: drop some pointless void* casts

Revision history for this message
Christof Mroz (christof-mroz) wrote :

Looks good, aside from what Diego mentioned already (I'll approve as soon as the conflict is resolved). One minor point:

+/**
+ * Convert the opaque value in the CHALLENGE_REQUEST to the seed value I of a
+ * HIP puzzle.
+ *
+ * The opaque value plays a dual role in a CHALLENGE_REQUEST:
+ * i) it is a challenge that needs to be echoed back by the responder and
+ * ii) it is used to derive the seed value for a cryptographic puzzle. The
+ * puzzle is defined in RFC5201.
+ *
+ * @param opaque the nonce (challenge) in the CHALLENGE_REQUEST
+ * @param opaque_len the length of the nonce
+ * @param puzzle_value the puzzle value generated from the nonce
+ * @return zero on success, -1 in case of an error
+ */
+int hip_midauth_puzzle_seed(const uint8_t opaque[],
+ const unsigned int opaque_len,
+ uint8_t puzzle_value[PUZZLE_LENGTH])
+{
+ unsigned char sha_digest[SHA_DIGEST_LENGTH];
+

+ if (!puzzle_value) {
+ HIP_ERROR("Parameter puzzle_value is not allocated\n");
+ return -1;
+ }

This looks like it should never happen, i.e. an assertion would be more appropriate. Same for the opaque parameter.

lp:~rene-hummen/hipl/midauth-hipd updated
6094. By René Hummen

replace if statement by HIP_ASSERT

Modify unit test accordingly.

6095. By René Hummen

move instruction from module Makefile to central one

Revision history for this message
René Hummen (rene-hummen) wrote :

On 08.08.2011, at 17:01, Christof Mroz wrote:
> Looks good, aside from what Diego mentioned already (I'll approve as soon as the conflict is resolved). One minor point:
>
> +/**
> + * Convert the opaque value in the CHALLENGE_REQUEST to the seed value I of a
> + * HIP puzzle.
> + *
> + * The opaque value plays a dual role in a CHALLENGE_REQUEST:
> + * i) it is a challenge that needs to be echoed back by the responder and
> + * ii) it is used to derive the seed value for a cryptographic puzzle. The
> + * puzzle is defined in RFC5201.
> + *
> + * @param opaque the nonce (challenge) in the CHALLENGE_REQUEST
> + * @param opaque_len the length of the nonce
> + * @param puzzle_value the puzzle value generated from the nonce
> + * @return zero on success, -1 in case of an error
> + */
> +int hip_midauth_puzzle_seed(const uint8_t opaque[],
> + const unsigned int opaque_len,
> + uint8_t puzzle_value[PUZZLE_LENGTH])
> +{
> + unsigned char sha_digest[SHA_DIGEST_LENGTH];
> +
>
> + if (!puzzle_value) {
> + HIP_ERROR("Parameter puzzle_value is not allocated\n");
> + return -1;
> + }
>
> This looks like it should never happen, i.e. an assertion would be more appropriate. Same for the opaque parameter.

You are right. I implemented it as an if statement because in contrast to asserts HIP_ASSERTs are always executed. I changed this to HIP_ASSERT nevertheless in order to make a transition to assert() less painful in the future.

Ciao,
René

--
Dipl.-Inform. Rene Hummen, Ph.D. Student
Chair of Communication and Distributed Systems
RWTH Aachen University, Germany
tel: +49 241 80 20772
web: http://www.comsys.rwth-aachen.de/team/rene-hummen/

Revision history for this message
René Hummen (rene-hummen) wrote :

I resolved the remaining issues. No further feedback has been given, so I will merge the code as announced yesterday.

Revision history for this message
René Hummen (rene-hummen) wrote :
Download full text (3.7 KiB)

On 08.08.2011, at 16:31, Diego Biurrun wrote:
> Review: Needs Fixing
> review needs-fixing
>
> On Mon, Aug 08, 2011 at 01:05:16PM +0000, René Hummen wrote:
>> René Hummen has proposed merging lp:~rene-hummen/hipl/midauth-hipd into lp:hipl.
>>
>> --- Makefile.am 2011-08-05 11:00:52 +0000
>> +++ Makefile.am 2011-08-08 13:01:27 +0000
>> @@ -91,7 +93,7 @@
>>
>> -hipd_hipd_SOURCES = hipd/accessor.c \
>> +hipd_hipd_sources = hipd/accessor.c \
>
> Hmmmm...
>
>> @@ -128,9 +130,12 @@
>>
>> if HIP_MIDAUTH
>> -hipd_hipd_SOURCES += hipd/pisa.c
>> +hipd_hipd_sources += hipd/pisa.c
>> endif
>>
>> +hipd_hipd_SOURCES = $(hipd_hipd_sources) \
>> + hipd/main.c
>
> Umm, this looks like a giant hack. Why is the main function moved around
> like this?

It was necessary to exclude the main function of the hipd for enabling unit tests (they ship their own main). One option was to move the main of the hipd to a different file and another one to exclude it by means of #ifdef. I chose to first option to be consistent with the unit tests in the firewall. hipd_hipd_sources are now used by hipd and the unit tests, so I don't see the point in keeping the list twice instead of maintaining it in a separate variable (same as for the firewall again).

>> @@ -220,6 +225,21 @@
>>
>> +<<<<<<< TREE
>> +=======
>> +test_check_modules_midauth_SOURCES = test/check_modules_midauth.c \
>> + test/modules/midauth/lib/midauth_builder.c \
>> + test/modules/midauth/hipd/midauth.c \
>> + $(hipd_hipd_sources)
>> +
>> +# Include module Makefile.am's.
>> +# TODO: Make this inclusion dynamic
>> +include modules/heartbeat/Makefile.am
>> +include modules/heartbeat_update/Makefile.am
>> +include modules/midauth/Makefile.am
>> +include modules/update/Makefile.am
>> +
>> +>>>>>>> MERGE-SOURCE
>
> Umm, no. Sorry to be blunt here, but this is not acceptable. We need
> to find a way to block merge requests with conflicts. Yes, it's a bug
> in Bazaar to accept them in the first place, but if our tools are not
> smart enough we will need to find a protocol to work around this.
>
> Submitting merge requests with conflicts is a needless waste of reviewer
> time. Now that I'm no longer a paid member of the project I'll exercise
> my privilege of free time allocation and refuse to review such merge
> requests outright. :)
>
> Any branch that gets submitted for merging and not just as an RFC needs
> to pass all mechanical checks that we have. That means compiling and
> passing the test suite, ideally plus a few manual tests. There is no
> point in using humans to check a branch before feeding it to the compiler.
> Plus, reviewer time is the scarce resource that we need to preserve.

I forgot your merge between my rebase and my merge proposal. That's why I didn't apply step (3) of the workflow below. I would have resubmitted my proposal after realizing this if the conflicts have been more serious. Of course you can refuse to review the merge request because of this.

IMO, we should have a merge proposal workflow t...

Read more...

Revision history for this message
Diego Biurrun (diego-biurrun) wrote :

On Wed, Aug 10, 2011 at 08:26:01AM +0000, René Hummen wrote:
>
> IMO, we should have a merge proposal workflow that states:
> 1.) Prepare your branch to an extend that you would except it yourself.
> 2.) Ensure that policies are followed (no compile warnings, correct
> coding style, const correctness, and unit tests).
> 3.) Locally merge to trunk in order to check for merge conflicts.
> 4.) If (1) - (3) have successfully been applied, propose your branch for merge.

The order is wrong, trunk should be merged or rebased *before* running
tests and preparing the branch. Nobody cares if the branch works, the
important thing is trunk.

> Did I forget anything important?

Yes:

5) Do not push your branch before addressing review comments.

Diego

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file '.bzrignore'
2--- .bzrignore 2011-07-21 17:27:23 +0000
3+++ .bzrignore 2011-08-09 11:19:24 +0000
4@@ -56,6 +56,7 @@
5 test/check_firewall
6 test/check_lib_core
7 test/check_lib_tool
8+test/check_modules_midauth
9 test/dh_performance
10 test/fw_port_bindings_performance
11 test/hc_performance
12
13=== modified file 'Makefile.am'
14--- Makefile.am 2011-08-05 11:00:52 +0000
15+++ Makefile.am 2011-08-09 11:19:24 +0000
16@@ -70,10 +70,12 @@
17 if HIP_UNITTESTS
18 TESTS = test/check_firewall \
19 test/check_lib_core \
20- test/check_lib_tool
21+ test/check_lib_tool \
22+ test/check_modules_midauth
23 check_PROGRAMS = test/check_firewall \
24 test/check_lib_core \
25- test/check_lib_tool
26+ test/check_lib_tool \
27+ test/check_modules_midauth
28 endif
29
30
31@@ -91,7 +93,7 @@
32 tools_hipconf_SOURCES = tools/hipconf.c
33 tools_pisacert_SOURCES = tools/pisacert.c
34
35-hipd_hipd_SOURCES = hipd/accessor.c \
36+hipd_hipd_sources = hipd/accessor.c \
37 hipd/cert.c \
38 hipd/close.c \
39 hipd/configfilereader.c \
40@@ -122,15 +124,20 @@
41 hipd/user_ipsec_sadb_api.c \
42 modules/heartbeat/hipd/heartbeat.c \
43 modules/heartbeat_update/hipd/hb_update.c \
44+ modules/midauth/lib/midauth_builder.c \
45 modules/update/hipd/update.c \
46 modules/update/hipd/update_builder.c \
47 modules/update/hipd/update_locator.c \
48 modules/update/hipd/update_param_handling.c
49
50 if HIP_MIDAUTH
51-hipd_hipd_SOURCES += hipd/pisa.c
52+hipd_hipd_sources += hipd/pisa.c
53 endif
54
55+hipd_hipd_SOURCES = $(hipd_hipd_sources) \
56+ modules/midauth/hipd/midauth.c \
57+ hipd/main.c
58+
59 firewall_hipfw_sources = firewall/cache.c \
60 firewall/dlist.c \
61 firewall/esp_prot_api.c \
62@@ -150,7 +157,8 @@
63 firewall/user_ipsec_api.c \
64 firewall/user_ipsec_esp.c \
65 firewall/user_ipsec_fw_msg.c \
66- firewall/user_ipsec_sadb.c
67+ firewall/user_ipsec_sadb.c \
68+ modules/midauth/lib/midauth_builder.c
69
70 if HIP_MIDAUTH
71 firewall_hipfw_sources += firewall/midauth.c \
72@@ -220,6 +228,12 @@
73 test/lib/tool/checksum.c \
74 test/lib/tool/pk.c
75
76+test_check_modules_midauth_SOURCES = test/check_modules_midauth.c \
77+ test/modules/midauth/lib/midauth_builder.c \
78+ test/modules/midauth/hipd/midauth.c \
79+ $(hipd_hipd_sources)
80+
81+
82 ### static library dependencies ###
83
84 firewall_hipfw_LDADD = lib/core/libhipcore.la
85@@ -228,6 +242,7 @@
86 test_check_firewall_LDADD = lib/core/libhipcore.la
87 test_check_lib_core_LDADD = lib/core/libhipcore.la
88 test_check_lib_tool_LDADD = lib/core/libhipcore.la
89+test_check_modules_midauth_LDADD = lib/core/libhipcore.la
90 test_certteststub_LDADD = lib/core/libhipcore.la
91 test_dh_performance_LDADD = lib/core/libhipcore.la
92 test_fw_port_bindings_performance_LDADD = lib/core/libhipcore.la
93
94=== modified file 'firewall/midauth.h'
95--- firewall/midauth.h 2011-04-20 09:10:18 +0000
96+++ firewall/midauth.h 2011-08-09 11:19:24 +0000
97@@ -1,5 +1,5 @@
98 /*
99- * Copyright (c) 2010 Aalto University and RWTH Aachen University.
100+ * Copyright (c) 2010-2011 Aalto University and RWTH Aachen University.
101 *
102 * Permission is hereby granted, free of charge, to any person
103 * obtaining a copy of this software and associated documentation
104@@ -36,6 +36,7 @@
105 #include <stdint.h>
106
107 #include "lib/core/protodefs.h"
108+#include "modules/midauth/lib/midauth_builder.h"
109 #include "firewall_defines.h"
110
111 typedef int (*midauth_handler)(struct hip_fw_context *ctx);
112
113=== modified file 'hipd/hipd.c'
114--- hipd/hipd.c 2011-05-16 11:39:06 +0000
115+++ hipd/hipd.c 2011-08-09 11:19:24 +0000
116@@ -205,7 +205,7 @@
117 * @param flags pointer to the startup flags container
118 * @return nonzero if the caller should exit, 0 otherwise
119 */
120-static int hipd_parse_cmdline_opts(int argc, char *argv[], uint64_t *flags)
121+int hipd_parse_cmdline_opts(int argc, char *argv[], uint64_t *flags)
122 {
123 int c;
124
125@@ -276,7 +276,7 @@
126 * @param flags startup flags
127 * @return 0 on success, negative error code otherwise
128 */
129-static int hipd_main(uint64_t flags)
130+int hipd_main(uint64_t flags)
131 {
132 int highest_descriptor = 0, err = 0;
133 struct timeval timeout;
134@@ -422,48 +422,3 @@
135
136 return err;
137 }
138-
139-/**
140- * the main function for hipd
141- *
142- * @param argc number of command line arguments
143- * @param argv the command line arguments
144- * @return zero on success or negative on error
145- */
146-int main(int argc, char *argv[])
147-{
148- uint64_t sflags = HIPD_START_FOREGROUND | HIPD_START_LOWCAP;
149-
150- /* The flushing is enabled by default. The reason for this is that
151- * people are doing some very experimental features on some branches
152- * that may crash the daemon and leave the SAs floating around to
153- * disturb further base exchanges. Use -N flag to disable this. */
154- sflags |= HIPD_START_FLUSH_IPSEC;
155-
156- /* The default behaviour is to allow hipd to load the required modules
157- * and unload them when exiting.
158- */
159- sflags |= HIPD_START_LOAD_KMOD;
160-
161- /* set the initial verbosity level */
162- hip_set_logdebug(LOGDEBUG_MEDIUM);
163-
164- /* One should be able to check the hipd version and usage,
165- * even without having root privileges.
166- */
167- if (hipd_parse_cmdline_opts(argc, argv, &sflags)) {
168- return EXIT_SUCCESS;
169- }
170-
171- /* We need to recreate the NAT UDP sockets to bind to the new port. */
172- if (getuid()) {
173- HIP_ERROR("hipd must be started as root!\n");
174- return EXIT_FAILURE;
175- }
176-
177- if (hipd_main(sflags)) {
178- return EXIT_FAILURE;
179- }
180-
181- return EXIT_SUCCESS;
182-}
183
184=== modified file 'hipd/hipd.h'
185--- hipd/hipd.h 2011-05-16 11:00:04 +0000
186+++ hipd/hipd.h 2011-08-09 11:19:24 +0000
187@@ -112,4 +112,7 @@
188 /* Functions for handling outgoing packets. */
189 int hip_sendto_firewall(const struct hip_common *msg);
190
191+int hipd_parse_cmdline_opts(int argc, char *argv[], uint64_t * flags);
192+int hipd_main(uint64_t flags);
193+
194 #endif /* HIP_HIPD_HIPD_H */
195
196=== added file 'hipd/main.c'
197--- hipd/main.c 1970-01-01 00:00:00 +0000
198+++ hipd/main.c 2011-08-09 11:19:24 +0000
199@@ -0,0 +1,84 @@
200+/*
201+ * Copyright (c) 2010-2011 Aalto University and RWTH Aachen University.
202+ *
203+ * Permission is hereby granted, free of charge, to any person
204+ * obtaining a copy of this software and associated documentation
205+ * files (the "Software"), to deal in the Software without
206+ * restriction, including without limitation the rights to use,
207+ * copy, modify, merge, publish, distribute, sublicense, and/or sell
208+ * copies of the Software, and to permit persons to whom the
209+ * Software is furnished to do so, subject to the following
210+ * conditions:
211+ *
212+ * The above copyright notice and this permission notice shall be
213+ * included in all copies or substantial portions of the Software.
214+ *
215+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
216+ * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES
217+ * OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
218+ * NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT
219+ * HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY,
220+ * WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
221+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
222+ * OTHER DEALINGS IN THE SOFTWARE.
223+ */
224+
225+/**
226+ * @file
227+ * The HIPL main file containing the daemon main function.
228+ */
229+
230+#include <stdint.h>
231+#include <unistd.h>
232+#include <sys/types.h>
233+
234+
235+#include "hipd/hipd.h"
236+#include "init.h"
237+#include "lib/core/debug.h"
238+
239+
240+/**
241+ * the main function for hipd
242+ *
243+ * @param argc number of command line arguments
244+ * @param argv the command line arguments
245+ * @return zero on success or negative on error
246+ */
247+int main(int argc, char *argv[])
248+{
249+ uint64_t sflags = HIPD_START_FOREGROUND | HIPD_START_LOWCAP;
250+
251+ /* The flushing is enabled by default. The reason for this is that
252+ * people are doing some very experimental features on some branches
253+ * that may crash the daemon and leave the SAs floating around to
254+ * disturb further base exchanges. Use -N flag to disable this. */
255+ sflags |= HIPD_START_FLUSH_IPSEC;
256+
257+ /* The default behaviour is to allow hipd to load the required modules
258+ * and unload them when exiting.
259+ */
260+ sflags |= HIPD_START_LOAD_KMOD;
261+
262+ /* set the initial verbosity level */
263+ hip_set_logdebug(LOGDEBUG_MEDIUM);
264+
265+ /* One should be able to check the hipd version and usage,
266+ * even without having root privileges.
267+ */
268+ if (hipd_parse_cmdline_opts(argc, argv, &sflags)) {
269+ return EXIT_SUCCESS;
270+ }
271+
272+ /* We need to recreate the NAT UDP sockets to bind to the new port. */
273+ if (getuid()) {
274+ HIP_ERROR("hipd must be started as root!\n");
275+ return EXIT_FAILURE;
276+ }
277+
278+ if (hipd_main(sflags)) {
279+ return EXIT_FAILURE;
280+ }
281+
282+ return EXIT_SUCCESS;
283+}
284
285=== modified file 'hipd/output.c'
286--- hipd/output.c 2011-08-08 13:33:21 +0000
287+++ hipd/output.c 2011-08-09 11:19:24 +0000
288@@ -1050,16 +1050,6 @@
289
290 /********** CHALLENGE_RESPONSE **********/
291 #ifdef CONFIG_HIP_MIDAUTH
292- /** @todo no caching is done for PUZZLE_M parameters. This may be
293- * a DOS attack vector.
294- */
295- HIP_IFEL(hip_solve_puzzle_m(ctx->output_msg, ctx->input_msg),
296- -1, "Building of Challenge_Response failed\n");
297- midauth_cert = hip_pisa_get_certificate();
298-
299- HIP_IFEL(hip_build_param(ctx->output_msg, ctx->hadb_entry->our_pub), -1,
300- "Building of host id failed\n");
301-
302 /* For now we just add some random data to see if it works */
303 HIP_IFEL(hip_build_param_cert(ctx->output_msg, 1, 1, 1, 1, midauth_cert,
304 strlen(midauth_cert)),
305
306=== modified file 'lib/core/builder.c'
307--- lib/core/builder.c 2011-08-08 14:35:56 +0000
308+++ lib/core/builder.c 2011-08-09 11:19:24 +0000
309@@ -679,13 +679,6 @@
310 HIP_PARAM_ESP_PROT_BRANCH,
311 HIP_PARAM_ESP_PROT_SECRET,
312 HIP_PARAM_ESP_PROT_ROOT
313-#ifdef CONFIG_HIP_MIDAUTH
314- ,
315- HIP_PARAM_ECHO_REQUEST_M,
316- HIP_PARAM_ECHO_RESPONSE_M,
317- HIP_PARAM_CHALLENGE_REQUEST,
318- HIP_PARAM_CHALLENGE_RESPONSE
319-#endif /* CONFIG_HIP_MIDAUTH */
320 };
321 hip_tlv type = hip_get_param_type(param);
322
323@@ -1166,10 +1159,8 @@
324 case HIP_PARAM_DST_ADDR: return "HIP_PARAM_DST_ADDR";
325 case HIP_PARAM_ECHO_REQUEST: return "HIP_PARAM_ECHO_REQUEST";
326 case HIP_PARAM_ECHO_REQUEST_SIGN: return "HIP_PARAM_ECHO_REQUEST_SIGN";
327- case HIP_PARAM_ECHO_REQUEST_M: return "HIP_PARAM_ECHO_REQUEST_M";
328 case HIP_PARAM_ECHO_RESPONSE: return "HIP_PARAM_ECHO_RESPONSE";
329 case HIP_PARAM_ECHO_RESPONSE_SIGN: return "HIP_PARAM_ECHO_RESPONSE_SIGN";
330- case HIP_PARAM_ECHO_RESPONSE_M: return "HIP_PARAM_ECHO_RESPONSE_M";
331 case HIP_PARAM_EID_ADDR: return "HIP_PARAM_EID_ADDR";
332 case HIP_PARAM_EID_ENDPOINT: return "HIP_PARAM_EID_ENDPOINT";
333 case HIP_PARAM_EID_IFACE: return "HIP_PARAM_EID_IFACE";
334@@ -1200,7 +1191,6 @@
335 case HIP_PARAM_NOTIFICATION: return "HIP_PARAM_NOTIFICATION";
336 case HIP_PARAM_PORTPAIR: return "HIP_PARAM_PORTPAIR";
337 case HIP_PARAM_PUZZLE: return "HIP_PARAM_PUZZLE";
338- case HIP_PARAM_CHALLENGE_REQUEST: return "HIP_PARAM_CHALLENGE_REQUEST";
339 case HIP_PARAM_R1_COUNTER: return "HIP_PARAM_R1_COUNTER";
340 case HIP_PARAM_REG_FAILED: return "HIP_PARAM_REG_FAILED";
341 case HIP_PARAM_REG_FROM: return "HIP_PARAM_REG_FROM";
342@@ -1213,7 +1203,6 @@
343 case HIP_PARAM_RVS_HMAC: return "HIP_PARAM_RVS_HMAC";
344 case HIP_PARAM_SEQ: return "HIP_PARAM_SEQ";
345 case HIP_PARAM_SOLUTION: return "HIP_PARAM_SOLUTION";
346- case HIP_PARAM_CHALLENGE_RESPONSE: return "HIP_PARAM_CHALLENGE_RESPONSE";
347 case HIP_PARAM_SRC_ADDR: return "HIP_PARAM_SRC_ADDR";
348 case HIP_PARAM_TO_PEER: return "HIP_PARAM_TO_PEER";
349 case HIP_PARAM_UINT: return "HIP_PARAM_UINT";
350@@ -2339,88 +2328,6 @@
351 hip_get_param_contents_direct(&puzzle));
352 }
353
354-#ifdef CONFIG_HIP_MIDAUTH
355-/**
356- * Build and append a HIP challenge_request to the message.
357- *
358- * The puzzle mechanism assumes that every value is in network byte order
359- * except for the hip_birthday_cookie.cv union, where the value is in
360- * host byte order. This is an exception to the normal builder rules, where
361- * input arguments are normally always in host byte order.
362- *
363- * @param msg the message where the puzzle_m is to be appended
364- * @param val_K the K value for the puzzle_m
365- * @param lifetime lifetime field of the puzzle_m
366- * @param opaque the opaque data filed of the puzzle_m
367- * @param opaque_len the length uf the opaque data field
368- *
369- * @return zero for success, or non-zero on error
370- */
371-int hip_build_param_challenge_request(struct hip_common *msg,
372- uint8_t val_K,
373- uint8_t lifetime,
374- uint8_t *opaque,
375- uint8_t opaque_len)
376-{
377- struct hip_challenge_request puzzle;
378-
379- /* note: the length cannot be calculated with calc_param_len() */
380- hip_set_param_contents_len((struct hip_tlv_common *) &puzzle,
381- sizeof(struct hip_challenge_request) -
382- sizeof(struct hip_tlv_common));
383- /* Type 2 (in R1) or 3 (in I2) */
384- hip_set_param_type((struct hip_tlv_common *) &puzzle,
385- HIP_PARAM_CHALLENGE_REQUEST);
386-
387- /* only the random_j_k is in host byte order */
388- puzzle.K = val_K;
389- puzzle.lifetime = lifetime;
390- memcpy(&puzzle.opaque, opaque, opaque_len);
391-
392- return hip_build_generic_param(msg, &puzzle, sizeof(struct hip_tlv_common),
393- hip_get_param_contents_direct(&puzzle));
394-}
395-
396-/**
397- * Build and append a HIP solution into the message.
398- *
399- * The puzzle mechanism assumes that every value is in network byte order
400- * except for the hip_birthday_cookie.cv union, where the value is in
401- * host byte order. This is an exception to the normal builder rules, where
402- * input arguments are normally always in host byte order.
403- *
404- * @param msg the message where the solution is to be appended
405- * @param pz values from the corresponding hip_challenge_request copied to the solution
406- * @param solution value for the solution (in host byte order)
407- *
408- * @return zero for success, or non-zero on error
409- */
410-int hip_build_param_challenge_response(struct hip_common *const msg,
411- const struct hip_challenge_request *const pz,
412- const uint8_t solution[PUZZLE_LENGTH])
413-{
414- struct hip_challenge_response cookie;
415- int opaque_len = 0;
416-
417- /* note: the length cannot be calculated with calc_param_len() */
418- hip_set_param_contents_len((struct hip_tlv_common *) &cookie,
419- sizeof(struct hip_challenge_response) -
420- sizeof(struct hip_tlv_common));
421- /* Type 2 (in R1) or 3 (in I2) */
422- hip_set_param_type((struct hip_tlv_common *) &cookie, HIP_PARAM_CHALLENGE_RESPONSE);
423-
424- memcpy(cookie.J, solution, PUZZLE_LENGTH);
425- cookie.K = pz->K;
426- cookie.lifetime = pz->K;
427- opaque_len = sizeof(pz->opaque) / sizeof(pz->opaque[0]);
428- memcpy(&cookie.opaque, pz->opaque, opaque_len);
429-
430- return hip_build_generic_param(msg, &cookie, sizeof(struct hip_tlv_common),
431- hip_get_param_contents_direct(&cookie));
432-}
433-
434-#endif /* CONFIG_HIP_MIDAUTH */
435-
436 /**
437 * Build and append a HIP solution into the message.
438 *
439@@ -3850,6 +3757,14 @@
440 SHA_CTX sha;
441 MD5_CTX md5;
442
443+ HIP_ASSERT(in != NULL);
444+ HIP_ASSERT(out != NULL);
445+
446+ if (in_len <= 0) {
447+ HIP_ERROR("invalid input length: %i\n", in_len);
448+ return -1;
449+ }
450+
451 switch (type) {
452 case HIP_DIGEST_SHA1:
453 SHA1_Init(&sha);
454
455=== modified file 'lib/core/builder.h'
456--- lib/core/builder.h 2011-07-18 13:10:26 +0000
457+++ lib/core/builder.h 2011-08-09 11:19:24 +0000
458@@ -133,12 +133,6 @@
459 const uint32_t opaque,
460 const uint8_t *const random_i);
461
462-int hip_build_param_challenge_request(struct hip_common *,
463- uint8_t,
464- uint8_t,
465- uint8_t *,
466- uint8_t);
467-
468 int hip_build_param_r1_counter(struct hip_common *, uint64_t);
469
470 int hip_build_param_signature2_contents(struct hip_common *,
471@@ -153,10 +147,6 @@
472 const struct hip_puzzle *,
473 uint8_t *const);
474
475-int hip_build_param_challenge_response(struct hip_common *const msg,
476- const struct hip_challenge_request *const pz,
477- const uint8_t *const solution);
478-
479 int hip_build_param(struct hip_common *, const void *);
480 void hip_set_msg_response(struct hip_common *msg, uint8_t on);
481 uint8_t hip_get_msg_response(struct hip_common *msg);
482
483=== modified file 'lib/core/protodefs.h'
484--- lib/core/protodefs.h 2011-07-28 18:11:17 +0000
485+++ lib/core/protodefs.h 2011-08-09 11:19:24 +0000
486@@ -140,7 +140,6 @@
487 #define HIP_PARAM_LOCATOR 193
488 #define HIP_PARAM_PUZZLE 257
489 #define HIP_PARAM_SOLUTION 321
490-#define HIP_PARAM_CHALLENGE_RESPONSE 322
491 #define HIP_PARAM_SEQ 385
492 #define HIP_PARAM_ACK 449
493 #define HIP_PARAM_DIFFIE_HELLMAN 513
494@@ -156,7 +155,6 @@
495 #define HIP_PARAM_REG_FAILED 936
496 #define HIP_PARAM_REG_FROM 950
497 #define HIP_PARAM_ECHO_RESPONSE_SIGN 961
498-#define HIP_PARAM_ECHO_RESPONSE_M 962
499 #define HIP_PARAM_ESP_TRANSFORM 4095
500 #define HIP_PARAM_ESP_PROT_TRANSFORMS 4120
501 #define HIP_PARAM_ESP_PROT_ANCHOR 4121
502@@ -230,8 +228,6 @@
503 //#define HIP_PARAM_REG_FROM 64010
504 #define HIP_PARAM_TO_PEER 64006
505 #define HIP_PARAM_FROM_PEER 64008
506-#define HIP_PARAM_ECHO_REQUEST_M 65332
507-#define HIP_PARAM_CHALLENGE_REQUEST 65334
508 #define HIP_PARAM_FROM 65498
509 #define HIP_PARAM_RVS_HMAC 65500
510 #define HIP_PARAM_VIA_RVS 65502
511@@ -783,25 +779,6 @@
512 uint8_t J[PUZZLE_LENGTH];
513 } __attribute__((packed));
514
515-
516-
517-struct hip_challenge_request {
518- hip_tlv type;
519- hip_tlv_len length;
520- uint8_t K;
521- uint8_t lifetime;
522- uint8_t opaque[24]; /**< variable length */
523-} __attribute__((packed));
524-
525-struct hip_challenge_response {
526- hip_tlv type;
527- hip_tlv_len length;
528- uint8_t K;
529- uint8_t lifetime;
530- uint8_t J[PUZZLE_LENGTH];
531- uint8_t opaque[24]; /**< variable length */
532-} __attribute__((packed));
533-
534 struct hip_dh_public_value {
535 uint8_t group_id;
536 uint16_t pub_len;
537
538=== modified file 'lib/core/solve.c'
539--- lib/core/solve.c 2011-04-20 09:38:33 +0000
540+++ lib/core/solve.c 2011-08-09 11:19:24 +0000
541@@ -1,5 +1,5 @@
542 /*
543- * Copyright (c) 2010 Aalto University and RWTH Aachen University.
544+ * Copyright (c) 2010-2011 Aalto University and RWTH Aachen University.
545 *
546 * Permission is hereby granted, free of charge, to any person
547 * obtaining a copy of this software and associated documentation
548@@ -174,59 +174,3 @@
549
550 return 0;
551 }
552-
553-#ifdef CONFIG_HIP_MIDAUTH
554-/**
555- * solve a midauth puzzle which is essentially a normal HIP cookie
556- * with some extra whipped cream on the top
557- *
558- * @param out the received R1 message
559- * @param in an I2 message where the solution will be written
560- * @return zero on success and negative on error
561- * @see <a
562- * href="http://tools.ietf.org/id/draft-heer-hip-middle-auth">Heer et
563- * al, End-Host Authentication for HIP Middleboxes, Internet draft,
564- * work in progress, February 2009</a>
565- */
566-int hip_solve_puzzle_m(struct hip_common *const out,
567- const struct hip_common *const in)
568-{
569- struct puzzle_hash_input puzzle_input;
570- const struct hip_challenge_request *pz;
571- uint8_t digest[HIP_AH_SHA_LEN];
572-
573- pz = hip_get_param(in, HIP_PARAM_CHALLENGE_REQUEST);
574- while (pz) {
575- if (hip_get_param_type(pz) != HIP_PARAM_CHALLENGE_REQUEST) {
576- break;
577- }
578-
579- if (hip_build_digest(HIP_DIGEST_SHA1, pz->opaque, 24, digest) < 0) {
580- HIP_ERROR("Building of SHA1 Random seed I failed\n");
581- return -1;
582- }
583-
584- memcpy(puzzle_input.puzzle,
585- &digest[HIP_AH_SHA_LEN - PUZZLE_LENGTH],
586- PUZZLE_LENGTH);
587- puzzle_input.initiator_hit = out->hits;
588- puzzle_input.responder_hit = out->hitr;
589- RAND_bytes(puzzle_input.solution, PUZZLE_LENGTH);
590-
591- if ((hip_solve_puzzle(&puzzle_input, pz->K))) {
592- HIP_ERROR("Solving of puzzle failed\n");
593- return -EINVAL;
594- }
595-
596- if (hip_build_param_challenge_response(out, pz, puzzle_input.solution) < 0) {
597- HIP_ERROR("Error while creating solution_m reply parameter\n");
598- return -1;
599- }
600- pz = (const struct hip_challenge_request *)
601- hip_get_next_param(in, (const struct hip_tlv_common *) pz);
602- }
603-
604- return 0;
605-}
606-
607-#endif /* CONFIG_HIP_MIDAUTH */
608
609=== modified file 'lib/core/solve.h'
610--- lib/core/solve.h 2011-07-28 18:11:17 +0000
611+++ lib/core/solve.h 2011-08-09 11:19:24 +0000
612@@ -1,5 +1,5 @@
613 /*
614- * Copyright (c) 2010 Aalto University and RWTH Aachen University.
615+ * Copyright (c) 2010-2011 Aalto University and RWTH Aachen University.
616 *
617 * Permission is hereby granted, free of charge, to any person
618 * obtaining a copy of this software and associated documentation
619@@ -55,7 +55,4 @@
620 int hip_verify_puzzle_solution(const struct puzzle_hash_input *const puzzle_input,
621 const uint8_t difficulty);
622
623-int hip_solve_puzzle_m(struct hip_common *const out,
624- const struct hip_common *const in);
625-
626 #endif /* HIP_LIB_CORE_SOLVE_H */
627
628=== added directory 'modules/midauth'
629=== added directory 'modules/midauth/hipd'
630=== added file 'modules/midauth/hipd/midauth.c'
631--- modules/midauth/hipd/midauth.c 1970-01-01 00:00:00 +0000
632+++ modules/midauth/hipd/midauth.c 2011-08-09 11:19:24 +0000
633@@ -0,0 +1,229 @@
634+/*
635+ * Copyright (c) 2010-2011 Aalto University and RWTH Aachen University.
636+ *
637+ * Permission is hereby granted, free of charge, to any person
638+ * obtaining a copy of this software and associated documentation
639+ * files (the "Software"), to deal in the Software without
640+ * restriction, including without limitation the rights to use,
641+ * copy, modify, merge, publish, distribute, sublicense, and/or sell
642+ * copies of the Software, and to permit persons to whom the
643+ * Software is furnished to do so, subject to the following
644+ * conditions:
645+ *
646+ * The above copyright notice and this permission notice shall be
647+ * included in all copies or substantial portions of the Software.
648+ *
649+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
650+ * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES
651+ * OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
652+ * NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT
653+ * HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY,
654+ * WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
655+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
656+ * OTHER DEALINGS IN THE SOFTWARE.
657+ */
658+
659+/**
660+ * @file
661+ * This file contains the implementation for the middlebox authentication
662+ * extension.
663+ *
664+ * @author Rene Hummen
665+ */
666+
667+#include <errno.h>
668+#include <stdint.h>
669+#include <string.h>
670+
671+#include "hipd/hidb.h"
672+#include "hipd/pkt_handling.h"
673+#include "lib/core/builder.h"
674+#include "lib/core/common.h"
675+#include "lib/core/ife.h"
676+#include "lib/core/modularization.h"
677+#include "lib/core/protodefs.h"
678+#include "lib/core/solve.h"
679+#include "modules/midauth/lib/midauth_builder.h"
680+#include "modules/update/hipd/update.h"
681+#include "midauth.h"
682+
683+
684+/**
685+ * Handle the CHALLENGE_REQUEST parameter.
686+ *
687+ * @param packet_type The packet type of the control message (RFC 5201, 5.3.)
688+ * @param ha_state The host association state (RFC 5201, 4.4.1.)
689+ * @param ctx Pointer to the packet context, containing all information for
690+ * the packet handling (received message, source and destination
691+ * address, the ports and the corresponding entry from the host
692+ * association database).
693+ *
694+ * @return zero if the challenge was processed correctly or no challenge
695+ * parameter was attached to the packet, negative value otherwise.
696+ */
697+static int handle_challenge_request_param(UNUSED const uint8_t packet_type,
698+ UNUSED const uint32_t ha_state,
699+ struct hip_packet_context *ctx)
700+{
701+ const struct hip_challenge_request *request = NULL;
702+
703+ request = hip_get_param(ctx->input_msg, HIP_PARAM_CHALLENGE_REQUEST);
704+
705+ // each on-path middlebox may add a challenge on its own
706+ while (request &&
707+ hip_get_param_type(request) == HIP_PARAM_CHALLENGE_REQUEST) {
708+ struct puzzle_hash_input tmp_puzzle;
709+ const unsigned int len = hip_challenge_request_opaque_len(request);
710+
711+ if (hip_midauth_puzzle_seed(request->opaque, len, tmp_puzzle.puzzle)) {
712+ HIP_ERROR("failed to derive midauth puzzle\n");
713+ return -1;
714+ }
715+
716+ tmp_puzzle.initiator_hit = ctx->input_msg->hitr;
717+ tmp_puzzle.responder_hit = ctx->input_msg->hits;
718+
719+ if (hip_solve_puzzle(&tmp_puzzle, request->K)) {
720+ HIP_ERROR("Solving of middlebox challenge failed\n");
721+ return -EINVAL;
722+ }
723+
724+ if (hip_build_param_challenge_response(ctx->output_msg,
725+ request,
726+ tmp_puzzle.solution) < 0) {
727+ HIP_ERROR("Error while creating CHALLENGE_RESPONSE parameter\n");
728+ return -1;
729+ }
730+
731+ // process next challenge parameter, if available
732+ request = (const struct hip_challenge_request *)
733+ hip_get_next_param(ctx->input_msg, &request->tlv);
734+ }
735+
736+ return 0;
737+}
738+
739+/**
740+ * Add a HOST_ID parameter corresponding to the local HIT of the association to
741+ * an UPDATE packet.
742+ *
743+ * @param packet_type The packet type of the control message (RFC 5201, 5.3.)
744+ * @param ha_state The host association state (RFC 5201, 4.4.1.)
745+ * @param ctx Pointer to the packet context, containing all information for
746+ * the packet handling (received message, source and destination
747+ * address, the ports and the corresponding entry from the host
748+ * association database).
749+ *
750+ * @return zero on success, negative value otherwise
751+ */
752+static int add_host_id_param_update(UNUSED const uint8_t packet_type,
753+ UNUSED const uint32_t ha_state,
754+ struct hip_packet_context *ctx)
755+{
756+ const struct hip_challenge_request *const challenge_request =
757+ hip_get_param(ctx->input_msg, HIP_PARAM_CHALLENGE_REQUEST);
758+
759+ // add HOST_ID to packets containing a CHALLENGE_RESPONSE
760+ if (challenge_request) {
761+ const struct local_host_id *const host_id_entry =
762+ hip_get_hostid_entry_by_lhi_and_algo(HIP_DB_LOCAL_HID,
763+ &ctx->input_msg->hitr,
764+ HIP_ANY_ALGO,
765+ -1);
766+ if (!host_id_entry) {
767+ HIP_ERROR("Unknown HIT\n");
768+ return -1;
769+ }
770+
771+ if (hip_build_param_host_id(ctx->output_msg, &host_id_entry->host_id)) {
772+ HIP_ERROR("Building of host id failed\n");
773+ return -1;
774+ }
775+ }
776+
777+ return 0;
778+}
779+
780+/**
781+ * Initialization function for the midauth module.
782+ *
783+ * @return zero on success, negative value otherwise
784+ */
785+int hip_midauth_init(void)
786+{
787+ if (lmod_register_parameter_type(HIP_PARAM_CHALLENGE_REQUEST,
788+ "HIP_PARAM_CHALLENGE_REQUEST")) {
789+ HIP_ERROR("failed to register parameter type\n");
790+ return -1;
791+ }
792+
793+ if (lmod_register_parameter_type(HIP_PARAM_CHALLENGE_RESPONSE,
794+ "HIP_PARAM_CHALLENGE_RESPONSE")) {
795+ HIP_ERROR("failed to register parameter type\n");
796+ return -1;
797+ }
798+
799+ const int challenge_request_R1_states[] = { HIP_STATE_I1_SENT,
800+ HIP_STATE_I2_SENT,
801+ HIP_STATE_CLOSING,
802+ HIP_STATE_CLOSED };
803+ for (unsigned i = 0; i < ARRAY_SIZE(challenge_request_R1_states); i++) {
804+ if (hip_register_handle_function(HIP_R1,
805+ challenge_request_R1_states[i],
806+ &handle_challenge_request_param,
807+ 32500)) {
808+ HIP_ERROR("Error on registering MIDAUTH handle function.\n");
809+ return -1;
810+ }
811+ }
812+
813+ //
814+ // we hook on every occasion that causes an R2 to get sent.
815+ // R2 packet is first allocated at 40000, so we use a higher
816+ // base priority here.
817+ //
818+ const int challenge_request_I2_states[] = { HIP_STATE_UNASSOCIATED,
819+ HIP_STATE_I1_SENT,
820+ HIP_STATE_I2_SENT,
821+ HIP_STATE_R2_SENT,
822+ HIP_STATE_ESTABLISHED,
823+ HIP_STATE_CLOSING,
824+ HIP_STATE_CLOSED,
825+ HIP_STATE_NONE };
826+ for (unsigned i = 0; i < ARRAY_SIZE(challenge_request_I2_states); i++) {
827+ if (hip_register_handle_function(HIP_I2,
828+ challenge_request_I2_states[i],
829+ &handle_challenge_request_param,
830+ 40322)) {
831+ HIP_ERROR("Error on registering MIDAUTH handle function.\n");
832+ return -1;
833+ }
834+ }
835+
836+ //
837+ // Priority computed the same as above, but UPDATE response is sent at
838+ // priority 30000 already (checking is 20000) and we must add our
839+ // CHALLENGE_REQUEST verification in between, hence a lower base priority.
840+ //
841+ const int challenge_request_UPDATE_states[] = { HIP_STATE_R2_SENT,
842+ HIP_STATE_ESTABLISHED };
843+ for (unsigned i = 0; i < ARRAY_SIZE(challenge_request_UPDATE_states); i++) {
844+ if (hip_register_handle_function(HIP_UPDATE,
845+ challenge_request_UPDATE_states[i],
846+ &handle_challenge_request_param,
847+ 20322)) {
848+ HIP_ERROR("Error on registering MIDAUTH handle function.\n");
849+ return -1;
850+ }
851+
852+ if (hip_register_handle_function(HIP_UPDATE,
853+ challenge_request_UPDATE_states[i],
854+ &add_host_id_param_update,
855+ 20750)) {
856+ HIP_ERROR("Error on registering MIDAUTH handle function.\n");
857+ return -1;
858+ }
859+ }
860+
861+ return 0;
862+}
863
864=== added file 'modules/midauth/hipd/midauth.h'
865--- modules/midauth/hipd/midauth.h 1970-01-01 00:00:00 +0000
866+++ modules/midauth/hipd/midauth.h 2011-08-09 11:19:24 +0000
867@@ -0,0 +1,34 @@
868+/*
869+ * Copyright (c) 2010-2011 Aalto University and RWTH Aachen University.
870+ *
871+ * Permission is hereby granted, free of charge, to any person
872+ * obtaining a copy of this software and associated documentation
873+ * files (the "Software"), to deal in the Software without
874+ * restriction, including without limitation the rights to use,
875+ * copy, modify, merge, publish, distribute, sublicense, and/or sell
876+ * copies of the Software, and to permit persons to whom the
877+ * Software is furnished to do so, subject to the following
878+ * conditions:
879+ *
880+ * The above copyright notice and this permission notice shall be
881+ * included in all copies or substantial portions of the Software.
882+ *
883+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
884+ * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES
885+ * OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
886+ * NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT
887+ * HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY,
888+ * WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
889+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
890+ * OTHER DEALINGS IN THE SOFTWARE.
891+ */
892+
893+#ifndef MODULES_MIDAUTH_HIPD_MIDAUTH_H
894+#define MODULES_MIDAUTH_HIPD_MIDAUTH_H
895+
896+#define HIP_PARAM_CHALLENGE_REQUEST 65334
897+#define HIP_PARAM_CHALLENGE_RESPONSE 322
898+
899+int hip_midauth_init(void);
900+
901+#endif /* MODULES_MIDAUTH_HIPD_MIDAUTH_H */
902
903=== added directory 'modules/midauth/lib'
904=== added file 'modules/midauth/lib/midauth_builder.c'
905--- modules/midauth/lib/midauth_builder.c 1970-01-01 00:00:00 +0000
906+++ modules/midauth/lib/midauth_builder.c 2011-08-09 11:19:24 +0000
907@@ -0,0 +1,181 @@
908+/*
909+ * Copyright (c) 2010-2011 Aalto University and RWTH Aachen University.
910+ *
911+ * Permission is hereby granted, free of charge, to any person
912+ * obtaining a copy of this software and associated documentation
913+ * files (the "Software"), to deal in the Software without
914+ * restriction, including without limitation the rights to use,
915+ * copy, modify, merge, publish, distribute, sublicense, and/or sell
916+ * copies of the Software, and to permit persons to whom the
917+ * Software is furnished to do so, subject to the following
918+ * conditions:
919+ *
920+ * The above copyright notice and this permission notice shall be
921+ * included in all copies or substantial portions of the Software.
922+ *
923+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
924+ * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES
925+ * OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
926+ * NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT
927+ * HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY,
928+ * WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
929+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
930+ * OTHER DEALINGS IN THE SOFTWARE.
931+ */
932+
933+/**
934+ * @file
935+ * This file contains parameter handling functionality for the middlebox
936+ * authentication extension.
937+ *
938+ * @author Rene Hummen
939+ * @author Christof Mroz <christof.mroz@rwth-aachen.de>
940+ */
941+
942+#include <stdint.h>
943+#include <string.h>
944+
945+#include "lib/core/ife.h"
946+#include "modules/midauth/hipd/midauth.h"
947+#include "midauth_builder.h"
948+
949+
950+/**
951+ * Build and append a HIP CHALLENGE_REQUEST to the message.
952+ *
953+ * @param msg the message where the CHALLENGE_REQUEST is appended
954+ * @param difficulty the puzzle difficulty for the CHALLENGE_REQUEST
955+ * @param lifetime lifetime the puzzle nonce
956+ * @param opaque the nonce (challenge) of the CHALLENGE_REQUEST
957+ * @param opaque_len the length of the nonce
958+ *
959+ * @return zero for success, or negative value on error
960+ */
961+int hip_build_param_challenge_request(struct hip_common *const msg,
962+ const uint8_t difficulty,
963+ const uint8_t lifetime,
964+ const uint8_t *opaque,
965+ const uint8_t opaque_len)
966+{
967+ struct hip_challenge_request request;
968+ static const size_t min_length = sizeof(request) -
969+ sizeof(request.tlv) -
970+ sizeof(request.opaque);
971+
972+ HIP_ASSERT(opaque);
973+
974+ /* note: the length cannot be calculated with calc_param_len() */
975+ hip_set_param_contents_len(&request.tlv, min_length + opaque_len);
976+ hip_set_param_type(&request.tlv, HIP_PARAM_CHALLENGE_REQUEST);
977+
978+ /* only the random_j_k is in host byte order */
979+ request.K = difficulty;
980+ request.lifetime = lifetime;
981+ memcpy(&request.opaque, opaque, opaque_len);
982+
983+ if (hip_build_param(msg, &request) != 0) {
984+ HIP_ERROR("failed to build parameter\n");
985+ return -1;
986+ }
987+
988+ return 0;
989+}
990+
991+/**
992+ * Build and append a HIP CHALLENGE_RESPONSE to the message.
993+ *
994+ * @param msg the message where the CHALLENGE_RESPONSE is appended
995+ * @param request the received CHALLENGE_REQUEST parameter for this response
996+ * @param solution the solution for the puzzle in the CHALLENGE_REQUEST
997+ *
998+ * @return zero for success, or negative value on error
999+ */
1000+int hip_build_param_challenge_response(struct hip_common *const msg,
1001+ const struct hip_challenge_request *const request,
1002+ const uint8_t solution[PUZZLE_LENGTH])
1003+{
1004+ struct hip_challenge_response response;
1005+ static const size_t min_length = sizeof(response) -
1006+ sizeof(response.tlv) -
1007+ sizeof(response.opaque);
1008+
1009+ if (!request || !solution) {
1010+ HIP_ERROR("Unexpected input parameter values (NULL).\n");
1011+ return -1;
1012+ }
1013+
1014+ const int opaque_len = hip_challenge_request_opaque_len(request);
1015+
1016+ /* note: the length cannot be calculated with calc_param_len() */
1017+ hip_set_param_contents_len(&response.tlv, min_length + opaque_len);
1018+ hip_set_param_type(&response.tlv, HIP_PARAM_CHALLENGE_RESPONSE);
1019+
1020+ memcpy(response.J, solution, PUZZLE_LENGTH);
1021+ response.K = request->K;
1022+ response.lifetime = request->lifetime;
1023+ memcpy(response.opaque, request->opaque, opaque_len);
1024+
1025+ if (hip_build_param(msg, &response)) {
1026+ HIP_ERROR("failed to build parameter\n");
1027+ return -1;
1028+ }
1029+
1030+ return 0;
1031+}
1032+
1033+/**
1034+ * Compute length of opaque field in CHALLENGE_REQUEST parameter.
1035+ *
1036+ * @param request the CHALLENGE_REQUEST parameter
1037+ * @return length of the opaque field; 0 in case of error or if length equals 0
1038+ */
1039+unsigned int hip_challenge_request_opaque_len(const struct hip_challenge_request *const request)
1040+{
1041+ if (!request) {
1042+ return 0;
1043+ }
1044+
1045+ static const size_t min_len = sizeof(*request) -
1046+ sizeof(request->tlv) -
1047+ sizeof(request->opaque);
1048+
1049+ return hip_get_param_contents_len(&request->tlv) - min_len;
1050+}
1051+
1052+/**
1053+ * Convert the opaque value in the CHALLENGE_REQUEST to the seed value I of a
1054+ * HIP puzzle.
1055+ *
1056+ * The opaque value plays a dual role in a CHALLENGE_REQUEST:
1057+ * i) it is a challenge that needs to be echoed back by the responder and
1058+ * ii) it is used to derive the seed value for a cryptographic puzzle. The
1059+ * puzzle is defined in RFC5201.
1060+ *
1061+ * @param opaque the nonce (challenge) in the CHALLENGE_REQUEST
1062+ * @param opaque_len the length of the nonce
1063+ * @param puzzle_value the puzzle value generated from the nonce
1064+ * @return zero on success, -1 in case of an error
1065+ */
1066+int hip_midauth_puzzle_seed(const uint8_t opaque[],
1067+ const unsigned int opaque_len,
1068+ uint8_t puzzle_value[PUZZLE_LENGTH])
1069+{
1070+ unsigned char sha_digest[SHA_DIGEST_LENGTH];
1071+
1072+ HIP_ASSERT(puzzle_value != NULL);
1073+
1074+ // the hashed opaque field is used as puzzle seed
1075+ if (hip_build_digest(HIP_DIGEST_SHA1,
1076+ opaque,
1077+ opaque_len,
1078+ sha_digest)) {
1079+ HIP_ERROR("Building of SHA1 random seed I failed\n");
1080+ return -1;
1081+ }
1082+
1083+ memcpy(puzzle_value,
1084+ &sha_digest[SHA_DIGEST_LENGTH - PUZZLE_LENGTH],
1085+ PUZZLE_LENGTH);
1086+
1087+ return 0;
1088+}
1089
1090=== added file 'modules/midauth/lib/midauth_builder.h'
1091--- modules/midauth/lib/midauth_builder.h 1970-01-01 00:00:00 +0000
1092+++ modules/midauth/lib/midauth_builder.h 2011-08-09 11:19:24 +0000
1093@@ -0,0 +1,79 @@
1094+/*
1095+ * Copyright (c) 2010-2011 Aalto University and RWTH Aachen University.
1096+ *
1097+ * Permission is hereby granted, free of charge, to any person
1098+ * obtaining a copy of this software and associated documentation
1099+ * files (the "Software"), to deal in the Software without
1100+ * restriction, including without limitation the rights to use,
1101+ * copy, modify, merge, publish, distribute, sublicense, and/or sell
1102+ * copies of the Software, and to permit persons to whom the
1103+ * Software is furnished to do so, subject to the following
1104+ * conditions:
1105+ *
1106+ * The above copyright notice and this permission notice shall be
1107+ * included in all copies or substantial portions of the Software.
1108+ *
1109+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
1110+ * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES
1111+ * OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
1112+ * NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT
1113+ * HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY,
1114+ * WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
1115+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
1116+ * OTHER DEALINGS IN THE SOFTWARE.
1117+ */
1118+
1119+/**
1120+ * @file
1121+ * This file contains parameter handling functionality for the middlebox
1122+ * authentication extension.
1123+ *
1124+ * @author Rene Hummen
1125+ * @author Christof Mroz <christof.mroz@rwth-aachen.de>
1126+ */
1127+
1128+#ifndef MODULES_MIDAUTH_HIPD_MIDAUTH_BUILDER_H
1129+#define MODULES_MIDAUTH_HIPD_MIDAUTH_BUILDER_H
1130+
1131+#include <stdint.h>
1132+
1133+#include "lib/core/builder.h"
1134+#include "lib/core/protodefs.h"
1135+
1136+
1137+#define MAX_CHALLENGE_LENGTH 256
1138+
1139+
1140+struct hip_challenge_request {
1141+ struct hip_tlv_common tlv;
1142+ uint8_t K;
1143+ uint8_t lifetime;
1144+ uint8_t opaque[MAX_CHALLENGE_LENGTH]; /**< variable length */
1145+} __attribute__ ((packed));
1146+
1147+struct hip_challenge_response {
1148+ struct hip_tlv_common tlv;
1149+ uint8_t K;
1150+ uint8_t lifetime;
1151+ uint8_t J[PUZZLE_LENGTH];
1152+ uint8_t opaque[MAX_CHALLENGE_LENGTH]; /**< variable length */
1153+} __attribute__ ((packed));
1154+
1155+
1156+int hip_build_param_challenge_request(struct hip_common *const msg,
1157+ const uint8_t difficulty,
1158+ const uint8_t lifetime,
1159+ const uint8_t *opaque,
1160+ const uint8_t opaque_len);
1161+
1162+int hip_build_param_challenge_response(struct hip_common *const msg,
1163+ const struct hip_challenge_request *const request,
1164+ const uint8_t *const solution);
1165+
1166+unsigned int hip_challenge_request_opaque_len(const struct hip_challenge_request *const request);
1167+
1168+int hip_midauth_puzzle_seed(const uint8_t *const opaque,
1169+ const unsigned int opaque_len,
1170+ uint8_t *const puzzle_value);
1171+
1172+#endif /* MODULES_MIDAUTH_HIPD_MIDAUTH_BUILDER_H */
1173
1174=== added file 'modules/midauth/module_info.xml'
1175--- modules/midauth/module_info.xml 1970-01-01 00:00:00 +0000
1176+++ modules/midauth/module_info.xml 2011-08-09 11:19:24 +0000
1177@@ -0,0 +1,47 @@
1178+<!--
1179+Copyright (c) 2010-2011 Aalto University and RWTH Aachen University.
1180+
1181+Permission is hereby granted, free of charge, to any person
1182+obtaining a copy of this software and associated documentation
1183+files (the "Software"), to deal in the Software without
1184+restriction, including without limitation the rights to use,
1185+copy, modify, merge, publish, distribute, sublicense, and/or sell
1186+copies of the Software, and to permit persons to whom the
1187+Software is furnished to do so, subject to the following
1188+conditions:
1189+
1190+The above copyright notice and this permission notice shall be
1191+included in all copies or substantial portions of the Software.
1192+
1193+THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
1194+EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES
1195+OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
1196+NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT
1197+HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY,
1198+WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
1199+FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
1200+OTHER DEALINGS IN THE SOFTWARE.
1201+-->
1202+
1203+<?xml version="1.0" encoding="UTF-8"?>
1204+
1205+<!-- Mandatory: name, version -->
1206+<module
1207+ name="midauth"
1208+ version="0.0.1"
1209+ description="Middlebox authentication functionality for the hip daemon."
1210+ developer=""
1211+ bugaddress="hipl-users@freelists.org"
1212+ webpage="http://infrahip.hiit.fi/">
1213+
1214+ <requires>
1215+ <module name="update" minversion="0.0.1" />
1216+ </requires>
1217+
1218+ <!-- Mandatory: name, header_file, init_function -->
1219+ <application
1220+ name="hipd"
1221+ header_file="modules/midauth/hipd/midauth.h"
1222+ init_function="hip_midauth_init" />
1223+</module>
1224+
1225
1226=== added file 'test/check_modules_midauth.c'
1227--- test/check_modules_midauth.c 1970-01-01 00:00:00 +0000
1228+++ test/check_modules_midauth.c 2011-08-09 11:19:24 +0000
1229@@ -0,0 +1,46 @@
1230+/*
1231+ * Copyright (c) 2010-2011 Aalto University and RWTH Aachen University.
1232+ *
1233+ * Permission is hereby granted, free of charge, to any person
1234+ * obtaining a copy of this software and associated documentation
1235+ * files (the "Software"), to deal in the Software without
1236+ * restriction, including without limitation the rights to use,
1237+ * copy, modify, merge, publish, distribute, sublicense, and/or sell
1238+ * copies of the Software, and to permit persons to whom the
1239+ * Software is furnished to do so, subject to the following
1240+ * conditions:
1241+ *
1242+ * The above copyright notice and this permission notice shall be
1243+ * included in all copies or substantial portions of the Software.
1244+ *
1245+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
1246+ * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES
1247+ * OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
1248+ * NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT
1249+ * HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY,
1250+ * WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
1251+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
1252+ * OTHER DEALINGS IN THE SOFTWARE.
1253+ */
1254+
1255+/**
1256+ * @file
1257+ * @brief Unit tests of modules/midauth (see doc/HACKING on unit tests).
1258+ */
1259+
1260+#include <stdlib.h>
1261+#include <check.h>
1262+
1263+#include "test/modules/midauth/test_suites.h"
1264+
1265+int main(void)
1266+{
1267+ int number_failed;
1268+ SRunner *sr = srunner_create(modules_midauth_lib_builder());
1269+ srunner_add_suite(sr, modules_midauth_hipd_midauth());
1270+
1271+ srunner_run_all(sr, CK_NORMAL);
1272+ number_failed = srunner_ntests_failed(sr);
1273+ srunner_free(sr);
1274+ return (number_failed == 0) ? EXIT_SUCCESS : EXIT_FAILURE;
1275+}
1276
1277=== added directory 'test/modules'
1278=== added directory 'test/modules/midauth'
1279=== added directory 'test/modules/midauth/hipd'
1280=== added file 'test/modules/midauth/hipd/midauth.c'
1281--- test/modules/midauth/hipd/midauth.c 1970-01-01 00:00:00 +0000
1282+++ test/modules/midauth/hipd/midauth.c 2011-08-09 11:19:24 +0000
1283@@ -0,0 +1,120 @@
1284+/*
1285+ * Copyright (c) 2011 Aalto University and RWTH Aachen University.
1286+ *
1287+ * Permission is hereby granted, free of charge, to any person
1288+ * obtaining a copy of this software and associated documentation
1289+ * files (the "Software"), to deal in the Software without
1290+ * restriction, including without limitation the rights to use,
1291+ * copy, modify, merge, publish, distribute, sublicense, and/or sell
1292+ * copies of the Software, and to permit persons to whom the
1293+ * Software is furnished to do so, subject to the following
1294+ * conditions:
1295+ *
1296+ * The above copyright notice and this permission notice shall be
1297+ * included in all copies or substantial portions of the Software.
1298+ *
1299+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
1300+ * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES
1301+ * OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
1302+ * NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT
1303+ * HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY,
1304+ * WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
1305+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
1306+ * OTHER DEALINGS IN THE SOFTWARE.
1307+ */
1308+
1309+#include <check.h>
1310+#include <stdint.h>
1311+
1312+#include "modules/midauth/hipd/midauth.c"
1313+#include "modules/midauth/lib/midauth_builder.h"
1314+#include "../test_suites.h"
1315+
1316+#define MIDAUTH_DEFAULT_NONCE_LENGTH 18
1317+
1318+/* Test where the HIP packet does not contain any CHALLANGE_REQUESTs */
1319+START_TEST(test_midauth_handle_challenge_request_param_0_CORRECT)
1320+{
1321+ struct hip_packet_context ctx = { 0 };
1322+ const struct hip_challenge_response *response = NULL;
1323+
1324+ ctx.input_msg = hip_msg_alloc();
1325+ ctx.output_msg = hip_msg_alloc();
1326+
1327+ fail_unless(handle_challenge_request_param(0, 0, &ctx) == 0, NULL);
1328+
1329+ /* NOTE we can only check for the existance of the response parameter here
1330+ * as the puzzle solution differs for each run. */
1331+ fail_unless((response = hip_get_param(ctx.output_msg,
1332+ HIP_PARAM_CHALLENGE_RESPONSE)) == NULL,
1333+ NULL);
1334+}
1335+END_TEST
1336+
1337+START_TEST(test_midauth_handle_challenge_request_param_1_CORRECT)
1338+{
1339+ const uint8_t opaque1[] = "\x01\x41\x01\x14\x05\x00\x48\x49\x0b"
1340+ "\x02\x42\x02\x15\x06\x08\x49\x50\x0c";
1341+ struct hip_packet_context ctx = { 0 };
1342+ const struct hip_challenge_response *response = NULL;
1343+
1344+ ctx.input_msg = hip_msg_alloc();
1345+ ctx.output_msg = hip_msg_alloc();
1346+
1347+ fail_unless(hip_build_param_challenge_request(ctx.input_msg, 0, 0, opaque1,
1348+ MIDAUTH_DEFAULT_NONCE_LENGTH) ==
1349+ 0, NULL);
1350+ fail_unless(handle_challenge_request_param(0, 0, &ctx) == 0, NULL);
1351+
1352+ /* NOTE we can only check for the existance of the response parameter here
1353+ * as the puzzle solution differs for each run. */
1354+ fail_unless((response = hip_get_param(ctx.output_msg,
1355+ HIP_PARAM_CHALLENGE_RESPONSE)) != NULL,
1356+ NULL);
1357+}
1358+END_TEST
1359+
1360+START_TEST(test_midauth_handle_challenge_request_param_2_CORRECT)
1361+{
1362+ const uint8_t opaque1[] = "\x01\x41\x01\x14\x05\x00\x48\x49\x0b"
1363+ "\x02\x42\x02\x15\x06\x08\x49\x50\x0c";
1364+ const uint8_t opaque2[] = "\x01\x41\x01\x14\x05\x00\x48\x49\x0b"
1365+ "\x02\x42\x02\x15\x06\x08\x49\x50\x0d";
1366+ struct hip_packet_context ctx = { 0 };
1367+ const struct hip_challenge_response *response = NULL;
1368+
1369+ ctx.input_msg = hip_msg_alloc();
1370+ ctx.output_msg = hip_msg_alloc();
1371+
1372+ fail_unless(hip_build_param_challenge_request(ctx.input_msg, 0, 0, opaque1,
1373+ MIDAUTH_DEFAULT_NONCE_LENGTH) ==
1374+ 0, NULL);
1375+ fail_unless(hip_build_param_challenge_request(ctx.input_msg, 0, 0, opaque2,
1376+ MIDAUTH_DEFAULT_NONCE_LENGTH) ==
1377+ 0, NULL);
1378+ fail_unless(handle_challenge_request_param(0, 0, &ctx) == 0, NULL);
1379+
1380+ /* NOTE we can only check for the existance of the response parameter here
1381+ * as the puzzle solution differs for each run. */
1382+ fail_unless((response = hip_get_param(ctx.output_msg,
1383+ HIP_PARAM_CHALLENGE_RESPONSE)) != NULL,
1384+ NULL);
1385+ fail_unless(hip_get_next_param(ctx.output_msg,
1386+ (const struct hip_tlv_common *) response) !=
1387+ NULL, NULL);
1388+}
1389+END_TEST
1390+
1391+Suite *modules_midauth_hipd_midauth(void)
1392+{
1393+ Suite *s = suite_create("modules/midauth/hipd/midauth");
1394+ TCase *tc_midauth = tcase_create("Midauth");
1395+
1396+ tcase_add_test(tc_midauth, test_midauth_handle_challenge_request_param_0_CORRECT);
1397+ tcase_add_test(tc_midauth, test_midauth_handle_challenge_request_param_1_CORRECT);
1398+ tcase_add_test(tc_midauth, test_midauth_handle_challenge_request_param_2_CORRECT);
1399+
1400+ suite_add_tcase(s, tc_midauth);
1401+
1402+ return s;
1403+}
1404
1405=== added directory 'test/modules/midauth/lib'
1406=== added file 'test/modules/midauth/lib/midauth_builder.c'
1407--- test/modules/midauth/lib/midauth_builder.c 1970-01-01 00:00:00 +0000
1408+++ test/modules/midauth/lib/midauth_builder.c 2011-08-09 11:19:24 +0000
1409@@ -0,0 +1,265 @@
1410+/*
1411+ * Copyright (c) 2011 Aalto University and RWTH Aachen University.
1412+ *
1413+ * Permission is hereby granted, free of charge, to any person
1414+ * obtaining a copy of this software and associated documentation
1415+ * files (the "Software"), to deal in the Software without
1416+ * restriction, including without limitation the rights to use,
1417+ * copy, modify, merge, publish, distribute, sublicense, and/or sell
1418+ * copies of the Software, and to permit persons to whom the
1419+ * Software is furnished to do so, subject to the following
1420+ * conditions:
1421+ *
1422+ * The above copyright notice and this permission notice shall be
1423+ * included in all copies or substantial portions of the Software.
1424+ *
1425+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
1426+ * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES
1427+ * OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
1428+ * NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT
1429+ * HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY,
1430+ * WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
1431+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
1432+ * OTHER DEALINGS IN THE SOFTWARE.
1433+ */
1434+
1435+#include <check.h>
1436+#include <stdint.h>
1437+
1438+#include "lib/core/protodefs.h"
1439+#include "modules/midauth/hipd/midauth.h"
1440+#include "modules/midauth/lib/midauth_builder.h"
1441+#include "../test_suites.h"
1442+
1443+#define MIDAUTH_DEFAULT_NONCE_LENGTH 18
1444+
1445+START_TEST(test_midauth_builder_build_param_challenge_request_NULL_msg)
1446+{
1447+ const uint8_t opaque[] = "\x01\x41\x01\x14\x05\x00\x48\x49\x0b"
1448+ "\x02\x42\x02\x15\x06\x08\x49\x50\x0c";
1449+
1450+ fail_unless(hip_build_param_challenge_request(NULL, 0, 0, opaque,
1451+ MIDAUTH_DEFAULT_NONCE_LENGTH) ==
1452+ -1, NULL);
1453+}
1454+END_TEST
1455+
1456+#ifdef HAVE_TCASE_ADD_EXIT_TEST
1457+START_TEST(test_midauth_builder_build_param_challenge_request_NULL_opaque)
1458+{
1459+ struct hip_common *const msg = hip_msg_alloc();
1460+
1461+ hip_build_param_challenge_request(msg, 0, 0, NULL,
1462+ MIDAUTH_DEFAULT_NONCE_LENGTH);
1463+}
1464+END_TEST
1465+#endif /* HAVE_TCASE_ADD_EXIT_TEST */
1466+
1467+START_TEST(test_midauth_builder_build_param_challenge_request_0_opaque_len)
1468+{
1469+ const uint8_t opaque[] = "\x01\x41\x01\x14\x05\x00\x48\x49\x0b"
1470+ "\x02\x42\x02\x15\x06\x08\x49\x50\x0c";
1471+ struct hip_common *const msg = hip_msg_alloc();
1472+
1473+ fail_unless(hip_build_param_challenge_request(msg, 0, 0, opaque,
1474+ 0) == 0, NULL);
1475+}
1476+END_TEST
1477+
1478+START_TEST(test_midauth_builder_build_param_challenge_request_CORRECT)
1479+{
1480+ const uint8_t opaque[] = "\x01\x41\x01\x14\x05\x00\x48\x49\x0b"
1481+ "\x02\x42\x02\x15\x06\x08\x49\x50\x0c";
1482+ const char cmp_challenge_request[] = "\xff\x36\x00\x14\x00\x00\x01\x41\x01"
1483+ "\x14\x05\x00\x48\x49\x0b\x02\x42\x02"
1484+ "\x15\x06\x08\x49\x50\x0C";
1485+ const struct hip_challenge_request *request = NULL;
1486+ struct hip_common *const msg = hip_msg_alloc();
1487+
1488+ fail_unless(hip_build_param_challenge_request(msg, 0, 0, opaque,
1489+ MIDAUTH_DEFAULT_NONCE_LENGTH) ==
1490+ 0, NULL);
1491+ fail_unless((request = hip_get_param(msg, HIP_PARAM_CHALLENGE_REQUEST)) != NULL,
1492+ NULL);
1493+ fail_unless(memcmp(request, &cmp_challenge_request,
1494+ hip_get_param_total_len(request)) == 0, NULL);
1495+}
1496+END_TEST
1497+
1498+START_TEST(test_midauth_builder_build_param_challenge_response_NULL_msg)
1499+{
1500+ const char challenge_request[] = "\xff\x36\x00\x14\x00\x00\x01\x41\x01"
1501+ "\x14\x05\x00\x48\x49\x0b\x02\x42\x02"
1502+ "\x15\x06\x08\x49\x50\x0C";
1503+ const uint8_t solution[] = "\x01\x41\x01\x14\x05\x00\x48\x49";
1504+ const struct hip_challenge_request *request =
1505+ (const struct hip_challenge_request *) challenge_request;
1506+
1507+ fail_unless(hip_build_param_challenge_response(NULL, request, solution) == -1,
1508+ NULL);
1509+}
1510+END_TEST
1511+
1512+START_TEST(test_midauth_builder_build_param_challenge_response_NULL_request)
1513+{
1514+ const uint8_t solution[] = "\x01\x41\x01\x14\x05\x00\x48\x49";
1515+ const struct hip_challenge_request *request = NULL;
1516+ struct hip_common *const msg = hip_msg_alloc();
1517+
1518+ fail_unless(hip_build_param_challenge_response(msg, request, solution) == -1,
1519+ NULL);
1520+}
1521+END_TEST
1522+
1523+START_TEST(test_midauth_builder_build_param_challenge_response_NULL_solution)
1524+{
1525+ const char challenge_request[] = "\xff\x36\x00\x14\x00\x00\x01\x41\x01"
1526+ "\x14\x05\x00\x48\x49\x0b\x02\x42\x02"
1527+ "\x15\x06\x08\x49\x50\x0C";
1528+ const struct hip_challenge_request *request =
1529+ (const struct hip_challenge_request *) challenge_request;
1530+ struct hip_common *const msg = hip_msg_alloc();
1531+
1532+ fail_unless(hip_build_param_challenge_response(msg, request, NULL) == -1,
1533+ NULL);
1534+}
1535+END_TEST
1536+
1537+START_TEST(test_midauth_builder_build_param_challenge_response_CORRECT)
1538+{
1539+ const char challenge_request[] = "\xff\x36\x00\x14\x00\x00\x01\x41\x01"
1540+ "\x14\x05\x00\x48\x49\x0b\x02\x42\x02"
1541+ "\x15\x06\x08\x49\x50\x0C";
1542+ const char cmp_challenge_response[] = "\x01\x42\x00\x1C\x00\x00\x01\x41\x01"
1543+ "\x14\x05\x00\x48\x49\x01\x41\x01\x14"
1544+ "\x05\x00\x48\x49\x0b\x02\x42\x02\x15"
1545+ "\x06\x08\x49\x50\x0c";
1546+ const uint8_t solution[] = "\x01\x41\x01\x14\x05\x00\x48\x49";
1547+ const struct hip_challenge_request *request =
1548+ (const struct hip_challenge_request *) challenge_request;
1549+ const struct hip_challenge_response *response = NULL;
1550+ struct hip_common *const msg = hip_msg_alloc();
1551+
1552+ fail_unless(hip_build_param_challenge_response(msg, request, solution) == 0,
1553+ NULL);
1554+ fail_unless((response = hip_get_param(msg, HIP_PARAM_CHALLENGE_RESPONSE)) != NULL,
1555+ NULL);
1556+ fail_unless(memcmp(response, &cmp_challenge_response,
1557+ hip_get_param_total_len(response)) == 0, NULL);
1558+}
1559+END_TEST
1560+
1561+START_TEST(test_midauth_builder_challenge_request_opaque_len_NULL)
1562+{
1563+ fail_unless(hip_challenge_request_opaque_len(NULL) == 0, NULL);
1564+}
1565+END_TEST
1566+
1567+START_TEST(test_midauth_builder_challenge_request_opaque_len_CORRECT)
1568+{
1569+ const uint8_t opaque[] = "\x01\x41\x01\x14\x05\x00\x48\x49\x0b"
1570+ "\x02\x42\x02\x15\x06\x08\x49\x50\x0c";
1571+ const struct hip_challenge_request *request = NULL;
1572+ struct hip_common *const msg = hip_msg_alloc();
1573+
1574+ fail_unless(hip_build_param_challenge_request(msg, 0, 0, opaque,
1575+ MIDAUTH_DEFAULT_NONCE_LENGTH) ==
1576+ 0, NULL);
1577+ fail_unless((request = hip_get_param(msg, HIP_PARAM_CHALLENGE_REQUEST)) != NULL,
1578+ NULL);
1579+ fail_unless(hip_challenge_request_opaque_len(request) ==
1580+ MIDAUTH_DEFAULT_NONCE_LENGTH, NULL);
1581+}
1582+END_TEST
1583+
1584+#ifdef HAVE_TCASE_ADD_EXIT_TEST
1585+START_TEST(test_midauth_builder_puzzle_seed_NULL_opaque)
1586+{
1587+ const uint8_t *const opaque = NULL;
1588+ uint8_t puzzle_value[PUZZLE_LENGTH];
1589+
1590+ hip_midauth_puzzle_seed(opaque, MIDAUTH_DEFAULT_NONCE_LENGTH, puzzle_value);
1591+}
1592+END_TEST
1593+
1594+START_TEST(test_midauth_builder_puzzle_seed_NULL_puzzle_value)
1595+{
1596+ const uint8_t opaque[] = "\x01\x41\x00\x14\x05\x00\x48\x49\x0b";
1597+
1598+ hip_midauth_puzzle_seed(opaque,
1599+ MIDAUTH_DEFAULT_NONCE_LENGTH,
1600+ NULL);
1601+}
1602+END_TEST
1603+#endif /* HAVE_TCASE_ADD_EXIT_TEST */
1604+
1605+START_TEST(test_midauth_builder_puzzle_seed_0_opaque_len)
1606+{
1607+ const uint8_t opaque[] = "\x01\x41\x00\x14\x05\x00\x48\x49\x0b";
1608+ uint8_t puzzle_value[PUZZLE_LENGTH];
1609+
1610+ fail_unless(hip_midauth_puzzle_seed(opaque,
1611+ 0,
1612+ puzzle_value) == -1, NULL);
1613+}
1614+END_TEST
1615+
1616+START_TEST(test_midauth_builder_puzzle_seed_CORRECT)
1617+{
1618+ const uint8_t opaque[] = "\x01\x41\x01\x14\x05\x00\x48\x49\x0b"
1619+ "\x02\x42\x02\x15\x06\x08\x49\x50\x0c";
1620+ uint8_t puzzle_value[PUZZLE_LENGTH];
1621+ uint8_t correct_puzzle_value[] = "\x43\x03\x8F\xD7\x2F\xC7\xD2\xF3";
1622+
1623+ fail_unless(hip_midauth_puzzle_seed(opaque,
1624+ MIDAUTH_DEFAULT_NONCE_LENGTH,
1625+ puzzle_value) == 0, NULL);
1626+ fail_unless(memcmp(correct_puzzle_value, puzzle_value, PUZZLE_LENGTH) == 0,
1627+ NULL);
1628+}
1629+END_TEST
1630+
1631+Suite *modules_midauth_lib_builder(void)
1632+{
1633+ Suite *s = suite_create("modules/midauth/lib/midauth_builder");
1634+ TCase *tc_midauth_builder = tcase_create("Midauth_builder");
1635+
1636+ tcase_add_test(tc_midauth_builder,
1637+ test_midauth_builder_build_param_challenge_request_NULL_msg);
1638+// the tcase_add_exit_test macro is only available in check 0.9.8 or later but
1639+// scratchbox uses an older version of checkc so we try to avoid this macro
1640+#ifdef HAVE_TCASE_ADD_EXIT_TEST
1641+ tcase_add_exit_test(tc_midauth_builder,
1642+ test_midauth_builder_build_param_challenge_request_NULL_opaque,
1643+ 1);
1644+#endif
1645+ tcase_add_test(tc_midauth_builder,
1646+ test_midauth_builder_build_param_challenge_request_0_opaque_len);
1647+ tcase_add_test(tc_midauth_builder,
1648+ test_midauth_builder_build_param_challenge_request_CORRECT);
1649+ tcase_add_test(tc_midauth_builder,
1650+ test_midauth_builder_build_param_challenge_response_NULL_msg);
1651+ tcase_add_test(tc_midauth_builder,
1652+ test_midauth_builder_build_param_challenge_response_NULL_request);
1653+ tcase_add_test(tc_midauth_builder,
1654+ test_midauth_builder_build_param_challenge_response_NULL_solution);
1655+ tcase_add_test(tc_midauth_builder,
1656+ test_midauth_builder_build_param_challenge_response_CORRECT);
1657+ tcase_add_test(tc_midauth_builder,
1658+ test_midauth_builder_challenge_request_opaque_len_NULL);
1659+ tcase_add_test(tc_midauth_builder,
1660+ test_midauth_builder_challenge_request_opaque_len_CORRECT);
1661+
1662+#ifdef HAVE_TCASE_ADD_EXIT_TEST
1663+ tcase_add_exit_test(tc_midauth_builder,
1664+ test_midauth_builder_puzzle_seed_NULL_opaque, 1);
1665+ tcase_add_exit_test(tc_midauth_builder,
1666+ test_midauth_builder_puzzle_seed_NULL_puzzle_value, 1);
1667+#endif
1668+ tcase_add_test(tc_midauth_builder, test_midauth_builder_puzzle_seed_0_opaque_len);
1669+ tcase_add_test(tc_midauth_builder, test_midauth_builder_puzzle_seed_CORRECT);
1670+
1671+ suite_add_tcase(s, tc_midauth_builder);
1672+
1673+ return s;
1674+}
1675
1676=== added file 'test/modules/midauth/test_suites.h'
1677--- test/modules/midauth/test_suites.h 1970-01-01 00:00:00 +0000
1678+++ test/modules/midauth/test_suites.h 2011-08-09 11:19:24 +0000
1679@@ -0,0 +1,34 @@
1680+/*
1681+ * Copyright (c) 2010 Aalto University and RWTH Aachen University.
1682+ *
1683+ * Permission is hereby granted, free of charge, to any person
1684+ * obtaining a copy of this software and associated documentation
1685+ * files (the "Software"), to deal in the Software without
1686+ * restriction, including without limitation the rights to use,
1687+ * copy, modify, merge, publish, distribute, sublicense, and/or sell
1688+ * copies of the Software, and to permit persons to whom the
1689+ * Software is furnished to do so, subject to the following
1690+ * conditions:
1691+ *
1692+ * The above copyright notice and this permission notice shall be
1693+ * included in all copies or substantial portions of the Software.
1694+ *
1695+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
1696+ * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES
1697+ * OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
1698+ * NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT
1699+ * HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY,
1700+ * WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
1701+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
1702+ * OTHER DEALINGS IN THE SOFTWARE.
1703+ */
1704+
1705+#ifndef HIP_TEST_MODULES_MIDAUTH_TEST_SUITES_H
1706+#define HIP_TEST_MODULES_MIDAUTH_TEST_SUITES_H
1707+
1708+#include <check.h>
1709+
1710+Suite *modules_midauth_lib_builder(void);
1711+Suite *modules_midauth_hipd_midauth(void);
1712+
1713+#endif /* HIP_TEST_MODULES_MIDAUTH_TEST_SUITES_H */

Subscribers

People subscribed via source and target branches

to all changes: