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

Signature validation #214

Merged

Conversation

celuchmarek
Copy link
Member

@celuchmarek celuchmarek commented Jun 28, 2023

No description provided.

@jsuchal
Copy link
Member

jsuchal commented Jul 19, 2023

@celuchmarek ku GUI mame nejake vlakno na slacku, pripadne Taja to videl?

@celuchmarek
Copy link
Member Author

@jsuchal jo, Taja to videl, ale nemal nič veľmi k tomu. Na slacku vo vlákne sú aktuálnejšie screenshoty.

Comment on lines 54 to 55
journalCertificateSource = new KeyStoreCertificateSource(
new File("/home/turtle/slovensko_digital/autogram/src/main/resources/lotlKeyStore.p12"), "PKCS12", "dss-password");
Copy link
Member Author

@celuchmarek celuchmarek Jul 19, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Toto je pomerne divná vec. List of Trusted Lists (napr pre EU) obyčajne nie je nijako trusted, keď to iba stiahneme z nejakej adresy. Preto existuje na internete 8 certifikátov tohto LOTL. Aj podľa DSS oficiálnej dokumentácie s examplami sa to má robiť tak, že do tohto keystoru človek uloží tých 8 certifikátov, ktorými sa validuje samotný LOTL. (google Pivot LOTL)

Tak som manuálne z oficiálnej stránky okopíroval tie certifikáty v textovej forme a nakoniec ich importol do tohoto keystoru. A takto to funguje. LOTL je teraz "well signed". Avšak, zdá sa mi divné, že keystore je chránený nejakým heslom, ktoré je public. Ale možno je tam to heslo iba preto, lebo to Java nevie bez hesla.

@celuchmarek celuchmarek marked this pull request as ready for review August 10, 2023 06:56
@celuchmarek
Copy link
Member Author

@jsuchal Tuto je zatiaľ validácia bez GUI. Je zatiaľ otázka, ako to bude fungovať ohľadom ukladania TL a čítania certifikátov na iných strojoch, ale uvidíme. Tie controllery som vytváral trochu inak, aby som vedel rozumne držať ich inštancie a posielať do nich potom niekedy v čase tie validačné reporty. To ale nefunguje úplne dobre, lebo, ak je otvorený zoznam podpisov, nedá sa klikať na okno podpisovania a podobne. Niekedy (10%) sa okná otvoria v zlom poradí, že okno podpisovania je vždy vpredu a ostatné sú za tým a nedajú sa dostať pred okno podpisovania.

-> ui.startSigning(job, this));

ui.onWorkThreadDo(()
-> checkAndValidateSignatures(job));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Toto sa ma diat aj pri CLI? Nemyslim uplne.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Dal som do startSigning do vnútra GUI.

Comment on lines 193 to 206
ui.onWorkThreadDo(() -> {
SignatureValidator.getInstance().initialize();
});

var timer = new Timer();
timer.scheduleAtFixedRate(new TimerTask() {
public void run() {
SignatureValidator.getInstance().refresh();
}
},
3600000L,
3600000L);

return timer;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Toto robi co?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Každých 8 hodín behu Autogramu refreshne Trusted Listy

Comment on lines 38 to 39
private Reports signatureCheckReport;
private Reports signatureValidationReport;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Toto mi tu domenovo uplne nesedi, Job bol immutable a report s nim asi nesuvisi ci?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No report je výsledok jobu a job chceme spúšťať iba raz, lebo trvá nejaký čas. Takže si ho uložíme sem.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Podobne sme to mali s vizualizaciou a netlacime to do jobu

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay. Zapracujem budúci týždeň

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Toto som prehodil do "pamäte" Controllera.

Comment on lines 51 to 54
public void onSignatureValidationCompleted(SigningJob job);

public void onSignatureCheckCompleted(SigningJob job);

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tu radsej poslime ten report ako separe parameter ako to posuvat cez signing job

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jsuchal Podľa signingJobu sa potom nájde príslušný controller, takže asi to nepôjde bez toho.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mozeme to spravit naopak. Report bude obsahovat odkaz na signingjob a uz vies.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Report je ale objekt z DSS. Chceme nad tým vytvoriť ďalší wrapper?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rozumiem a hej, dava mi to zmysel.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Vyrobil som core.ValidationReportsWrapper

Comment on lines 586 to 602
-fx-background-color: #cce2d8;
-fx-border-color: #cce2d8;
}

.autogram-tag-invalid {
-fx-background-color: #f8d7da;
-fx-border-color: #f8d7da;
}

.autogram-tag-valid--text {
-fx-text-fill: #005a30;
-fx-fill: #005a30;
}

