Opened 7 years ago

Last modified 4 years ago

#11565 needs_work enhancement

RSA Cryptosystem

Reported by: ajeeshr Owned by: mvngu
Priority: major Milestone: sage-6.6
Component: cryptography Keywords: RSA, crypto, public key encryption
Cc: nguyenminh2@… Merged in:
Authors: Peter Story, Ajeesh Ravindran Reviewers:
Report Upstream: N/A Work issues:
Branch: u/peter.story/rsa_cryptosystem (Commits) Commit: c45c4dd7966114639a5d39b1e526729235d14e7a
Dependencies: Stopgaps:

Description (last modified by peter.story)

The Rivest, Shamir and Adleman encryption system is a widely accepted public key encryption scheme. The security depends on the difficulty of factoring the product of large primes.

Attachments (2)

rsa_cryptosystem.py (14.3 KB) - added by ajeeshr 7 years ago.
This is the python code I implemented in python. Just copy this into the public_key folder in crypto, import the class in this code into all.py in the public_key and re-build sage and run it in a worksheet, its simple!!!
rsa_cryptosystem.2.py (22.8 KB) - added by peter.story 4 years ago.
Major rewrite of the module, with the goal of a pedagogical implementation.

Download all attachments as: .zip

Change History (28)

Changed 7 years ago by ajeeshr

This is the python code I implemented in python. Just copy this into the public_key folder in crypto, import the class in this code into all.py in the public_key and re-build sage and run it in a worksheet, its simple!!!

comment:1 Changed 7 years ago by ajeeshr

  • Component changed from PLEASE CHANGE to cryptography
  • Owner changed from tbd to mvngu

comment:2 Changed 7 years ago by ajeeshr

  • Description modified (diff)
  • Milestone changed from sage-4.7.2 to sage-4.7.1

comment:3 Changed 7 years ago by tevinjoseph

well done, keep working

comment:4 Changed 7 years ago by leif

  • Authors changed from ajeesh r to Ajeesh Ravindran
  • Description modified (diff)

comment:5 Changed 7 years ago by nbruin

Just a few quick observations:

  • What is the intended use of the code one included in Sage? If it's for teaching you would probably want to expose more of the details. In fact, for educational purposes it's probably better to do the whole construction "in the open" instead of wrapping it in a class, unless the educational part is wrapping things in classes. For actual cryptographic use, one would probably prefer a whole protocol library. The algorithm is a very small part of deploying cryptography in a secure manner.
  • In your code you call euler_phi(n) to compute the private key d from e. Since the public key is (n,e), anyone could do that same calculation. That means that if it is doable for you to compute the private part of the key, then it is also doable for anyone. You don't have an advantage. (HINT: the key is that euler_phi computes the factorisation of n. If you would make sure that 2p-1 and 2q-1 are actually prime, you would know the factorization of n and hence euler_phi(n). But you should not call euler_phi(n), because that throws away your advantage).

comment:6 Changed 7 years ago by ajeeshr

Thank you nbruin. This was a valid information for me. I will correct it very soon. Keep supporting me in future also

comment:7 Changed 5 years ago by jdemeyer

  • Milestone changed from sage-5.11 to sage-5.12

comment:8 Changed 5 years ago by vbraun_spam

  • Milestone changed from sage-6.1 to sage-6.2

comment:9 Changed 5 years ago by vbraun_spam

  • Milestone changed from sage-6.2 to sage-6.3

comment:10 Changed 4 years ago by vbraun_spam

  • Milestone changed from sage-6.3 to sage-6.4

Changed 4 years ago by peter.story

Major rewrite of the module, with the goal of a pedagogical implementation.

comment:11 Changed 4 years ago by peter.story

I've rewritten ajeeshr's original module, to address nbruin's concerns and to have an implementation that is simpler and better suited for teaching. In summary, I made the following changes:

  • Eliminated calls to euler_phi. These were unnecessary, because we know the primes, and $\phi(pq) = (p-1)*(q-1)$.
  • Made primality checks optional (but enabled by default).
  • Combined the methods that generate public and private keys into a single method, calculate_keys. This is more representative of how actual keygen programs are used.
  • Stopped using Mersenne Primes. The original author seems to have been using them to allow the encoding of large messages, but I don't think this is necessary for a teaching module.
  • Added more accessible references.

