Am 10.02.20 um 18:59 schrieb Mateusz Markowicz via Openvpn-devel: > when using "--verify-x509-name [hostname] name" hostname will now be > accepted > also when matched against one of the X509v3 Subject Alternative Name IP > or DNS > entries (instead of just Subject's CN). > > see also: https://github.com/OpenVPN/openvpn/pull/136/ > > Signed-off-by: Mateusz Markowicz <ponie...@protonmail.com > <mailto:ponie...@protonmail.com>>
If this should have a chance of being included it needs to cover mbed
TLS as well.
Also documentation in the man page is missing.
> ---
> src/openvpn/options.c | 4 +++
> src/openvpn/ssl_verify.c | 18 +++++++++++---
> src/openvpn/ssl_verify.h | 1 +
> src/openvpn/ssl_verify_backend.h | 7 ++++++
> src/openvpn/ssl_verify_mbedtls.c | 11 +++++++++
> src/openvpn/ssl_verify_openssl.c | 42 ++++++++++++++++++++++++++++++++
> 6 files changed, 80 insertions(+), 3 deletions(-)
>
> diff --git a/src/openvpn/options.c b/src/openvpn/options.c
> index 173a1eea..438dfff0 100644
> --- a/src/openvpn/options.c
> +++ b/src/openvpn/options.c
> @@ -8144,6 +8144,10 @@ add_option(struct options *options,
> {
> type = VERIFY_X509_SUBJECT_RDN_PREFIX;
> }
> + else if (streq(p[2], "subject-alt-name"))
> + {
> + type = VERIFY_X509_SAN;
> + }
> else
> {
> msg(msglevel, "unknown X.509 name type: %s", p[2]);
> diff --git a/src/openvpn/ssl_verify.c b/src/openvpn/ssl_verify.c
> index 65188d23..6480b5eb 100644
> --- a/src/openvpn/ssl_verify.c
> +++ b/src/openvpn/ssl_verify.c
> @@ -390,15 +390,27 @@ verify_peer_cert(const struct tls_options *opt,
> openvpn_x509_cert_t *peer_cert,
> /* verify X509 name or username against --verify-x509-[user]name */
> if (opt->verify_x509_type != VERIFY_X509_NONE)
> {
> - if ( (opt->verify_x509_type == VERIFY_X509_SUBJECT_DN
> + bool match;
> + if (opt->verify_x509_type == VERIFY_X509_SAN)
> + {
> + bool have_alt_names;
> + match = x509v3_is_host_in_alternative_names(peer_cert,
> opt->verify_x509_name, &have_alt_names)
> + || (!have_alt_names &&
> strcmp(opt->verify_x509_name, common_name) == 0);
I know that this technically correct C but setting a variable in the
first part of via & and then using it in the second part feels like not
good style. I would rather like too a bit more verbose and less clever
code that is easier to understand.
> + }
> + else
> + {
> + match = (opt->verify_x509_type == VERIFY_X509_SUBJECT_DN
> && strcmp(opt->verify_x509_name, subject) == 0)
> || (opt->verify_x509_type == VERIFY_X509_SUBJECT_RDN
> && strcmp(opt->verify_x509_name, common_name) == 0)
> || (opt->verify_x509_type == VERIFY_X509_SUBJECT_RDN_PREFIX
> && strncmp(opt->verify_x509_name, common_name,
> - strlen(opt->verify_x509_name)) == 0) )
> + strlen(opt->verify_x509_name)) == 0);
> + }
> +
> + if (match)
> {
> - msg(D_HANDSHAKE, "VERIFY X509NAME OK: %s", subject);
> + msg(D_HANDSHAKE, "VERIFY X509NAME OK: %s",
> opt->verify_x509_name);
This changes the log message. If you want to verify that the cert prefix
with OVPN-client or something you don't get to see what certificate you
accepted anymore. If you want to log opt->verify_x509_name that needs to
be in addition.
> }
> else
> {
> diff --git a/src/openvpn/ssl_verify.h b/src/openvpn/ssl_verify.h
> index c54b89a6..1295e76b 100644
> --- a/src/openvpn/ssl_verify.h
> +++ b/src/openvpn/ssl_verify.h
> @@ -64,6 +64,7 @@ struct cert_hash_set {
> #define VERIFY_X509_SUBJECT_DN 1
> #define VERIFY_X509_SUBJECT_RDN 2
> #define VERIFY_X509_SUBJECT_RDN_PREFIX 3
> +#define VERIFY_X509_SAN 4
>
> #define TLS_AUTHENTICATION_SUCCEEDED 0
> #define TLS_AUTHENTICATION_FAILED 1
> diff --git a/src/openvpn/ssl_verify_backend.h
> b/src/openvpn/ssl_verify_backend.h
> index d6b31bfa..927a5a29 100644
> --- a/src/openvpn/ssl_verify_backend.h
> +++ b/src/openvpn/ssl_verify_backend.h
> @@ -268,4 +268,11 @@ result_t x509_write_pem(FILE *peercert_file,
> openvpn_x509_cert_t *peercert);
> */
> bool tls_verify_crl_missing(const struct tls_options *opt);
>
> +/**
> + * Return true iff {host} was found in {cert} Subject Alternative Names
> DNS or IP entries.
> + * If {has_alt_names} != NULL it'll return true iff Subject Alternative
> Names were defined
> + * for {cert}.
> + */
> +bool x509v3_is_host_in_alternative_names(openvpn_x509_cert_t *cert,
> const char *host, bool *has_alt_names);
> +
> #endif /* SSL_VERIFY_BACKEND_H_ */
> diff --git a/src/openvpn/ssl_verify_mbedtls.c
> b/src/openvpn/ssl_verify_mbedtls.c
> index fd31bbbd..2f2e04be 100644
> --- a/src/openvpn/ssl_verify_mbedtls.c
> +++ b/src/openvpn/ssl_verify_mbedtls.c
> @@ -245,6 +245,17 @@ x509_get_subject(mbedtls_x509_crt *cert, struct
> gc_arena *gc)
> return subject;
> }
>
> +bool
> +x509v3_is_host_in_alternative_names(mbedtls_x509_crt *cert, const char
> *host, bool *has_alt_names)
> +{
> + msg(M_WARN, "Missing support for subject alternative names in
> mbedtls.");
> + if (has_alt_names != NULL)
> + {
> + *has_alt_names = false;
> + }
> + return false;
> +}
*has_alt_names = (has_alt_names != NULL);
signature.asc
Description: OpenPGP digital signature
_______________________________________________ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