.autogram-tag-invalid--text {
-fx-text-fill: #721c24;
-fx-fill: #721c24;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Farby potom vytiahnime do konstant

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ďalej pôjdem robiť GUI, tak pri tej príležitosti to povyťahujem.

@celuchmarek celuchmarek requested a review from a team as a code owner August 17, 2023 12:31
if (!SignatureValidator.hasSignatures(job.getDocument()))
return;

var reports = SignatureValidator.getInstance().getSignatureValidationReport(job.getDocument());
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nedava zmysel, aby nas validator dostal job a vratil uz ten ValidationReports? (Wrapper by som dropol, ved to je nas namespace)

@@ -31,6 +32,8 @@ public void start(Stage windowStage) throws Exception {

windowStage.setOnCloseRequest(event -> {
new Thread(server::stop).start();
timer.cancel();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Toto mi pride ako prilisna vnutornost. Nemal by mat mat autogram len nejaky close kde sa toto udeje?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tomuto nerozumiem. Vytvára a zastavuje sa to na rovnakom mieste ako server. To nie je ok?

@celuchmarek
Copy link
Member Author

@tajovic @jsuchal Už je to v takom stave, že by som sa nehanbil to mergnúť. Nakoniec som ten redizajn dokončil takto, že link som dal osobitne pod tabuľku.

image

Warning je potom takto:

image

@celuchmarek
Copy link
Member Author

@jsuchal snažil som sa prerobiť aj flow podpisovania, ako sme sa bavili, ale radšej by som to spravil v osobitnom PR. Rovnako pred releasom by sme podľa mňa mali dorobiť aj nastavenia. Dám k tomu dokopy ešte nejaký návrh a postúpim ho na ux oddelenie.

@jsuchal
Copy link
Member

jsuchal commented Sep 21, 2023

@celuchmarek to UI este ja nieco skusim, mam napad.

co sa tyka prerabky flowu, suhlasim, ze to dajme bokom. settings nutne do release netreba, ale nekvasme dlho s tym PR lebo sa nam to rozlezie.

@celuchmarek
Copy link
Member Author

Nerozumiem poslednej vete.

@jsuchal
Copy link
Member

jsuchal commented Sep 21, 2023

Ze ten settings PR riesme dalej, ale nutne do release nam ho myslim netreba.

@jsuchal
Copy link
Member

jsuchal commented Sep 22, 2023

@celuchmarek taketo sa mi podarilo
image

Matie ma to Vysledok overenia: neznamy podpis.

@celuchmarek
Copy link
Member Author

To je ten revokovaný OP?

@jsuchal
Copy link
Member

jsuchal commented Sep 22, 2023

Neviem ci je revokovany ale toto je detail
image

@celuchmarek
Copy link
Member Author

Aha, čiže v sľubovanom čase podpisu bol ešte certifikát validný. Teraz je už ekspirovaný a nemá pečiatku, ktorá by dosvedčila ten čas. Tým pádom sa nedá zaručiť, že je to platné a že podpis hovorí pravdu.

Ale hej, Neznámy popis by v tomto prípade bolo dobré zmeniť na niečo iné.

@celuchmarek
Copy link
Member Author

@jsuchal tak teda už som nejako dokončil ten zoznam podpisov. Je tam tiaž taký divný hack, že treba zavolať stage.sizeToScene, setManaged(true) a zase stage.sizeToScene. Inak sa neresizol správne mainBox a pôvodí to z toho GridPanu. Možno ešte otázka, či nedať max 3 a nie max 4 podpisy.

Pridal som aj wrapovanie do vn[tra badgu/tagu (máme najviac 45 znakov) a tiež som trochu pofixoval aj wrapovanie viacerých badgov. To funguje pekne v signing dialogu v grid pane, ale nefunguje natívne v detailoch podpisov - tam je napevno 300px, ale tam sa okno nedá resizovať, takže asi ok.

Trochu viac som ohandloval aj tie hlášky "výsledok overenia".

@jsuchal
Copy link
Member

jsuchal commented Sep 29, 2023

@celuchmarek uz sme skoro tam, este updatnime release checklist kedze pridavame riadne velku funkcionalitu.

@celuchmarek
Copy link
Member Author

Ešte tam bolo niečo čarovné. Zle sa zobrazili tie nižšie badge, ak ten horný pretekal. Ak som ho zabalil, tak sa to zase kazilo ak pretekalo meno. Keď som zavolal dvakrát stage.show(), už to ide ok vo všetkých prípadoch :D Do checklistu chceme dať aj nejaké sample podpísané súbory na testovanie overenia a podobne alebo len, že to vie overiť nejaké podpisy?

@@ -133,7 +133,7 @@ public static VBox createSignatureBox(Reports reports, boolean isValidated, Stri
if (!isValidated)
badge = SignatureBadgeFactory.createInProgressBadge();
else if (isFailed)
badge = SignatureBadgeFactory.createInvalidBadge("Neplatný podpis");
badge = SignatureBadgeFactory.createInvalidBadge("Neplatný podpis Neplatný podpis Neplatný podpis Neplatný podpis");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

?

@jsuchal
Copy link
Member

jsuchal commented Sep 29, 2023

Nejaky sample podpisany by sa hodil ale neviem kam ho dame. Niekam ho treba asi zavesit do nejake issue nech sa nestrati.

@celuchmarek celuchmarek merged commit 8f57bb3 into slovensko-digital:main Sep 29, 2023
3 checks passed
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

Successfully merging this pull request may close these issues.

3 participants