Skip to content
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

Generate valid time values in ca program #3444

Closed
kroeckx opened this issue May 11, 2017 · 3 comments
Closed

Generate valid time values in ca program #3444

kroeckx opened this issue May 11, 2017 · 3 comments

Comments

@kroeckx
Copy link
Member

kroeckx commented May 11, 2017

I just found 2 bugs reports that point out that the openssl ca app just puts -startdate and -enddate into the certificate as it was specified on the command line even when it was invalid. We should either reject them or normalize them.

Bugs:
https://gitlab.com/gnutls/gnutls/issues/196
https://bugs.debian.org/862335
@rockdaboot
Copy link

From the Debian bug report:

Just for the record, the latest openssl (1.1.1-dev from Github) accepts
this (seen from the code):

[SS] is optional, <+|-> = either + or - must be present

  1. YYMMDDHHMM[SS]Z YYMMDDHHMM[SS]<+|->hhmm
    If valid, these date strings are written to ASN.1 into an UTCTime field.

  2. YYYYMMDDHHMM[SS]Z or YYYYMMDDHHMMSS<+|->hhmm
    If valid, these date strings are written to ASN.1 into a GeneralizedTime
    field.

Regarding RFC5280 in both cases (UTCTime and GeneralizedTime) the
seconds (SS) and Z (Zulu) timezone is a MUST.

See RFC5280 '4.1.2.5.1. UTCTime' and '4.1.2.5.2. GeneralizedTime'.

OpenSSL relies on their ASN.1 code to check for validity, which is
simply not strict enough. Other implementors do a strict check and thus
might reject certificates generated by openssl.

@InfoHunter InfoHunter mentioned this issue May 26, 2017
Closed
2 tasks
@InfoHunter
Copy link
Member

Hi guys, I have made a fix for this issue. For UTC time, the format of time string is limited to 'YYMMDDHHMMSSZ', and for GeneralizedTime is 'YYYYMMDDHHMMSSZ', without +/- and fractional seconds support.

InfoHunter added a commit to InfoHunter/openssl that referenced this issue Jun 9, 2017
@InfoHunter
Introduce ASN1_TIME_set_string_X509 API
acd89ed 
Fixes issue openssl#3444.

This one is used to enforce strict format (RFC 5280) check and to
convert GeneralizedTime to UTCTime.

apps/ca has been changed to use the new API.

Test cases and documentation are updated/added

Signed-off-by: Paul Yang <paulyang.inf@gmail.com>
levitte pushed a commit that referenced this issue Jun 11, 2017
Introduce ASN1_TIME_set_string_X509 API
04e6271 
Make funcs to deal with non-null-term'd string
in both asn1_generalizedtime_to_tm() and asn1_utctime_to_tm().

Fixes issue #3444.

This one is used to enforce strict format (RFC 5280) check and to
convert GeneralizedTime to UTCTime.

apps/ca has been changed to use the new API.

Test cases and documentation are updated/added

Signed-off-by: Paul Yang <paulyang.inf@gmail.com>

Reviewed-by: Kurt Roeckx <kurt@openssl.org>
Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from #3566)
@richsalz
Copy link
Contributor

closing, okay @kroeckx ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

RFC5280 strict time string check InfoHunter/openssl
4 participants
@InfoHunter @kroeckx @rockdaboot @richsalz