comment:12 Changed 4 years ago by kcrisman

Hi! Can you make this a branch on the Trac serve with the git workflow? Also, what connection will this have to the (thus far) only currently implemented public system in that folder in Sage? Finally, should this be globally imported, or imported the way that one is? Thanks!

comment:13 Changed 4 years ago by peter.story

  • Branch set to u/peter.story/rsa_cryptosystem

comment:14 Changed 4 years ago by peter.story

  • Commit set to 4b667369410afa8400b009b8f4f5cc0ad968c78c

In the associated branch, I've added RSACryptosystem to the global namespace. Is there any reason why this is a bad idea?

BlumGoldwasser, the public key system already included with Sage, has a very similar API to RSACryptosystem. There are a few differences:

  • In RSACryptosystem, I combined the public_key and private_key methods into a single calculate_keys method. This is because the exponent e would have had to be calculated independently (and identically) in the two methods. In BlumGoldwasser, the keys can more naturally be calculated independently.
  • I am missing a random_key method. This would be an easy addition, but I'm not sure how valuable it is; for RSACryptosystem it would only need to find two primes, and plug them into calculate_keys.

BlumGoldwasser doc: http://www.sagemath.org/doc/reference/cryptography/sage/crypto/public_key/blum_goldwasser.html

comment:15 Changed 4 years ago by peter.story

  • Authors changed from Ajeesh Ravindran to Peter Story, Ajeesh Ravindran

comment:16 Changed 4 years ago by peter.story

  • Description modified (diff)

comment:17 Changed 4 years ago by leif

  • Milestone changed from sage-6.4 to sage-6.6
  • Status changed from new to needs_review

Presumably rather 6.7, but there's no new milestone yet.


By definition, factoring large primes is exceptionally easy... ;-)

comment:18 Changed 4 years ago by peter.story

  • Description modified (diff)

Haha, good catch! Changed the description to "factoring the product of large primes."

comment:19 Changed 4 years ago by vdelecroix

  • Status changed from needs_review to needs_work
  1. If __init__ is empty, you can just avoid it.
  1. What is the point of having the comparison implemented?! Moreover, the way you did is completely wrong
    + if self.__repr__() == other.__repr__():
    +     return True
    
    The following will return True
    sage: rc1 = RSACryptosystem()
    sage: a = str(rc1)
    sage: rc1 == a
    
  1. In the function encrypt in the section TESTS you repeat many times the definition of rc as well as the computation of the keys. Just remove all these duplication.

comment:20 Changed 4 years ago by nbruin

Before people spend too much time on the code here: Is there a reasonable scenario that justifies having this in the library? We're not going to use it for industrial encryption applications, so I think people think it's for teaching. However, when I teach RSA, the students have to *see* the exponentiation and the construction of the moduli. At that point, the fact that you can do square-and-multiply to do the exponentiation efficiently is already a worthwhile revelation for them.

So having that all wrapped up in library routines is no help at all. They're going to have these things in front of them on a worksheet.

Do people have some lower level scenario in mind where it's sufficient for the students to play around with a crypto "black box"?

I don't mind if it gets included for a good reason, but if the code here isn't going to be used properly and frequently in teaching I'd rather not have it in there, because it will take away from the excitement that students get from implementing their own RSA if there's already such an implementation available in sage.

comment:21 Changed 4 years ago by kcrisman

Trivial comment: a number of things in the documentation are marked as code

``561=3*11*17``

that should probably be marked as math markup

`561=3*11*17`

or something along those lines.


Nils' argument presumably applies to nearly all the stuff in the crypto folder, though, doesn't it? Certainly the only current public-key one.

I guess it depends on whether you are going to ask students to code things like DH or RSA, or only to "do" them. For my own purposes, I think it could be useful to have a well-commented implementation of this (assuming this is in fact a well-commented implementation) that could be used by any given instructor who may not want to/have the expertise to implement something well. (Mathematicians, not programmers.)

That said, I agree that the utility of this is clearly as a pedagogical tool, so a lot of examples of it (and/or why RSA could fail to be particularly secure with some choices of p and q, etc.) along with examples of "brute-force" or other attacks would seem to be the main use of this. So that one can demonstrate such things without having to recode it all from scratch - built-in, as it were.

