user/5391: ipsecctl generates conflicting section names; patch

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
1 message Options
Reply | Threaded
Open this post in threaded view
|

user/5391: ipsecctl generates conflicting section names; patch

sthen
>Number:         5391
>Category:       user
>Synopsis:       ipsecctl generates conflicting section names; patch
>Confidential:   yes
>Severity:       non-critical
>Priority:       medium
>Responsible:    bugs
>State:          open
>Quarter:        
>Keywords:      
>Date-Required:
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Fri Feb 23 12:00:01 GMT 2007
>Closed-Date:
>Last-Modified:
>Originator:     sthen
>Release:        -current
>Organization:
net
>Environment:
       
        System      : OpenBSD 4.1
        Architecture: OpenBSD.i386
        Machine     : i386
>Description:
       
        If multiple ipsec.conf rules are listed without a recognisable peer
        address, multiple isakmpd configuration sections with the same name are
        generated (peer-default, default-ID, and so on).

>How-To-Repeat:
       

$ ipsecctl -vnf - << EOF
# @0
ike passive esp from 10.1.10.0/24 to 10.1.44.100/24 \
        quick auth hmac-sha1 enc aes group grp2 \
        srcid me.mylan.net dstid net.100 psk 22222

# @1
ike passive esp from 10.1.10.0/24 to 10.1.44.101/24 \
        quick auth hmac-sha1 enc aes group grp2 \
        srcid me.mylan.net dstid net.101 psk 33333
EOF

the following peer config sections (and others) are generated:

@0 C set [Phase 1]:Default=peer-default force
C set [peer-default]:Phase=1 force
C set [peer-default]:Authentication=22222 force
C set [peer-default]:Configuration=mm-default force
C set [peer-default]:ID=me.mylan.net-ID force
C set [peer-default]:Remote-ID=default-ID force
C set [default-ID]:ID-type=FQDN force
C set [default-ID]:Name=net.100 force

@1 C set [Phase 1]:Default=peer-default force
C set [peer-default]:Phase=1 force
C set [peer-default]:Authentication=33333 force
C set [peer-default]:Configuration=mm-default force
C set [peer-default]:ID=me.mylan.net-ID force
C set [peer-default]:Remote-ID=default-ID force
C set [default-ID]:ID-type=FQDN force
C set [default-ID]:Name=net.101 force

>Fix:
       

I think this is a reasonable way to handle this; regression tests
ike56 and ikefail7 fail, but they were already failing with the in-tree
code. Comments would be very welcome.

