• Post Reply Bookmark Topic Watch Topic
  • New Topic
programming forums Java Mobile Certification Databases Caching Books Engineering Micro Controllers OS Languages Paradigms IDEs Build Tools Frameworks Application Servers Open Source This Site Careers Other all forums
this forum made possible by our volunteer staff, including ...
Marshals:
  • Campbell Ritchie
  • Liutauras Vilda
  • Bear Bibeault
  • Paul Clapham
  • Jeanne Boyarsky
Sheriffs:
  • Devaka Cooray
  • Junilu Lacar
  • Tim Cooke
Saloon Keepers:
  • Tim Moores
  • Ron McLeod
  • Tim Holloway
  • Claude Moore
  • Stephan van Hulst
Bartenders:
  • Winston Gutkowski
  • Carey Brown
  • Frits Walraven

Network socket for sending info to server object (encrypted)  RSS feed

 
Greenhorn
Posts: 8
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Hello all, this is my first post on this forum.
Iv been coding by my self for about 6 months learning just from youtube and messing about in eclipse. Seems easy enough.
Anyways, here i have a object for sending information to a server with encryption. Iv done it this way to have different types "data blocks" be send with a pre set command
This is the 4th time i have re-writen this before this i tried to do it without objects and without threads. This is built for both
Any kind harted person want to look threw it and see if there is room for improvements?

 
Tyler McClain
Greenhorn
Posts: 8
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator

Tyler McClain wrote:Hello all, this is my first post on this forum.
Iv been coding by my self for about 6 months learning just from youtube and messing about in eclipse. Seems easy enough.
Anyways, here i have a object for sending information to a server with encryption. Iv done it this way to have different types "data blocks" be send with a pre set command
This is the 4th time i have re-writen this before this i tried to do it without objects and without threads. This is built for both
Any kind harted person want to look threw it and see if there is room for improvements?



Just relized to pasted it without the try catch thing. lol, im tired.

 
Saloon Keeper
Posts: 9857
199
  • Likes 2
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
  • Package names should start with a lower-case letter.
  • Use nouns for class names, e.g. Sender instead of Sending.
  • Use verbs for method names, e.g. send() instead of socketsMain().
  • Make classes package private unless you have reason to use them outside the package.
  • Make classes final unless you have REALLY good reason to extend them.
  • Make fields final, unless you have good reason to change their values after object creation.
  • Don't provide setters for all properties. Usually it's better if you can only set fields in the constructor.
  • Don't use fields if the data is only relevant for one method and you can pass that data as a method argument, e.g. make toSend a parameter of send().
  • As formal types of variables, use the least specific type that still does what you need, e.g. instead of ArrayList use List or even Collection.
  • Use the diamond operator when the generic type argument can be inferred from the context, e.g. new ArrayList<>().
  • Use appropriate types for the concepts that variables represent, e.g. InetAddress instead of String + int.
  • Don't declare constructor parameters if you're not going to use them.
  • Perform parameter checking in your constructors and methods. Throw exceptions when arguments aren't valid.
  • Write identifiers out, e.g. lastReceivedId.
  • Use try-with-resources instead of manually closing your sockets/writers/streams.
  • Use the enhanced for-loop instead of a loop with a counter.
  • Don't print stack traces. Let exceptions bubble up the call stack (optionally wrapped in another exception of an appropriate type) so that calling code can deal with it properly.

  • I'd really like to see your Encryption class. The fact that the encrypt() method is static and doesn't accept a key worries me. Encryption is also notoriously difficult to get right, so there are probably some important improvements you can make there.

    After you've made changes to your code, show us the result and we can give you more tips.
     
    Tyler McClain
    Greenhorn
    Posts: 8
    • Mark post as helpful
    • send pies
    • Quote
    • Report post to moderator

    Stephan van Hulst wrote:
    I'd really like to see your Encryption class. The fact that the encrypt() method is static and doesn't accept a key worries me. Encryption is also notoriously difficult to get right, so there are probably some important improvements you can make there.



    i have them split, cant remember why i did it this way.
    here is encryption class




    aaand decryption class


    After reading some feed back i got on another post i made after this, just now relizing how badly setup this is, getting the key from a static string... boi
    Anyways i have to do so reasearch on some of the good points youv made so far, and will figure out how to do so. Thanks
     
    Tyler McClain
    Greenhorn
    Posts: 8
    • Mark post as helpful
    • send pies
    • Quote
    • Report post to moderator
    Heyo, trying to figure out how to use enhanced for loop, using it instead of a foorloop with counter.
    Before the forloop with counter, the array list just used the counter to get the arraylist index
    but without a counter i would have to make a int that counts to do it. did i miss something?

     
    Tyler McClain
    Greenhorn
    Posts: 8
    • Mark post as helpful
    • send pies
    • Quote
    • Report post to moderator
    So i did learn more about objects. I dont know why i thought you could just create one object and then just update the "toSend" array when needed before sending... which is what i was trying to do.
    This is what is going on in the GUI when the butten gets pushed, just gets from fields adds it to array and it sends the data



    uses a thread to keep the gui from locking up.
     
    Stephan van Hulst
    Saloon Keeper
    Posts: 9857
    199
    • Mark post as helpful
    • send pies
    • Quote
    • Report post to moderator
    With enhanced for-loops, the loop variable already holds the value you're interested in, so you don't have to get it from the array anymore.

    Don't use the Thread class directly, and *definitely* don't extend Thread to create a background task. For Swing background tasks, use SwingWorker. For other background tasks, submit a Runnable or a Callable to an ExecutorService.

    In your UI, use a field of type Action to represent the actions that your user interface can perform:

    You have some grave mistakes in your Encryption class. I will address those in a later post.
     
    Stephan van Hulst
    Saloon Keeper
    Posts: 9857
    199
    • Mark post as helpful
    • send pies
    • Quote
    • Report post to moderator
    As promised, part two.

    There are three massive flaws with your encryption code. First of all, simply encoding a password with some character set to get key material is a really poor substitute for a key derivation algorithm. You need to use something proper, like bcrypt or PBKDF2.

    Secondly, using your key material as the initialization vector destroys the entire purpose of the IV, and may even expose your key to an attacker. The idea behind the IV is that you generate it randomly every time, so that the output of your encryption step is random every time. The IV is not secret, so you can just append it to your encrypted message so the receiving end can reuse it to decrypt the message again.

    Finally, it doesn't look like you're authenticating your messages before you decrypt them. You really should use an authenticated encryption scheme, such as using AES in Galois Counter Mode.

    There are some other issues to deal with:

  • Don't put all your constants in one class who's only purpose is to act as a container for those constants. Put constants in the classes where they're used.
  • In your decryption step, why do you encode the string to UTF-8, only to decode it as Base64? Base64.Decoder has a decode() method that takes a string.
  • Catch specific exception types, and know what to do in each case. Definitely don't print out every exception you get when encrypting/decrypting, because it may leak sensitive information.
  •  
    • Post Reply Bookmark Topic Watch Topic
    • New Topic
    Boost this thread!