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.

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

CBOR encoding in GreenCertificateEncoder will be incorrect for date-times #6

Open
martin-lindstrom opened this issue Apr 15, 2021 · 13 comments

Comments

@martin-lindstrom
Copy link

Hi

I've been looking into the CBOR encoding more in detail and se that if you use CBORObject.FromJSONString(json) all elements that should be coded as a CBOR dateTime type (tag 0 or 1) will be coded as ordinary strings (plain or UTF-8). The reason is of course that a date in JSON is a string and there is no way for the CBORObject.FromJSONString to know that some of the strings should be coded as a CBOR dateTime type.

    private byte[] getCborBytes(String json) {
        CBORObject cborObject = CBORObject.FromJSONString(json);

        return cborObject.EncodeToBytes();
    }

See ehn-dcc-development/hcert-schema#17.

Also check out my tests at: https://github.com/DIGGSweden/hcert-impl/blob/main/src/test/java/se/digg/hcert/eu_hcert/v1/MapperUtilsTest.java

Just want to make sure that we are interoperable ...

@jkiddo
Copy link
Collaborator

jkiddo commented Apr 15, 2021

@martin-lindstrom there's a lot of great stuff in your impl, really great stuff. I almost feel I should either copy 80% of your project or discard this repo, with a couple of exceptions. The reason why I feel I cannot discard this repo are the following:

  • The use of lombok. I'm not allowed to lombok by the authorities that I deliver software to.
  • Barcode deps. The scope of this repo is limited to simply generating the string of bytes that are intended to be rendered and as such does not intend to include BarCode libs
  • Limitation to Java 8 -I haven't checked whether that is an issue with your repo actually.

-anyways, your stuff looks good

@martin-lindstrom
Copy link
Author

Many thank for your kind words.

Regarding your comments:

  • Lombok: I can remove the Lombok-dependency. Right now I am only using the @Slf4j annotation.
  • What if I make the dependencies to the zxing-jars optional?
  • I can make a Maven-profile where I build using Java 8 ...

@martin-lindstrom
Copy link
Author

I tried to build with Java 8. Won't be possible since we include a dependency to the credentials-support library that is essential for our HSM signing capabilities. See diggsweden/dgc-java#5

But what are you still doing using Java 8? It was long ago it was declared EOL.

@jkiddo
Copy link
Collaborator

jkiddo commented Apr 15, 2021

Tell that to my customer ...

@jkiddo
Copy link
Collaborator

jkiddo commented Apr 15, 2021

Ill see if i can change their minds

@martin-lindstrom
Copy link
Author

While your at it, tell them that Lombok is only used at compile time ...

@jkiddo
Copy link
Collaborator

jkiddo commented Apr 15, 2021

I know ... But that is another thing

@jkiddo
Copy link
Collaborator

jkiddo commented Apr 15, 2021

Many thank for your kind words.

Regarding your comments:

  • Lombok: I can remove the Lombok-dependency. Right now I am only using the @Slf4j annotation.
  • What if I make the dependencies to the zxing-jars optional?
  • I can make a Maven-profile where I build using Java 8 ...

@martin-lindstrom If you can remove the use of lombok and move the use of the zxing-jars to test scope (which should be possible - and it would still illustrate how you could transform the certificate to a QR) it would certainly fit my case

@martin-lindstrom
Copy link
Author

I will mark the zxing-jars as optional which makes it possible for you to use the library (as long as you don't create or decode barcodes). I can't have them in test-scope since creating a barcode will be one of the features of the library.

@martin-lindstrom
Copy link
Author

The stuff checked in to master is now written in a way that you can use the DGCEncoder and DGCDecoder without barcodes.

@jkiddo
Copy link
Collaborator

jkiddo commented Apr 17, 2021

Great @martin-lindstrom - do you still think it is nessecary with the swedish dependency in https://github.com/ehn-digital-green-development/dgc-java/blob/173c4dfc380ff19ff105b3439282554f6d1160d1/pom.xml#L135 since it is now a fork that probably should be usable by others than the swedish produced solutions (which it ofc. is but I don't see the dep usable by other than the swedish manufactures). As far as I can see, it could be abstracted away

@martin-lindstrom
Copy link
Author

Great @martin-lindstrom - do you still think it is nessecary with the swedish dependency in https://github.com/ehn-digital-green-development/dgc-java/blob/173c4dfc380ff19ff105b3439282554f6d1160d1/pom.xml#L135 since it is now a fork that probably should be usable by others than the swedish produced solutions (which it ofc. is but I don't see the dep usable by other than the swedish manufactures). As far as I can see, it could be abstracted away

That dependency is to a generic library for PKI credentials (private key/certificate) and it is nothing Swedish about it other than it is released from se.swedenconnect. I will not remove that dependency since it has a very good support for PKCS#11 (HSM) which I hope more than Sweden will use for their signers.

@jkiddo
Copy link
Collaborator

jkiddo commented Apr 17, 2021

Point taken

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants