There really isn't all that much to see here when it comes to cryptography. Basically this is a wrapper class that tries to wrap the knowledge of the person to writethat wrote it (that's you I suppose) rather than to implement security for a specific use case. In such a sense, it is only useful for practice purposes.
Never ever create such a class and make it the center of your security strategy (I've been there, steered there by "professionals", took me weeks to recover from it a couple of years later).
I'll just focus on the security parts:
- a string is not a key, if you want to use a password, use a Password Based Key Derivation Mechanism such as PBKDF2 (and keep in mind that most passwords are insecure even if you do take that measure) - a number is certainly not a great password;password);
- just
"AES"
will default to the insecure ECB mode for most if not all Java / provider implementations - it is OK for the key creation, but not for the "transformation" parameter inCipher.getInstance(String transformation): Cipher
; - all other modes require an IV for generic messages, as otherwise identical messages (or parts of messages) may leak data - and even all data in the case of counter mode encryption;
- nowadays authenticated ciphers such as GCM should be used; this makes sure that an adversary cannot alter the ciphertext undetected by the receiver - such alterations may even threaten the confidentiality of the message (see plaintext / padding oracle attacks);
- using
pValue.getBytes()
is dangerous asgetBytes
will default to platform encoding, and the platform encoding may differ per platform (Windows uses Windows-1252 and Linux / Android uses UTF-8, to name just one such difference); - try e.g.pValue.getBytes(StandardCharsets.UTF_8)
instead; - just throwing security exceptions doesn't make sense, as it doesn't distinguish between runtime issues (algorithm not available) and input / output issues - see here how to handle security exceptions;
The first thing I do in my IDE is to change e.printStackTrace()
to new IllegalStateException("No exception yet specified", e);
, usually with a // TODO change exception
comment in front of it (picked up by the task system in Eclipse).
The problems with just printing the stack trace is that it is easily forgotten, it keeps running the code even after an exception is turned up and finally, it messes up the compilers detection of dead code.
CRY_ALGO
of course is a cry-out for a better name.