comment:22 Changed 4 years ago by git

  • Commit changed from 4b667369410afa8400b009b8f4f5cc0ad968c78c to 3c8e43ae07f14dbc6c42f42c5e91cfee5b3f2e45

Branch pushed to git repo; I updated commit sha1. New commits:

3c8e43aCorrected the implementation of ``__eq__``.

comment:23 Changed 4 years ago by peter.story

@vdelecroix, thanks for bringing the __init__ and __eq__ methods to my attention. I copied the implementations from ajeeshr, who had copied them from BlumGoldwasser, the other PublicKeyCryptosystem. The reason BlumGoldwasser implements them is to override the PublicKeyCryptosystem implementations, which is necessary because BlumGoldwasser doesn't initialize its superclass properly (it may be helpful to read the code for the Cryptosystem class's __init__ and __eq__, which can be found on lines 104-212 in cryptosystem.py).

I modified __eq__ to fix the issue you identified. Now, the method just compares the class of the two objects being compared. This is a change that should also be made to BlumGoldwasser.

However, __init__ will be trickier to fix properly. According to the Cryptosystem class, which RSACryptosystem inherits through PublicKeyCryptosystem, I should be passing the following arguments to my superclass's __init__ method: plaintext_space, ciphertext_space, key_space, block_length=1, period=None. Without fixing this, inherited methods like the following will fail:

  sage: rc = RSACryptosystem()
  sage: rc.key_space()
  AttributeError: 'RSACryptosystem' object has no attribute '_key_space'

The following values seem to make the most sense to me, but I'm not certain:

  • plaintext_space = group of all integers
  • ciphertext_space = group of all integers
  • key_space: I have no idea how to specify this one as a group.

Ideally, I would give plaintext_space and ciphertext_space as integers mod n, instead of the group of all integers. However, this would require giving the RSACryptosystem class state, recording public and private keys internally. This would have the effect of making it more opaque, since users wouldn't need to explicitly pass keys to encrypt and decrypt. Thus, I think it makes sense to keep plaintext_space and ciphertext_space as the groups of all integers. Comments on this would be appreciated.

Side note: __init__ will also need to be fixed in BlumGoldwasser:

  sage: from sage.crypto.public_key.blum_goldwasser import BlumGoldwasser
  sage: bs = BlumGoldwasser()
  sage: bs.key_space()
  AttributeError: 'BlumGoldwasser' object has no attribute '_key_space'

Also, as suggested by kcrisman, I corrected several formatting issues with the documentation. Thanks to vdelecroix, I removed unnecessary redeclarations of variables in my tests, except where redeclaring improved readability.

comment:24 Changed 4 years ago by kcrisman

A few comments on this:

  • You can open a separate ticket for the BlumGoldwasser stuff, no need to fix it here.
  • You should use the TestSuite functionality, I guess, to ensure that all such possible errors are caught.
  • One possible option would be to make those spaces be NotImplementedError or something, and then document it as such.
  • Another one is that, considering
    class PublicKeyCryptosystem(Cryptosystem):
        r"""
        The base class for asymmetric or public-key cryptosystems.
        """
    
    is the entirety of the constructor for PublicKeyCryptosystem, and that several of the systems in the crypto/ folder don't even inherit from Cryptosystem, one could just ignore this altogether and not inherit from it if there seems to be no reason to.
  • Or you could use integers.
  • Or you could build it in but then RSACryptosystem(n) would be needed. That's sort of like how the other ciphers require you to put in the alphabet, so it isn't totally farfetched.

comment:25 Changed 4 years ago by peter.story

Of the options you described, I think the best is simply to not inherit from PublicKeyCryptosystem or Cryptosystem. If I inherit from either of them, implementing the plaintext_space, ciphertext_space, etc. would require accepting p and q when an RSACryptosystem object is initialized. However, since this is a pedagogical module, that is undesirable, since I want to keep calculate_keys(p, q) explicit.

Consequently, in this next patch I have removed inheritance from PublicKeyCryptosystem.

comment:26 Changed 4 years ago by git

  • Commit changed from 3c8e43ae07f14dbc6c42f42c5e91cfee5b3f2e45 to c45c4dd7966114639a5d39b1e526729235d14e7a

Branch pushed to git repo; I updated commit sha1. New commits:

c45c4ddStopped inheriting from PublicKeyCryptosystem.
Note: See TracTickets for help on using tickets.