tech-net archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

Re: RFC: ipsec(4) pseudo interface


Thank you for your reviewing.

On 2017/12/20 21:08, Christos Zoulas wrote:
> In article <>,
> Kengo NAKAHARA  <> wrote:
>> Hi,
>> On 2017/12/19 2:54, Christos Zoulas wrote:
>>> In article <>,
>>> Kengo NAKAHARA  <> wrote:
>>>> Hi,
>>>> We implement ipsec(4) pseudo interface for route-based VPNs. This pseudo
>>>> interface manages its security policy(SP) by itself, in particular, we do
>>>>    # ifconfig ipsec0 tunnel
>>>> the SPs " ->"(out) and " ->"(in) are
>>>> generated automatically and atomically. And then, when we do
>>>>    # ifconfig ipsec0 deletetunnel
>>>> the SPs are destroyed automatically and atomically, too.
>>>> Here is the patches and an unified patch.
>>>> By the way, I have one question. In the above patch(s), I temporarily add
>>>> manual for ipsecX pseudo interface as if_ipsec.4, because there is already
>>>> ipsec.4 for general ipsec protocol. How should I add the man of ipsec(4)
>>>> pseudo interface?
>>>>    (a) Add if_ipsec.4
>>>>    (b) move current ipsec.4(for ipsec protocol) to ipsec.9, and then
>>>>        add ipsec.4(for ipsec pseudo interface)
>>>>    (c) any other
>>>> Could you comment the patch or the question?
>>> I've wanted this feature for a long time! Looks ok to me, but the
>>> sockaddr_copy()/port setting code, should be abstracted to a single
>>> function since it is repeated in ioctl().
>> Thank you for your reviewing. I fix it in the following patch.
>>    - patch series
>>      -
>>    - unified patch
>>      -
> Thanks:
> +	error = var->iv_output(var, family, m);
> +	if (!error) {
> - It is simpler to do (early returns):
> 	if (error)
> 		return error;
> 	...
> 	return 0;

I apply it.

> - What's the point of 'goto bad' in ioctl if there is no cleanup to be done?

Oh, indeed. I refactor cleanup processing.

> - There is one more place you could use the addr_port function?
>   [or perhaps abstract it in in_port_t *get_port(struct sockaddr *)?

Sorry, I missed it.

> - We should use in_port_t instead of unsigned short for ports.

Ah, the original code is quite old, I missed updating it.

> - I am not clear how the code interracts with ipsec endpoints created
>   from /etc/ipsec.conf and ones created via ifconfig? I.e. if I create
>   endpoints in /etc/ipsec.conf, does the cloner interface get automatically
>   constructed?

In a nutshell, it is first-come basis. If the SPs required by ipsec(4)
interface are already added by /etc/ipsec.conf or manually setkey(8),
'ifconfig ipsec0 tunnel "src" "dst"' fails, and vice versa.
Sorry, there was no information about that and SPs required by ipsec(4)
interface. I add the information to man.
Furthermore, I find a bug in handling that error case. Thank you for
your pointing out!

Here is the update patch series and unified patch.


Internet Initiative Japan Inc.

Device Engineering Section,
IoT Platform Development Department,
Network Division,
Technology Unit


Home | Main Index | Thread Index | Old Index