AmiSSL support #3677

chris-y · 2019-03-13T00:01:22Z

This adds a configure option --with-amissl which selects AmiSSL as the SSL engine.
AmiSSL is an Amiga native library which provides a wrapper over OpenSSL.
It also requires all programs using it to use bsdsocket.library directly, rather than accessing socket functions through clib, which libcurl was not necessarily doing previously. Configure will now check for the headers and ensure they are included if found.

Because of the way 68k Amiga functions are called, assigning a library function to a variable does not work correctly, so we create a dummy function to workaround this problem. This also affects the calling of MD5 functions. This only affects Amiga OS3 libraries, not the new OS4 library interface, but the workaround is harmless there.

…tattr.

…e internal Curl MD5 functions instead if we're using AmiSSL.

…id mixing bsdsocket and C library functions

More specifically, even though GCC is built without any trace of ixemul, the defines are still present and everything breaks.

Require this header in order to enable AmiSSL support If it is available, use it. This will bypass any C lib interface. Note that the code in amigaos.c which opens/closes bsdsocket.library should not really be used - this should be opened in user code. It has been left in for compatibility but probably should be removed.

…them with macros

bagder · 2019-03-13T07:35:30Z

It seems to be lacking some #ifdefs or macro definitions:

easy.c: In function ‘curl_global_cleanup’:
easy.c:276:3: error: implicit declaration of function ‘Curl_amiga_cleanup’; did you mean ‘Curl_digest_cleanup’? [-Werror=implicit-function-declaration]
   Curl_amiga_cleanup();
   ^~~~~~~~~~~~~~~~~~
   Curl_digest_cleanup
easy.c:276:3: error: nested extern declaration of ‘Curl_amiga_cleanup’ [-Werror=nested-externs]

chris-y · 2019-03-13T09:30:02Z

Oops, looks like I cocked up the #ifdefs by nesting them.
Should be something more like #if defined(__AMIGA__) && defined(HAVE_PROTO_BSDSOCKET_H) && !defined(USE_AMISSL)
I'll correct this later today.

bagder · 2019-03-13T14:26:03Z

configure.ac

+  AC_SUBST(HAVE_PROTO_BSDSOCKET_H, [1])],
+  [],
+  []
+)


And this header check can't be put among all the other headers that are checked?

I put it there as it needs to be resolved before any of the socket functions are checked, due to required added headers like the winsock check above it.
There's a case for moving it down to the gethostbyname section and making it part of the bsdsocket.library check there (I can't see any other socket lib function checks above this point). That would also confirm that functions from the library are working as expected which would be a useful additional test before it gets enabled.

I've now moved this to be part of the "gethostbyname is in bsdsocket.library" check.

bagder · 2019-03-13T14:27:42Z

lib/vtls/vtls.c

@@ -248,7 +248,7 @@ Curl_ssl_connect(struct connectdata *conn, int sockindex)
  conn->ssl[sockindex].use = TRUE;
  conn->ssl[sockindex].state = ssl_connection_negotiating;

-  result = Curl_ssl->connect(conn, sockindex);
+  result = Curl_ssl->connect_blocking(conn, sockindex);


This isn't gcc's fault, this is some silly headers you use that seems to implement some of the socket API functions as macros and thus banning the function names from use everywhere. I'd say that is pretty bad style but I also realize you're not to blame for that...

Yes, it's the bsdsocket inline header. All Amiga 68k libraries are called using macros like that, the macros set the values in the correct registers then jump to the function code. Unfortunately not much I can do about that, it was a design decision taken ~35 years ago!

bagder · 2019-03-14T08:31:00Z

I would imagine that it should keep including the "mandatory" three last include files (in the same order) to reduce the risk of memory screwups when built debug-enabled. That's why test 1132 fails:

 ./../lib/amigaos.c doesn't include "memdebug.h"
 ./../lib/amigaos.c doesn't include "curl_memory.h"

bagder · 2019-03-14T08:48:58Z

src/tool_operate.c

-          SetComment(outs.filename, url);
+          if(strlen(urlnode->url) > 78)
+            urlnode->url[79] = '\0';
+          SetComment(outs.filename, urlnode->url);


Thinking of this code path: shouldn't this rather be done if the --xattr option is used, like it works for other platforms? That code path also scrubs off any username + password that might exist in the URL, while this amiga-specific block doesn't.

(No, this isn't really related to this particular change but it struck me now when I saw that this code is updated which means it is used...)

I agree, although I can't see where the code needs moving to, unless it is literally a few lines up under the /* Set file extended attributes */ comment?

That's what I had in mind, yes. But I'm sorry, that's a separate issue and should be done separately from this PR. I just happened to notice since you fixed that code now.

…bsdsocket.library

bagder · 2019-03-15T09:00:14Z

The red CI builds look like false positives.

bagder · 2019-03-15T09:27:24Z

Thanks! I did some edits and squashes before merge, but I hope I didn't ruin anything! =)

chris-y added 9 commits March 8, 2019 00:06

Add option --with-amissl

dbd2cc4

Fix tool_operate.c build on AmigaOS

2f9afa9

A workaround for having termios.h on AmigaOS 3, but no tcgetattr/tcse…

049965d

…tattr.

AmigaOS 3 library functions can't be assigned to variables, so use th…

f859a3d

…e internal Curl MD5 functions instead if we're using AmiSSL.

Tidy up USE_AMISSL, avoid multiple openings of bsdsocket.library, avo…

1bd3e56

…id mixing bsdsocket and C library functions

ixemul needs to die

6a02785

More specifically, even though GCC is built without any trace of ixemul, the defines are still present and everything breaks.

Rename some parts of the SSL function structure as GCC was confusing …

ba106b1

…them with macros

ghost approved these changes Mar 13, 2019

View reviewed changes

bagder reviewed Mar 13, 2019

View reviewed changes

Fix #ifdefs

9e9d24a

bagder reviewed Mar 14, 2019

View reviewed changes

chris-y added 4 commits March 14, 2019 19:39

Add missing mandatory includes

edb0c7a

Check for proto/bsdsocket.h as part of checking for gethostbyname in …

83a4317

…bsdsocket.library

Remove tab

5242b93

Formatting

9d87471

bagder closed this in 1e85365 Mar 15, 2019

captain-caveman2k mentioned this pull request Apr 14, 2019

Centralise the TLS specific MD4 code away from the NTLM code #3780

Closed

lock bot locked as resolved and limited conversation to collaborators Jun 13, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

AmiSSL support #3677

AmiSSL support #3677

chris-y commented Mar 13, 2019

bagder commented Mar 13, 2019

chris-y commented Mar 13, 2019

bagder Mar 13, 2019

chris-y Mar 13, 2019

chris-y Mar 14, 2019

bagder Mar 13, 2019

chris-y Mar 13, 2019

bagder commented Mar 14, 2019

bagder Mar 14, 2019

chris-y Mar 14, 2019

bagder Mar 15, 2019

bagder commented Mar 15, 2019

bagder commented Mar 15, 2019

AmiSSL support #3677

AmiSSL support #3677

Conversation

chris-y commented Mar 13, 2019

bagder commented Mar 13, 2019

chris-y commented Mar 13, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bagder commented Mar 14, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bagder commented Mar 15, 2019

bagder commented Mar 15, 2019