Skip to content

Fix ca crash (1.0.2) #5115

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
mattcaswell wants to merge 2 commits into from
Closed

Fix ca crash (1.0.2) #5115

mattcaswell wants to merge 2 commits into from

Conversation

@mattcaswell
Copy link
Member

@mattcaswell mattcaswell commented Jan 19, 2018

Two related issues were causing a seg fault in the ca application. Firstly it is possible to create a certificate with an empty Subject. Secondly if ca reads an index.txt file with a missing subject in an entry then it will crash.

This is the 1.0.2 version of #5114.

Fixes #5109

@mattcaswell
Don't allow an empty Subject when creating a Certificate
ade4fa6 
Misconfiguration (e.g. an empty policy section in the config file) can
lead to an empty Subject. Since certificates should have unique Subjects
this should not be allowed.
@mattcaswell
Don't crash on a missing Subject in index.txt
5b0367e 
An index.txt entry which has an empty Subject name field will cause ca
to crash. Therefore check it when we load it to make sure its not empty.

Fixes openssl#5109
@mattcaswell mattcaswell added the branch: 1.0.2 Applies to OpenSSL_1_0_2-stable branch (EOL) label Jan 19, 2018
@mattcaswell mattcaswell mentioned this pull request Jan 19, 2018
Closed
@richsalz richsalz added the approval: done This pull request has the required number of approvals label Jan 19, 2018
@mattcaswell
Copy link
Member Author

mattcaswell commented Jan 22, 2018

Pushed. Thanks.

levitte pushed a commit that referenced this pull request Jan 22, 2018
@mattcaswell
Don't allow an empty Subject when creating a Certificate
dd37f6f 
Misconfiguration (e.g. an empty policy section in the config file) can
lead to an empty Subject. Since certificates should have unique Subjects
this should not be allowed.

Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from #5115)
levitte pushed a commit that referenced this pull request Jan 22, 2018
@mattcaswell
Don't crash on a missing Subject in index.txt
a3d684f 
An index.txt entry which has an empty Subject name field will cause ca
to crash. Therefore check it when we load it to make sure its not empty.

Fixes #5109

Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from #5115)
@vdukhovni
Copy link

vdukhovni commented Feb 22, 2018

Empty subjects DNs (empty RDN sequences) are actually valid, and are normally printed as "/" on output, and were accepted via the -subj / option on input. In particular, per RFC5280:

   The subject field identifies the entity associated with the public
   key stored in the subject public key field.  The subject name MAY be
   carried in the subject field and/or the subjectAltName extension.  If
   the subject is a CA (e.g., the basic constraints extension, as
   discussed in Section 4.2.1.9, is present and the value of cA is
   TRUE), then the subject field MUST be populated with a non-empty
   distinguished name matching the contents of the issuer field (Section
   4.1.2.4) in all certificates issued by the subject CA.  If the
   subject is a CRL issuer (e.g., the key usage extension, as discussed
   in Section 4.2.1.3, is present and the value of cRLSign is TRUE),
   then the subject field MUST be populated with a non-empty
   distinguished name matching the contents of the issuer field (Section
   5.1.2.3) in all CRLs issued by the subject CRL issuer.  If subject
   naming information is present only in the subjectAltName extension
   (e.g., a key bound only to an email address or URI), **then the subject
   name MUST be an empty sequence** and the subjectAltName extension MUST
   be critical.

I should also point out that some mainstream CAs (e.g. Let's Encrypt) include only a DNS name "CN" in the subject DN, but this is limited to ~64 bytes, and longer CNs are disallowed. In such cases one is forced to use an empty subject DN. Therefore, this PR is incorrect, and the change must be reverted.

Instead the ca code MUST handle empty subject DNs gracefully. Of course such DNs cannot be required unique, so they should be excluded from the uniqueness checks.

@vdukhovni vdukhovni reopened this Feb 22, 2018
@vdukhovni vdukhovni mentioned this pull request Feb 22, 2018
Closed
@vdukhovni vdukhovni removed the approval: done This pull request has the required number of approvals label Feb 22, 2018
@mattcaswell
Copy link
Member Author

mattcaswell commented Feb 23, 2018

Closing this in favour of tracking it as part of the original issue, #5109.

This was referenced Feb 23, 2018
Closed
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@richsalz richsalz richsalz approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

branch: 1.0.2 Applies to OpenSSL_1_0_2-stable branch (EOL)

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@mattcaswell @vdukhovni @richsalz