search

wbond / asn1crypto

Python ASN.1 library with a focus on performance and a pythonic API
MIT License
335 stars 140 forks source link

Fractional seconds in ocsp.CertStatus #185

Open neildunbar opened 4 years ago

neildunbar commented 4 years ago

Because CertStatus can take a revocation time, when setting that, might it be an idea to zero any fractional seconds which might exist in the native datetime?

RFC 6960's entry on GeneralizedTime refers back to RFC 5280, Section 4.1.2.5.2, which states:

GeneralizedTime values MUST NOT include fractional seconds.

Of course, one can always just .replace(microsecond=0) on the datetime value one sets, but this might avoid the danger of creating OCSP Responses (and possibly CRLs, after 2050!) which violate the relevant RFCs.

joernheissler commented 4 years ago

Hello! Could you please provide a code snippet that generates such a CertStatus with fractions?

joernheissler commented 4 years ago

Quote from X.690 (§11.7.3):

The fractional-seconds elements, if present, shall omit all trailing zeros; if the elements correspond to 0, they shall be wholly omitted, and the decimal point element also shall be omitted.

So the GeneralizedTime type is allowed to include fractions. RFC5280 restricts it further. Full quote:

For the purposes of this profile, GeneralizedTime values MUST be expressed in Greenwich Mean Time (Zulu) and MUST include seconds (i.e., times are YYYYMMDDHHMMSSZ), even where the number of seconds is zero. GeneralizedTime values MUST NOT include fractional seconds.

So in general it's fine to accept fractions. Only when creating X.509 objects, it's not.

neildunbar commented 4 years ago

So a bit of code using ocspbuilder:

from datetime import datetime, timezone

from oscrypto import asymmetric

from ocspbuilder import OCSPResponseBuilder

privkey_bytes = b64decode(
    """
MIIBVgIBADANBgkqhkiG9w0BAQEFAASCAUAwggE8AgEAAkEA5hMXb2uO2TZ6BUUU
nHwA+7xVBw0ifF13bt339r9hLhJKnAGbqqxRF/PcSm3IlM14/j53T9KbZmxmd6JQ
GiLapwIDAQABAkAiKzGuzXWAktOaVsER4GSw/i5OhsfZWnQzVenOjmubULoGmPB9
L3Dz4UC1B/usKdbkHgH6moyOR9Oigyvp7MfJAiEA9Y11vPkWrHPqyBY1Dv2GgHag
9dpkAEmV2R5nVCiDj+UCIQDv3Qvhenut3UpETan9hp1LHrl2ZUt8+YfHcPyIBvYf
mwIhAL2YLfJtOW6KShuX2fvrEPEbp4hsyY3XQ1ZTPWEjrwFpAiEAlSPfGEKdFhzq
6Y9UrAOAV83xyTDwf/NzPkn9auLRNBMCIQC94SBmMuEjWz1x/EzGI7fBuRJ76iHH
hCSFyAAYLXlkJQ==
"""
)

issuer_bytes = b64decode(
    """
MIIBezCCASWgAwIBAgIUaYsvXH25fbTOhy0zqJ3XextaoYIwDQYJKoZIhvcNAQEL
BQAwEjEQMA4GA1UEAwwHRmFrZS1DQTAeFw0yMDA2MDQxMzA1MzNaFw0zMDAxMDIx
MzA1MzNaMBIxEDAOBgNVBAMMB0Zha2UtQ0EwXDANBgkqhkiG9w0BAQEFAANLADBI
AkEAq8d0vbR6Z3DrQRaDnPIRzAOXOCFSL5/F6Px/BJ2oFj9rxkwYrDvCKYp7vh+3
ctAVFBPo6IxLimjSPBQDKHWpgwIDAQABo1MwUTAdBgNVHQ4EFgQUKRRbFI+sgdMn
m4vpg+Z2wDXQz7cwHwYDVR0jBBgwFoAUKRRbFI+sgdMnm4vpg+Z2wDXQz7cwDwYD
VR0TAQH/BAUwAwEB/zANBgkqhkiG9w0BAQsFAANBAHhWtrHl2h2Y7TYl7jYJttOb
7Tww5WU5kiP2Rmxmle3B8nRSpmi14Bp/FmLhcLfbFG6N3HYWAuuysTKveIOgDCg=
"""
)

subject_bytes = b64decode(
    """
MIIBKDCB0wIUYBmxJxoPx5p9buqW1+mocngt9gswDQYJKoZIhvcNAQELBQAwEjEQ
MA4GA1UEAwwHRmFrZS1DQTAeFw0yMDA2MDQxMzI3MzNaFw0yMTA1MzAxMzI3MzNa
MBoxGDAWBgNVBAMMD0Zha2UtRW5kX0VudGl0eTBcMA0GCSqGSIb3DQEBAQUAA0sA
MEgCQQCnCXhR6CvQCv4tGRZ2onZ4GyUbI4Mlw8wcqSSANgYXAdtflycurrpxpA5R
PHUWIegKpOGfgiI1ffFpDqUvx1fZAgMBAAEwDQYJKoZIhvcNAQELBQADQQBROqMb
QPrrFN/dync8miHtUp5HSzMzxP1DVUysEwkPJoKQ7UoH3doje6aIwGuhsfbt+URe
9uTVSfVBja9WXaB2
"""
)