Index: sbin/ipsecctl/ike.c
===================================================================
RCS file: /cvs/src/sbin/ipsecctl/ike.c,v
retrieving revision 1.60
diff -u -p -u -r1.60 ike.c
--- sbin/ipsecctl/ike.c 19 Feb 2007 09:00:46 -0000 1.60
+++ sbin/ipsecctl/ike.c 23 Feb 2007 11:42:29 -0000
@@ -80,35 +80,32 @@ ike_section_general(struct ipsec_rule *r
 static void
 ike_section_peer(struct ipsec_rule *r, FILE *fd)
 {
- if (r->peer) {
- fprintf(fd, SET "[Phase 1]:%s=peer-%s force\n", r->peer->name,
-    r->peer->name);
- fprintf(fd, SET "[peer-%s]:Phase=1 force\n", r->peer->name);
- fprintf(fd, SET "[peer-%s]:Address=%s force\n", r->peer->name,
+ char *peername = "default";
+
+ if (r->peer)
+ peername=r->peer->name;
+ else if (r->auth->dstid)
+ peername=r->auth->dstid;
+
+ fprintf(fd, SET "[Phase 1]:%s=peer-%s force\n",
+    peername == "default" ? "Default" : peername, peername);
+
+ fprintf(fd, SET "[peer-%s]:Phase=1 force\n", peername);
+ if (r->peer)
+ fprintf(fd, SET "[peer-%s]:Address=%s force\n", peername,
     r->peer->name);
- if (r->local)
- fprintf(fd, SET "[peer-%s]:Local-address=%s force\n",
-    r->peer->name, r->local->name);
- if (r->ikeauth->type == IKE_AUTH_PSK)
- fprintf(fd, SET "[peer-%s]:Authentication=%s force\n",
-    r->peer->name, r->ikeauth->string);
- } else {
- fprintf(fd, SET "[Phase 1]:Default=peer-default force\n");
- fprintf(fd, SET "[peer-default]:Phase=1 force\n");
- if (r->local)
- fprintf(fd, SET
-    "[peer-default]:Local-address=%s force\n",
-    r->local->name);
- if (r->ikeauth->type == IKE_AUTH_PSK)
- fprintf(fd, SET
-    "[peer-default]:Authentication=%s force\n",
-    r->ikeauth->string);
- }
+ if (r->local)
+ fprintf(fd, SET "[peer-%s]:Local-address=%s force\n",
+    peername, r->local->name);
+ if (r->ikeauth->type == IKE_AUTH_PSK)
+ fprintf(fd, SET "[peer-%s]:Authentication=%s force\n",
+    peername, r->ikeauth->string);
 }
 
 static void
 ike_section_ids(struct ipsec_rule *r, FILE *fd)
 {
+ char *peername = "default";
  char myname[MAXHOSTNAMELEN];
 
  if (r->auth == NULL)
@@ -120,15 +117,17 @@ ike_section_ids(struct ipsec_rule *r, FI
  if ((r->auth->srcid = strdup(myname)) == NULL)
  err(1, "ike_section_ids: strdup");
  }
+
+ if (r->peer)
+ peername=r->peer->name;
+ else if (r->auth->dstid)
+ peername=r->auth->dstid;
+
  if (r->auth->srcid) {
  int idtype = ike_get_id_type(r->auth->srcid);
 
- if (r->peer)
- fprintf(fd, SET "[peer-%s]:ID=%s-ID force\n",
-    r->peer->name, r->auth->srcid);
- else
- fprintf(fd, SET "[peer-default]:ID=%s-ID force\n",
-    r->auth->srcid);
+ fprintf(fd, SET "[peer-%s]:ID=%s-ID force\n",
+    peername, r->auth->srcid);
 
  fprintf(fd, SET "[%s-ID]:ID-type=%s force\n", r->auth->srcid,
     ike_id_types[idtype]);
@@ -138,21 +137,12 @@ ike_section_ids(struct ipsec_rule *r, FI
  if (r->auth->dstid) {
  int idtype = ike_get_id_type(r->auth->dstid);
 
- if (r->peer) {
- fprintf(fd, SET "[peer-%s]:Remote-ID=%s-ID force\n",
-    r->peer->name, r->peer->name);
- fprintf(fd, SET "[%s-ID]:ID-type=%s force\n",
-    r->peer->name, ike_id_types[idtype]);
- fprintf(fd, SET "[%s-ID]:Name=%s force\n", r->peer->name,
-    r->auth->dstid);
- } else {
- fprintf(fd, SET
-    "[peer-default]:Remote-ID=default-ID force\n");
- fprintf(fd, SET "[default-ID]:ID-type=%s force\n",
-    ike_id_types[idtype]);
- fprintf(fd, SET "[default-ID]:Name=%s force\n",
-    r->auth->dstid);
- }
+ fprintf(fd, SET "[peer-%s]:Remote-ID=%s-ID force\n",
+    peername, peername);
+ fprintf(fd, SET "[%s-ID]:ID-type=%s force\n",
+    peername, ike_id_types[idtype]);
+ fprintf(fd, SET "[%s-ID]:Name=%s force\n", peername,
+    r->auth->dstid);
  }
 }
 
@@ -168,15 +158,17 @@ ike_get_id_type(char *string)
 static void
 ike_section_ipsec(struct ipsec_rule *r, FILE *fd)
 {
- fprintf(fd, SET "[IPsec-%s]:Phase=2 force\n", r->p2name);
-
+ char *peername = "default";
+
  if (r->peer)
- fprintf(fd, SET "[IPsec-%s]:ISAKMP-peer=peer-%s force\n",
-    r->p2name, r->peer->name);
- else
- fprintf(fd, SET
-    "[IPsec-%s]:ISAKMP-peer=peer-default force\n", r->p2name);
+ peername=r->peer->name;
+ else if (r->auth->dstid)
+ peername=r->auth->dstid;
 
+ fprintf(fd, SET "[IPsec-%s]:Phase=2 force\n", r->p2name);
+
+ fprintf(fd, SET "[IPsec-%s]:ISAKMP-peer=peer-%s force\n", r->p2name,
+    peername);
  fprintf(fd, SET "[IPsec-%s]:Configuration=qm-%s force\n", r->p2name,
     r->p2name);
  fprintf(fd, SET "[IPsec-%s]:Local-ID=lid-%s force\n", r->p2name,
@@ -340,6 +332,12 @@ static int
 ike_section_p1(struct ipsec_rule *r, FILE *fd)
 {
  char *tag, *exchange_type;
+ char *peername = "default";
+
+ if (r->peer)
+ peername=r->peer->name;
+ else if (r->auth->dstid)
+ peername=r->auth->dstid;
 
  switch (r->p1ie) {
  case IKE_MM:
@@ -355,19 +353,11 @@ ike_section_p1(struct ipsec_rule *r, FIL
  return (-1);
  }
 
- if (r->peer) {
- fprintf(fd, SET "[peer-%s]:Configuration=%s-%s force\n",
-    r->peer->name, tag, r->peer->name);
- fprintf(fd, SET "[%s-%s]:EXCHANGE_TYPE=%s force\n",
-    tag, r->peer->name, exchange_type);
- fprintf(fd, ADD "[%s-%s]:Transforms=", tag, r->peer->name);
- } else {
- fprintf(fd, SET
-    "[peer-default]:Configuration=%s-default force\n", tag);
- fprintf(fd, SET "[%s-default]:EXCHANGE_TYPE=%s force\n",
-    tag, exchange_type);
- fprintf(fd, ADD "[%s-default]:Transforms=", tag);
- }
+ fprintf(fd, SET "[peer-%s]:Configuration=%s-%s force\n",
+    peername, tag, peername);
+ fprintf(fd, SET "[%s-%s]:EXCHANGE_TYPE=%s force\n",
+    tag, peername, exchange_type);
+ fprintf(fd, ADD "[%s-%s]:Transforms=", tag, peername);
 
  if (r->p1xfs && r->p1xfs->encxf) {
  switch (r->p1xfs->encxf->id) {

>Release-Note:
>Audit-Trail:
>Unformatted: