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

Es netssl #1

Open
wants to merge 19 commits into
base: master
Choose a base branch
from
Open

Es netssl #1

wants to merge 19 commits into from

Conversation

jocelyn
Copy link
Owner

@jocelyn jocelyn commented Feb 26, 2018

for review

jvelilla and others added 14 commits January 10, 2018 16:20
Added example showing encrypt- decrypt using aes gcm 256 based on OpenSSL
example.
Added test case gcm decrypt
Merge openssl.ecf and test into openssl.ecf.
Splitted the APPLICATION.make feature.
Removed a few warnings.
Made BYTE_ARRAY_CONVERTER as hidden (implementation) class.
… low level

code wrapping C code.
Initial commit gcm encrypt (work in progress).
Added new interfaces to represent the different modes and algorithm supported
by OpenSSL and a generic way to encrypt/decrypt.
Added ERROR abstractions to handle OpenSSL errors.
Updated low level api to handle errors at the moment using Exceptions.
Added test cases to show AES with GCM mode encrypt/decrypt.
… into es_openssl_crypto

# Please enter a commit message to explain why this merge is necessary,
# especially if it merges an updated upstream into a topic branch.
#
# Lines starting with '#' will be ignored, and an empty message aborts
# the commit.
SSL class
Added `ssl_get_error` to get the result code of for a preceding call to SSL_connect(), SSL_accept(), SSL_do_handshake(), SSL_read(), SSL_peek(), or SSL_write() on ssl.
Added externals features to get the value of different error codes:
SSL_ERROR_WANT_READ, SSL_ERROR_NONE, SSL_ERROR_ZERO_RETURN, SSL_ERROR_WANT_WRITE
Added external feature `ssl_pending` to return the number of bytes with are available
inside SSL for inmediate read.

Updated NetSSL library to support non-blocking reads and writes.
Added a non blocking client/server example.
Copy link
Owner Author

@jocelyn jocelyn left a comment

Choose a reason for hiding this comment

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

Major issue related to visibility of BYTE_ARRAYED_CONVERTER.
Many functions should be renamed to follow Eiffel rules.

and a few cosmetic points.

@@ -1,7 +1,7 @@
<?xml version="1.0" encoding="ISO-8859-1"?>
<system xmlns="http://www.eiffel.com/developers/xml/configuration-1-17-0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://www.eiffel.com/developers/xml/configuration-1-17-0 http://www.eiffel.com/developers/xml/configuration-1-17-0.xsd" name="openssl" uuid="22EE78DC-1AC1-4EBC-8AA9-9E8D6B8C3989" library_target="openssl">
<target name="openssl">
<description>Eiffel OpenSSL Wrap library.
<description>Eiffel OpenSSL Wrap library.
Copyright (c) 1984-2006, Eiffel Software and others.
Copy link
Owner Author

Choose a reason for hiding this comment

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

copyright year should be updated

Choose a reason for hiding this comment

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

done.

verify_key_size: BOOLEAN
-- has the current algorithm a valid key size?
do
if key_sizes.has (key_size) then
Copy link
Owner Author

Choose a reason for hiding this comment

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

Result := key_sizes.has (key_size)

Choose a reason for hiding this comment

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

done

Result := ctx.update (a_data)
end

update_into(a_data, a_buf: MANAGED_POINTER): INTEGER
Copy link
Owner Author

Choose a reason for hiding this comment

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

bad indentation, and missing space before (

Choose a reason for hiding this comment

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

done.

else
updated := True
bytes_processed := bytes_processed + a_data_size
-- if bytes_processed > ctx.mode.MAX_ENCRYPTED_BYTES then
Copy link
Owner Author

Choose a reason for hiding this comment

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

either add a comment related to the commented lines, or remove those lines

Choose a reason for hiding this comment

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

done


feature -- Access

ctx: SSL_CIPHER_CONTEXT_EXTERNALS
Copy link
Owner Author

Choose a reason for hiding this comment

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

should ctx be protected (export NONE) and renamed implementation or externals ?

Choose a reason for hiding this comment

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

yes, I will export it to {NONE}, but I like the name ctx or context since it's more natural in this domain.

"return SSL_pending((const SSL *)$a_ssl);"
end


note
copyright: "Copyright (c) 1984-2016, Eiffel Software and others"
Copy link
Owner Author

Choose a reason for hiding this comment

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

update copyright year.


feature -- Errors

retrieve_errors: detachable LIST [SSL_ERROR]
Copy link
Owner Author

Choose a reason for hiding this comment

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

bad name for a function

do

-- Create and initialise the context
l_ctx := {SSL_EVP}.c_evp_cipher_ctx_new
Copy link
Owner Author

Choose a reason for hiding this comment

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

SSL_EVP is in folder "to_remove" ... please also move related tests to a folder "to_remove".
As I guess this is useless to review code that is about to be removed.

create l_mode.make ("8b23299fde174053f3d652ba", Void)
create cipher.make (l_algo, l_mode)
l_encryptor := cipher.encryptor
l_computed_ct := l_encryptor.update (create {MANAGED_POINTER}.make_from_array ((create {BYTE_ARRAY_CONVERTER}.make_from_hex_string ("28286a321293253c3e0aa2704a278032")).to_natural_8_array))
Copy link
Owner Author

Choose a reason for hiding this comment

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

Note that BYTE_ARRAY_CONVERTER is for now hidden (as implementation).
So user won't be able to use it directly.
then, the interface should have several version of update ... update_with_string, update_with_hexadecimal_string and so on ...

Copy link
Owner Author

Choose a reason for hiding this comment

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

I am not sure if l_computed_ct as a MANAGED_POINTER is really user friendly.
Could we have higher abstraction?
maybe the l_encryptor should keep an attribute with the internal computed result after update ...

I think this is what we have in crypto lib for HMAC_SHA256 and related.

@@ -0,0 +1,6 @@
#include <windows.h>
Copy link
Owner Author

Choose a reason for hiding this comment

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

do not commit generated .rc files

jvelilla added 5 commits March 1, 2018 20:55
Clean Code.
Updated SSL_CIPHER_INTERFACE's to follow Eiffel Style.
Refactor rename features to follow Eiffel Style.
…tion

Clean code.
Remove obsolete cluster: to_remove.
Updated OpenSSL dynamic ecf to use the latest dynamic library.
Updated OpenSSL readme with information about how to compile it (generate static and dynamic library) and how
to install it on Ubuntu (Debian) Linux.
Updated Delivery with the latest OpenSSL libraries for Visual Studio C++ win64 and win32
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.

2 participants