minor changes to isakmpd(8)

classic Classic list List threaded Threaded
5 messages Options
Reply | Threaded
Open this post in threaded view
|

minor changes to isakmpd(8)

Sevan Janiyan-3
Use an It tag to label the additional steps referred to at the beginning
of step 2:
"This step, as well as the next one, needs to be done for every peer.
Furthermore the last step will need to be done once for each ID you want
the peer to have"

Change the file name passed to openssl for extfile to indicate cloned
file is used.

For FQDN certificates use change the CERTFQDN field.

Add instructions on how to check the subjectAltName field of a certificate.

s/fashion/convention.


Sevan

patch-openbsd-isakmpd8.txt (1K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: minor changes to isakmpd(8)

Jason McIntyre-2
On Sat, Feb 16, 2019 at 02:35:08AM +0000, Sevan Janiyan wrote:

> Use an It tag to label the additional steps referred to at the beginning
> of step 2:
> "This step, as well as the next one, needs to be done for every peer.
> Furthermore the last step will need to be done once for each ID you want
> the peer to have"
>
> Change the file name passed to openssl for extfile to indicate cloned
> file is used.
>
> For FQDN certificates use change the CERTFQDN field.
>
> Add instructions on how to check the subjectAltName field of a certificate.
>
> s/fashion/convention.
>
>
> Sevan

morning.

if you agree with my comments below, could you mail us an updated diff
and i'll try to prod some folks for oks.

thanks,
jmc

> Index: sbin/isakmpd/isakmpd.8
> ===================================================================
> RCS file: /cvs/src/sbin/isakmpd/isakmpd.8,v
> retrieving revision 1.120
> diff -u -p -r1.120 isakmpd.8
> --- sbin/isakmpd/isakmpd.8 17 Apr 2018 12:13:29 -0000 1.120
> +++ sbin/isakmpd/isakmpd.8 16 Feb 2019 02:27:13 -0000
> @@ -607,6 +607,8 @@ Encoding the ID in the common name is re
>  # openssl req -new -key /etc/isakmpd/private/local.key \e
>   -out /etc/isakmpd/private/10.0.0.1.csr
>  .Ed
> +.It
> +Generate signed certificates from Certificate Signing Requests (CSRs)

adding an extra list item makes the description of step 2 a little
redundant. what about just making this change:

        This step, as well as the next one, needs to be done...
to
        These steps need to be done...

>  .Pp
>  Now take these certificate signing requests to your CA and process
>  them as below.
> @@ -626,12 +628,12 @@ with 10.0.0.1, then run:
>  # openssl x509 -req \e
>   -days 365 -in 10.0.0.1.csr \e
>   -CA /etc/ssl/ca.crt -CAkey /etc/ssl/private/ca.key \e
> - -CAcreateserial -extfile /etc/ssl/x509v3.cnf \e
> + -CAcreateserial -extfile ~/tmp_x509v3.cnf \e

i agree with this

>   -extensions x509v3_IPAddr -out 10.0.0.1.crt
>  .Ed
>  .Pp
>  For a FQDN certificate, replace
> -.Dv $ENV::CERTIP
> +.Dv $ENV::CERTFQDN

i am hazy here. can you elaborate a little why this change is needed?

>  with the hostname and run:
>  .Bd -literal -offset indent
>  # openssl x509 -req \e
> @@ -651,6 +653,18 @@ in
>  A similar setup will be required if
>  .Xr isakmpd.conf 5
>  is being used instead.
> +To verify the
> +.Va subjectAltName
> +of the certificate matches the
> +.Ic srcid
> +referenced in
> +.Xr ipsec.conf 5
> +use:
> +.Bd -literal -offset indent
> +# openssl x509 -noout -text -in somehost.somedomain.crt
> +.Ed
> +.It

i think we could replace that entire blurb with:

        ...
        in
        .Xr ipsec.conf 5 :
        .Pp
        .Dl # openssl x509 ...
        .Pp
        A similar setup ...

> +Copy certificates into place

again, no so keen on adding more .It

>  .Pp
>  Put the certificate (the file ending in .crt) in
>  .Pa /etc/isakmpd/certs/
> @@ -721,7 +735,7 @@ has the same mode requirements as
>  .Pa isakmpd.conf .
>  .It Pa /etc/isakmpd/pubkeys/
>  The directory in which trusted public keys are kept.
> -The keys must be named in the fashion described above.
> +The keys must be named in the convention described above.

this is definitely not an improvement.

>  .It Pa /var/run/isakmpd.fifo
>  The FIFO used to manually control
>  .Nm isakmpd .

Reply | Threaded
Open this post in threaded view
|

Re: minor changes to isakmpd(8)

Sevan Janiyan-3
Hi Jason,

On 18/02/2019 07:23, Jason McIntyre wrote:
> if you agree with my comments below, could you mail us an updated diff
> and i'll try to prod some folks for oks.

Thanks for taking a look. I've dropped the changes with the exception of
s/CERTIP/CERTFQDN that is an actual bug and changing the file name to
indicate a copy.

If you look at /etc/x509v3.cnf you'll see that for the x509v3_FQDN
extension, the subjectAltName field is populated using $ENV::CERTFQDN,
not $ENV::CERTIP


Sevan

patch-openbsd-isakmpd8.txt (832 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: minor changes to isakmpd(8)

Sevan Janiyan-3


On 18/02/2019 14:35, Sevan Janiyan wrote:
> I've dropped the changes with the exception of
> s/CERTIP/CERTFQDN that is an actual bug and changing the file name to
> indicate a copy.
>
> If you look at /etc/x509v3.cnf you'll see that for the x509v3_FQDN
> extension, the subjectAltName field is populated using $ENV::CERTFQDN,
> not $ENV::CERTIP

Sorry about the noise, updated diff.


Sevan

patch-openbsd-isakmpd8.txt (886 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: minor changes to isakmpd(8)

Stuart Henderson
On 2019/02/18 14:47, Sevan Janiyan wrote:

>
>
> On 18/02/2019 14:35, Sevan Janiyan wrote:
> > I've dropped the changes with the exception of
> > s/CERTIP/CERTFQDN that is an actual bug and changing the file name to
> > indicate a copy.
> >
> > If you look at /etc/x509v3.cnf you'll see that for the x509v3_FQDN
> > extension, the subjectAltName field is populated using $ENV::CERTFQDN,
> > not $ENV::CERTIP
>
> Sorry about the noise, updated diff.
>
>
> Sevan

> Index: sbin/isakmpd/isakmpd.8
> ===================================================================
> RCS file: /cvs/src/sbin/isakmpd/isakmpd.8,v
> retrieving revision 1.120
> diff -u -p -r1.120 isakmpd.8
> --- sbin/isakmpd/isakmpd.8 17 Apr 2018 12:13:29 -0000 1.120
> +++ sbin/isakmpd/isakmpd.8 18 Feb 2019 14:45:19 -0000
> @@ -630,14 +630,14 @@ with 10.0.0.1, then run:
>   -extensions x509v3_IPAddr -out 10.0.0.1.crt
>  .Ed
>  .Pp
> -For a FQDN certificate, replace
> -.Dv $ENV::CERTIP
> +For an FQDN certificate, replace
> +.Dv $ENV::CERTFQDN

OK for this as-is, that's an actual manpage bug that needs fixing.

>  with the hostname and run:
>  .Bd -literal -offset indent
>  # openssl x509 -req \e
>   -days 365 -in somehost.somedomain.csr \e
>   -CA /etc/ssl/ca.crt -CAkey /etc/ssl/private/ca.key \e
> - -CAcreateserial -extfile /etc/ssl/x509v3.cnf \e
> + -CAcreateserial -extfile ~/tmp_x509v3.cnf \e
>   -extensions x509v3_FQDN -out somehost.somedomain.crt
>  .Ed
>  .Pp

If we're changing that /etc/ssl/x509v3.cnf reference then we should
also change the one for x509v3_IP. But the text above just says to copy
and edit it but without mentioning ~/tmp_x509v3.cnf anywhere other than
in the command line it seems a bit unnecessarily awkward for the reader.

Maybe these for the two sections?

# sed 's,\$ENV::CERTIP,10.0.0.1,' < /etc/ssl/x509v3.cnf > ~/tmp_x509v3.cnf
# openssl x509 -req [...]

and

# sed 's,\$ENV::CERTFQDN,somehost.somedomain,' < /etc/ssl/x509v3.cnf > ~/tmp_x509v3.cnf
# openssl x509 -req [...]