Clarify flags argument of X509_check_ip by tniessen · Pull Request #17536 · openssl/openssl

tniessen · 2022-01-17T15:27:51Z

Because no supported flag affects the behavior of X509_check_ip, the flags argument currently has no effect. Clarify this in the documentation to avoid confusion.

Checklist

documentation is added or updated

None of the supported options have any effect on X509_check_ip_asc. (iPAddress is not a typo, it is what RFC 5280 calls subject alternative names that are IP addresses.) Refs: openssl/openssl#17536

None of the supported options have any effect on X509_check_ip_asc. Refs: openssl/openssl#17536

paulidale · 2022-01-18T02:42:16Z

X509_check_ip passes the flags to do_x509_check which does use them.

tniessen · 2022-01-18T12:37:50Z

X509_check_ip passes the flags to do_x509_check which does use them.

@paulidale While that's technically true, I don't think any of the flags affect the behavior of do_x509_check when called in the context of X509_check_ip. And that's exactly the confusion that this change is trying to address :)

X509_CHECK_FLAG_ALWAYS_CHECK_SUBJECT has no effect since X509_check_ip never considers the certificate subject, it only considers subject alternative names. Similarly, X509_CHECK_FLAG_NEVER_CHECK_SUBJECT has no effect since it matches the default behavior of X509_check_ip.

X509_check_ip sets check_type to GEN_IPADD and thus cnid to NID_undef in do_x509_check. Because cnid == NID_undef, the flags X509_CHECK_FLAG_ALWAYS_CHECK_SUBJECT and X509_CHECK_FLAG_NEVER_CHECK_SUBJECT have no effect.

openssl/crypto/x509/v3_utl.c

Lines 938 to 944 in e304aa8

    
               if (san_present && !(flags & X509_CHECK_FLAG_ALWAYS_CHECK_SUBJECT)) 
        
                   return 0; 
        
           } 
        
           /* We're done if CN-ID is not pertinent */ 
        
           if (cnid == NID_undef || (flags & X509_CHECK_FLAG_NEVER_CHECK_SUBJECT)) 
        
               return 0;

The remaining four flags (X509_CHECK_FLAG_NO_WILDCARD, X509_CHECK_FLAG_NO_PARTIAL_WILDCARDS, X509_CHECK_FLAG_MULTI_LABEL_WILDCARDS, X509_CHECK_FLAG_SINGLE_LABEL_SUBDOMAINS) only affect equal_wildcard, which is only used by X509_check_host. For these flags, that is already documented:

openssl/doc/man3/X509_check_host.pod

Lines 104 to 122 in 15b7175

    
           If set, B<X509_CHECK_FLAG_NO_WILDCARDS> disables wildcard 
        
           expansion; this only applies to B<X509_check_host>. 
        
           If set, B<X509_CHECK_FLAG_NO_PARTIAL_WILDCARDS> suppresses support 
        
           for "*" as wildcard pattern in labels that have a prefix or suffix, 
        
           such as: "www*" or "*www"; this only applies to B<X509_check_host>. 
        
           If set, B<X509_CHECK_FLAG_MULTI_LABEL_WILDCARDS> allows a "*" that 
        
           constitutes the complete label of a DNS name (e.g. "*.example.com") 
        
           to match more than one label in B<name>; this flag only applies 
        
           to B<X509_check_host>. 
        
           If set, B<X509_CHECK_FLAG_SINGLE_LABEL_SUBDOMAINS> restricts B<name> 
        
           values which start with ".", that would otherwise match any sub-domain 
        
           in the peer certificate, to only match direct child sub-domains. 
        
           Thus, for instance, with this flag set a B<name> of ".example.com" 
        
           would match a peer certificate with a DNS name of "www.example.com", 
        
           but would not match a peer certificate with a DNS name of 
        
           "www.sub.example.com"; this flag only applies to B<X509_check_host>.

doc/man3/X509_check_host.pod

Because no supported flag affects the behavior of X509_check_ip, the flags argument currently has no effect.

paulidale · 2022-01-19T00:13:25Z

equal_case and equal_nocase call through to skip_prefix which uses _X509_CHECK_FLAG_DOT_SUBDOMAINS and X509_CHECK_FLAG_SINGLE_LABEL_SUBDOMAINS. This indicates to me that the documentation is wrong :(

Sigh, this is way too convoluted.

tniessen · 2022-01-19T01:06:16Z

equal_case and equal_nocase call through to skip_prefix which uses _X509_CHECK_FLAG_DOT_SUBDOMAINS and X509_CHECK_FLAG_SINGLE_LABEL_SUBDOMAINS. This indicates to me that the documentation is wrong :(

@paulidale I'm not sure I understand what this means. IP addresses are not stored as text (RFC 5280):

   When the subjectAltName extension contains an iPAddress, the address
   MUST be stored in the octet string in "network byte order", as
   specified in [RFC791].  The least significant bit (LSB) of each octet
   is the LSB of the corresponding byte in the network address.  For IP
   version 4, as specified in [RFC791], the octet string MUST contain
   exactly four octets.  For IP version 6, as specified in
   [RFC2460], the octet string MUST contain exactly sixteen octets.

As far as I can tell, X509_check_ip should only match if the given IP address is equal to the stored IP address. No string processing of any kind should be performed.

paulidale · 2022-01-19T01:23:00Z

X509_check_ip calls do_x509_check with a type of GEN_IPADD. This sets equal to equal_case. When called this calls skip_prefix where the flags are used. However, it looks like removing _X509_CHECK_FLAG_DOT_SUBDOMAINS makes this behave as desired.

Way too convoluted.

tniessen · 2022-01-19T01:27:12Z

Way too convoluted.

Agreed 😃

None of the supported options have any effect on X509_check_ip_asc. Refs: openssl/openssl#17536

None of the supported options have any effect on X509_check_ip_asc. Refs: openssl/openssl#17536 PR-URL: #41571 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com>

Wildcard options do not affect X509_check_email. Refs: openssl/openssl#17536 Refs: nodejs#41571

openssl-machine · 2022-01-20T10:00:22Z

This pull request is ready to merge

Because no supported flag affects the behavior of X509_check_ip, the flags argument currently has no effect. Reviewed-by: Paul Dale <pauli@openssl.org> Reviewed-by: Tomas Mraz <tomas@openssl.org> (Merged from #17536) (cherry picked from commit 2d280fe)

Because no supported flag affects the behavior of X509_check_ip, the flags argument currently has no effect. Reviewed-by: Paul Dale <pauli@openssl.org> Reviewed-by: Tomas Mraz <tomas@openssl.org> (Merged from #17536)

t8m · 2022-01-20T10:19:32Z

Merged to master and 3.0 branches. Thank you for your contribution.