def main():
    issuer_key = asymmetric.load_private_key(privkey_bytes)
    issuer_cert = asymmetric.load_certificate(issuer_bytes)
    subject_cert = asymmetric.load_certificate(subject_bytes)
    dt = datetime(
        year=2019,
        month=12,
        day=25,
        hour=12,
        minute=30,
        second=6,
        microsecond=12345,
        tzinfo=timezone.utc,
    )
    builder = OCSPResponseBuilder("successful", subject_cert, "key_compromise", dt)
    ocsp_response = builder.build(issuer_key, issuer_cert)
    print(b64encode(ocsp_response.dump()).decode("ascii"))

if __name__ == "__main__":
    main()

Run that (say it's called frac-ocsp.py) as

python frac-ocsp.py | base64 -d | openssl ocsp -respin - -noverify -resp_text

and you get

OCSP Response Data: OCSP Response Status: successful (0x0) Response Type: Basic OCSP Response Version: 1 (0x0) Responder Id: 29145B148FAC81D3279B8BE983E676C035D0CFB7 Produced At: Jun 4 13:30:11.666238 2020 GMT Responses: Certificate ID: Hash Algorithm: sha1 Issuer Name Hash: 1896673A3F00652F42E9713459F6C4FB712C708E Issuer Key Hash: 29145B148FAC81D3279B8BE983E676C035D0CFB7 Serial Number: 6019B1271A0FC79A7D6EEA96D7E9A872782DF60B Cert Status: revoked Revocation Time: Dec 25 12:30:06.012345 2019 GMT Revocation Reason: keyCompromise (0x1) This Update: Jun 4 13:30:11.666238 2020 GMT Next Update: Jun 11 13:30:11.666238 2020 GMT

Signature Algorithm: sha256WithRSAEncryption
     d9:bb:0e:3a:b0:c4:63:ba:57:dc:92:f9:d0:db:c0:f0:9d:d6:
     2f:69:02:e7:95:74:65:e3:3a:79:fb:b8:2f:26:71:04:8c:9b:
     f9:9a:88:bc:7a:41:2d:22:9e:d4:89:bb:ee:b3:e8:34:03:74:
     63:c2:74:2f:d3:84:17:fc:8c:bd
neildunbar commented 4 years ago

So in general it's fine to accept fractions.

Correct. The GeneralizedTime object is 100% fine. It could and should accept fractional seconds.

Only when creating X.509 objects, it's not.

Yup; and since a CertStatus is part of a profile defined via RFC 6960 (via RFC 5280), fractional seconds are a bad thing. It's not a bug, it's just that if you don't take care to zero the microseconds before you build, you'll get a non-compliant object.

neildunbar commented 4 years ago

Also to point out: the produced_at, this_update and next_update shouldn't have fractional components either, but that's OCSP in general, not CertStatus.

joernheissler commented 4 years ago

There are several options here:

I don't think fraction support should be removed, because it's valid and someone might have a legitimate reason to encode them. There are many incorrectly encoded ASN.1 objects in the wild and asn1crypto tries to do better. It's not a fully-featured ASN.1 library that allows generating all sorts of wrong encodings. For sake of user experience and generating correct output, best would be to strip fractions when encoding certain types.

Might be possible by adding a mixin similar like _ForceNullParameters to some classes.

@wbond your thoughts on this?

neildunbar commented 4 years ago

There are several options here:

  • Do nothing. As you pointed out, this may result in incorrect encodings.

And that's fine - as long as the programmer knows it could create RFC violating objects (ie, just mention it in the docs)

  • When encoding certain types (e.g. CertStatus), silently remove fractions.

I think that's what I would do. See which types embed types and are created out of PKIX RFCs and strip fractional seconds for them. It would even be possible to have an override flag which allows the fractional seconds, if the programmer wanted that behaviour.

  • Print a warning, but include the fractions.
  • Print a warning, but strip the fractions.

Both useful.

  • Raise an Exception.

Definitely helps to educate the programmer, but isn't it just as easy to remove the fraction component?

  • Remove fraction support from GeneralizedTime.

I don't think this is a good approach either. GeneralizedTime isn't defined from the PKIX RFCs, so the ability to timestamp fractions is useful. I can see a high accuracy timestamping service based on X.500 objects needing such precision.

wbond commented 4 years ago

It sounds like the actionable items from this are:

I haven't spent any time looking to see if the TSP or CMS usage should allow fractional components. I probably won't in the near term, but if one of you has guidance or knowledge about that, I'd be happy to work on getting it fixed.