Issue on format of `Private Key Usage Period` extension

The-Lum commented 10 months ago

Describe the bug

Issue on format of Private Key Usage Period extension:

should be in GeneralizedTime.

Maybe I'm missing something....

Reference: RFC 3280

From the spec. RFC 3280 (§4.2.1.4 Private Key Usage Period):

   PrivateKeyUsagePeriod ::= SEQUENCE {
         notBefore       [0]     GeneralizedTime OPTIONAL,
         notAfter        [1]     GeneralizedTime OPTIONAL }

   Where used, notBefore and notAfter are represented as GeneralizedTime
   and MUST be specified and interpreted as defined in section
   4.1.2.5.2.

Creating `Private Key Usage Period` extension

It seems that the format of date fields for Private Key Usage Period are erroneous. ⁉️

Steps to reproduce the behavior (on V5.5.3 with PKCS #12 KeyStore):

Go to 'Generate Key Pair Cert.'
Click on 'Add Extensions'
Click on 'Private Key Usage Period'
Add a date on 'Not Before'
Add a date on 'Not After'
Click on 'OK'

Then:

Go to 'View cert. Details'
Click on 'Extensions'
Cklick on 'Private Key Usage Period'
Click on 'ASN.1'

See the result:

SEQUENCE
{
TAGGED [0] IMPLICIT :
    OCTET STRING (L:15)=
        32 30 32 34 30 31 32 37  30 37 34 38 32 31 5A      20240127074821Z
TAGGED [1] IMPLICIT :
    OCTET STRING (L:15)=
        32 30 32 34 30 33 31 35  30 37 34 38 33 30 5A      20240315074830Z
}

The dates should be in the GeneralizedTime format, and not simply on Octet String.

Probably impacted code: https://github.com/kaikramer/keystore-explorer/blob/a7f95d1a09689fa1543e7e364bc2844db3b329ea/kse/src/main/java/org/kse/gui/dialogs/extensions/DPrivateKeyUsagePeriod.java#L223

Here is my example cert.:

Reading `Private Key Usage Period` extension

With certificate with GeneralizedTime on Private Key Usage Period:

SEQUENCE
{
    TAGGED [0]:
        GENERALIZED TIME=03/Sep/2012 17:05:00.000 CEST (20120903150500GMT+02:00)
    TAGGED [1]:
        GENERALIZED TIME=03/Sep/2013 17:05:00.000 CEST (20130903150500GMT+02:00)
}

We observe this Java error:

java.lang.IllegalStateException: unexpected implicit constructed encoding
    at org.bouncycastle.asn1.ASN1UniversalType.fromImplicitConstructed(ASN1UniversalType.java:34)
    at org.bouncycastle.asn1.ASN1TaggedObject.getBaseUniversal(ASN1TaggedObject.java:372)
    at org.bouncycastle.asn1.ASN1UniversalType.getContextInstance(ASN1UniversalType.java:49)
    at org.bouncycastle.asn1.ASN1GeneralizedTime.getInstance(ASN1GeneralizedTime.java:103)
    at org.bouncycastle.asn1.x509.PrivateKeyUsagePeriod.<init>(Unknown Source)
    at org.bouncycastle.asn1.x509.PrivateKeyUsagePeriod.getInstance(Unknown Source)
    at org.kse.crypto.x509.X509Ext.getPrivateKeyUsagePeriodStringValue(X509Ext.java:711)
    at org.kse.crypto.x509.X509Ext.getStringValue(X509Ext.java:217)
    at org.kse.gui.dialogs.extensions.DViewExtensions.updateExtensionValue(DViewExtensions.java:288)
    at org.kse.gui.dialogs.extensions.DViewExtensions.lambda$initComponents$0(DViewExtensions.java:160)
    ...

Probably impacted code: https://github.com/kaikramer/keystore-explorer/blob/a7f95d1a09689fa1543e7e364bc2844db3b329ea/kse/src/main/java/org/kse/crypto/x509/X509Ext.java#L711

Environment

Version of KSE: 5.5.3
Version of Java: openjdk version "11.0.8" 2020-07-14
Platform (OS): Windows 10

kaikramer commented 10 months ago

The encoding of the extension is correct.

I don't have much time to explain it in detail, but in a nutshell:

The ASN.1 module with the PrivateKeyUsagePeriod extension in RFC 5280 (or 3280) is implicitly tagged, which means the default is IMPLICIT.

Implicit tagging basically means that the byte with the data type is replaced with the tag. So you have to know the ASN.1 definition in order to correctly parse the extension.

The KSE ASN.1 viewer does not know the real data type because it does a generic processing using heuristics. It assumes OCTET STRING as the data type here which is reasonable looking at the data but actually wrong. I can take a look at the ASN.1 viewer, maybe this can be improved.

The-Lum commented 10 months ago

Hello @kaikramer,

Thanks for all those precisions.

In the other hand:

Is the explicit form forbidden (as on the second example, on the reading of existing cert.)? In this case, it's me who has a malformed certificate... 😋

Regards, Th.

The-Lum commented 10 months ago

Hello @kaikramer, and all,

After quick analysis (See A.2 Implicitly Tagged Module, 1988 Syntax):

It seems to me that I had a bad example in my test battery... 😥

Sorry for the noise. 📣

Then the main goal now will be:

How to manage malformed certificate?

Regards, Th.

kaikramer commented 10 months ago

Then the main goal now will be: How to manage malformed certificate?

What do you mean by this? If you mean that KSE should somehow handle malformed certificates, then I disagree. There are so many ways to create malformed certificates and to add workarounds for all these is simply not possible. If input data contains garbage then KSE will show an error.

I will close this issue now, as there is no bug in KSE. I will enhance the handling of implicitly tagged ASN.1 objects in the ASN.1 viewer a bit, but being a heuristic this will never be perfect.

The-Lum commented 10 months ago

Hello @kaikramer,

I will close this issue now, as there is no bug in KSE.

I agree. And sorry for the noise of this report...

I will enhance the handling of implicitly tagged ASN.1 objects in the ASN.1 viewer a bit, but being a heuristic this will never be perfect.

Thanks for this enhancement.

Regards, Th.

kaikramer / keystore-explorer