JuhoPaananen / RSA-salaaja

0 stars 0 forks source link

Vertaisarviointi #2

Open asvorg opened 1 year ago

asvorg commented 1 year ago

Repo haettu 24.6 klo 20.30.

Yleistä

Asennus

Asennus ei suoraan ohjeiden mukaan onnistunut. Poetry install antaa virheen

Installing dependencies from lock file

No dependencies to install or update

/home/aapkoski//RSA-salaaja/rsa_salaaja does not contain any element

ja ./rsa-salaaja virheen

[26725] Error loading Python lib '/tmp/_MEInml3CW/libpython3.10.so.1.0': dlopen: /lib/x86_64-linux-gnu/libm.so.6: version `GLIBC_2.35' not found (required by /tmp/_MEInml3CW/libpython3.10.so.1.0)

Syytä tähän en tiedä, ongelma voi hyvin myös olla omassa päädyssäni, en sitä ruvennut sen kummemmin selvittelemään. Tästä kuitenkin selvittiin navigoimalla src kansioon ja ajamalla python3 rsa-salaaja.py, ja ohjelma käynnistyy ongelmitta. Epäilisin kuitenkin että ongelma on omani.

Käyttö

Ohjelma tuntuu toimivan oikein hyvin ja ilman suurempia ongelmia. Salaus ja purku toimii myös avaimilla mitkä olen generoinut omalla RSA toteutuksellani. Pieni asia minkä huomasin, oli että vaikka ohjelma tarkistaa että viestin koko ei ole liian pitkä, on tarkistajan ymmärtämä pituus ilmeisesti kiinteä, ja kun generoin pienemmät avaimet (omalla toteutuksellani) kuin 1024, mitä tämä totetus generoi, hajottaa se purun, mutta tämä ei tietenkään mitään katastrofaalista ole. Samalla tavalla en saanut tätä hajoamaan käyttämällä pelkästää ohjelman itse generoimia avaimia. Myös avainten kaksi eri komponenttia (n ja d, sekä n ja e) voisi ruudussa erottaa tyhjällä rivillä, viivalla tai jollain muulla vastaavalla.

Koodikatselmointi

Alussa olisi hyvä mainita, että koodikatselmointi on suht pikaisesti tehty. Tiedostossa gui.py olevaan koodiin en perehtynyt ollenkaan muuten kuin kommentit lukemalla, kommentit kuitenkin hyviä ja kattavia.

Ydinohjelman koodi kokonaisuudessaan mielestäni hyvää. Docstringit kunnossa ja kattavia, muuttujat nimetty järkevästi ja ymmärrettävästi. Alkulukujen generointiin liittyvät funktiot vaikuttavat toimivan oikein, ja vastaavat melko tarkasti omaa toteustani. Epäselväksi jäi, miksi e lasketaan erikseen, omissa lähteissäni suositeltiin lähes yksinomaan käyttämään lukua 65537.

Osa tiedostoista mielestäni nimetty hieman kummallisesti, esimerkiksi rsa.py voisi olla mielummin jotain encrypt_decrypt.py suuntaista. Myös utils.py ja keygen.py hieman epäselvät, utils.py piti allaan kaiken alkulukuihin liittyvän, ja tämän voisi nimetä jotenkin fiksummin. Keygen.py importissa käytetyt sulkeet turhia? Nämä ei tietenkään mitään katastrofaalisia mokia ole.

Kaikenkaikkiaan koodi kuitenkin omaan silmääni todella hyvää, funktionaalisuudessa ei turhia looppeja tms, eikä minulla tähän mitään erityistä lisättävää ole. Plussaa kattavista docstringeistä

Kokonaisuus

Oikein mielenkiintoinen ja toimiva RSA projekti! Graafinen käyttöliittymä oli mielestäni oikein hyvä idea, vaikka itse olin omalla kohdallani todennut että se ei tähän projektiin kovin hyvin istu, niin oikein hyvin näyttää toimivan. Myös avainten generointiin käytetyn ajan näyttäminen oli hauska idea, minkä ajattelin omaan projektiini toteuttaa, nyt kun sen tässä näin. Kehitysideana mahdollisuus käyttäjälle päättää avaimen koko, ja kokonaisen tekstitiedoston salaus.

Hyvä!

JuhoPaananen commented 1 year ago

Kiitos ajastasi ja kommenteista!

Paketin ajamisen suhteen virhe johtuu luultavasti GLIBC 2.35 puuttumisesta sinun päässäsi (virhe siis kuitenkin minun päässä pakettia luodessa). En suoraan sanottuna hoksannut, että Python 3.10 tai jonkin muu paketoinnissa syntynyt riippuvuus edellyttää tuota GLIBC versiota. Lisäsin tästä maininnan käyttöohjeeseen. Korjausta en ehdi tähän nyt tehdä.

Poetryn asennukseen liittyen rsa_salaaja hakemistoa ei ole projektissa olemassa, joten pyproject.toml on sisältänyt virheen ja tämä varmasti aiheuttaa kyseisen virheilmoituksen. Lisäksi taskit sisälsivät vanhan index.py viittauksen 'poetry run invoke start" kommennolle, joten sekään ei voinut toimia kuten oli tarkoitettu. Hyvä, että maltoit suorittaa ohjelman perinteiseen tyyliin python3 tulkilla.

Erinomainen huomio e:n suhteen. Omassa toteutuksessa se lasketaan erikseen, koska tuo idea syntyi suoraan pseudokoodista Wikipedian artikkelissa. Tämä ei ole tarpeen kuten mainitsitkin, mutta sainpa toteuttaa oman Eukleideen algoritmin pienimmän yhteisen tekijän löytämiseksi :) Hieman lisää aikaa tämä toki ottaa verrattuna 65537 käyttämiseen, joten avaimien luomisen tehostamiseksi voisi hyvinkin harkita tuon muuttamista.

Koodikatselmoinin nimeämishuomiot olivat hyviä. On aivan totta, että moduulien nimet eivät ole parhaasta päästä kuvaamaan sisältöjä.