experiments/clixon
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

* Fixed Yang parsing of comments in (extension) unknown statements, to allow multiple white space

Browse source 
* Fixed cli_start_shell: comments and cli_error
* Fixed [making cli_show_options's output more human readable #199](https://github.com/clicon/clixon/issues/199)
* Fixed SSL/evhtp limited read buffer problem
This commit is contained in:
Olof hagsand 2021-04-01 12:41:36 +02:00
parent 0020c7ff3f
commit 2def5d2367
8 changed files with 142 additions and 89 deletions
Download patch file Download diff file Expand all files Collapse all files

3
CHANGELOG.md
View file

@ -84,6 +84,9 @@ Developers may need to change their code
### Corrected Bugs ### Corrected Bugs
* Fixed [making cli_show_options's output more human readable #199](https://github.com/clicon/clixon/issues/199)
* Fixed Yang parsing of comments in (extension) unknown statements, to allow multiple white space
* this also caused spaces to be printed to stdout after clixon-restconf was terminated
* Fixed: [clixon_restconf not properly configed and started by clixon_backend #193](clixon_restconf not properly configed and started by clixon_backend #193) * Fixed: [clixon_restconf not properly configed and started by clixon_backend #193](clixon_restconf not properly configed and started by clixon_backend #193)
* Fixed: [backend start resconf failed due to path string truncated #192](https://github.com/clicon/clixon/issues/192) * Fixed: [backend start resconf failed due to path string truncated #192](https://github.com/clicon/clixon/issues/192)
* Fixed: [state showing error in cli with CLICON_STREAM_DISCOVERY_RFC8040 #191](https://github.com/clicon/clixon/issues/191) * Fixed: [state showing error in cli with CLICON_STREAM_DISCOVERY_RFC8040 #191](https://github.com/clicon/clixon/issues/191)

17
apps/cli/cli_common.c
View file

@ -550,8 +550,7 @@ cli_set_mode(clicon_handle h,
} }
/*! Start bash from cli callback /*! Start bash from cli callback
* XXX Application specific?? * Typical usage: shell("System Bash") <source:rest>, cli_start_shell();
* XXX replace fprintf with clicon_err?
*/ */
int int
cli_start_shell(clicon_handle h, cli_start_shell(clicon_handle h,
@ -567,13 +566,11 @@ cli_start_shell(clicon_handle h,
cmd = (cvec_len(vars)>1 ? cv_string_get(cv1) : NULL); cmd = (cvec_len(vars)>1 ? cv_string_get(cv1) : NULL);
if ((pw = getpwuid(getuid())) == NULL){ if ((pw = getpwuid(getuid())) == NULL){
fprintf(stderr, "%s: getpwuid: %s\n", clicon_err(OE_UNIX, errno, "getpwuid");
__FUNCTION__, strerror(errno));
goto done; goto done;
} }
if (chdir(pw->pw_dir) < 0){ if (chdir(pw->pw_dir) < 0){
fprintf(stderr, "%s: chdir(%s): %s\n", clicon_err(OE_UNIX, errno, "chdir");
__FUNCTION__, pw->pw_dir, strerror(errno));
endpwent(); endpwent();
goto done; goto done;
} }
@ -584,22 +581,20 @@ cli_start_shell(clicon_handle h,
snprintf(bcmd, 128, "bash -l -c \"%s\"", cmd); snprintf(bcmd, 128, "bash -l -c \"%s\"", cmd);
if (system(bcmd) < 0){ if (system(bcmd) < 0){
cli_signal_block(h); cli_signal_block(h);
fprintf(stderr, "%s: system(bash -c): %s\n", clicon_err(OE_UNIX, errno, "system(bash -c)");
__FUNCTION__, strerror(errno));
goto done; goto done;
} }
} }
else else
if (system("bash -l") < 0){ if (system("bash -l") < 0){
cli_signal_block(h); cli_signal_block(h);
fprintf(stderr, "%s: system(bash): %s\n", clicon_err(OE_UNIX, errno, "system(bash)");
__FUNCTION__, strerror(errno));
goto done; goto done;
} }
cli_signal_block(h); cli_signal_block(h);
#if 0 /* Allow errcodes from bash */ #if 0 /* Allow errcodes from bash */
if (retval != 0){ if (retval != 0){
fprintf(stderr, "%s: system(%s) code=%d\n", __FUNCTION__, cmd, retval); clicon_err(OE_UNIX, errno, "system(%s) %d", cmd, retval);
goto done; goto done;
} }
#endif #endif

2
apps/cli/cli_show.c
View file

@ -877,7 +877,7 @@ cli_show_options(clicon_handle h,
fprintf(stdout, "%s: 0x%p , length %zu\n", keys[i], val, vlen); fprintf(stdout, "%s: 0x%p , length %zu\n", keys[i], val, vlen);
} }
else else
fprintf(stdout, "%s: NULL", keys[i]); fprintf(stdout, "%s: NULL\n", keys[i]);
} }
/* Next print CLICON_FEATURE and CLICON_YANG_DIR from config tree /* Next print CLICON_FEATURE and CLICON_YANG_DIR from config tree
* Since they are lists they are placed in the config tree. * Since they are lists they are placed in the config tree.

2
apps/netconf/netconf_main.c
View file

@ -464,7 +464,7 @@ netconf_input_cb(int s,
{ {
int retval = -1; int retval = -1;
clicon_handle h = arg; clicon_handle h = arg;
unsigned char buf[BUFSIZ]; unsigned char buf[BUFSIZ]; /* from stdio.h, typically 8K */
int i; int i;
int len; int len;
cbuf *cb=NULL; cbuf *cb=NULL;

167
apps/restconf/restconf_main_evhtp.c
View file

@ -227,8 +227,28 @@ buf_write(char *buf,
if (ssl){ if (ssl){
if ((ret = SSL_write(ssl, buf, buflen)) <= 0){ if ((ret = SSL_write(ssl, buf, buflen)) <= 0){
clicon_err(OE_SSL, 0, "SSL_write"); int e = errno;
goto done; switch (SSL_get_error(ssl, ret)){
case SSL_ERROR_SYSCALL: /* 5 */
if (e == ECONNRESET) {/* Connection reset by peer */
clicon_debug(1, "%s HERE", __FUNCTION__);
if (ssl)
SSL_free(ssl);
close(s);
clixon_event_unreg_fd(s, restconf_connection);
goto ok; /* Close socket and ssl */
}
else{
clicon_err(OE_RESTCONF, e, "SSL_write %d", e);
goto done;
}
break;
default:
clicon_err(OE_SSL, 0, "SSL_write");
goto done;
break;
}
goto done;
} }
} }
else{ else{
@ -237,6 +257,7 @@ buf_write(char *buf,
goto done; goto done;
} }
} }
ok:
retval = 0; retval = 0;
done: done:
return retval; return retval;
@ -477,6 +498,7 @@ evhtp_params_set(clicon_handle h,
goto done; goto done;
retval = 1; retval = 1;
done: done:
clicon_debug(1, "%s %d", __FUNCTION__, retval);
if (subject) if (subject)
free(subject); free(subject);
if (cvv) if (cvv)
@ -542,6 +564,7 @@ restconf_path_root(evhtp_request_t *req,
goto done; goto done;
retval = 0; retval = 0;
done: done:
clicon_debug(1, "%s %d", __FUNCTION__, retval);
/* Catch all on fatal error. This does not terminate the process but closes request stream */ /* Catch all on fatal error. This does not terminate the process but closes request stream */
if (retval < 0){ if (retval < 0){
evhtp_send_reply(req, EVHTP_RES_ERROR); evhtp_send_reply(req, EVHTP_RES_ERROR);
@ -791,7 +814,7 @@ close_openssl_socket(int s,
* @param[in] ssl if set, it will be freed * @param[in] ssl if set, it will be freed
*/ */
static int static int
accept_badrequest(clicon_handle h, send_badrequest(clicon_handle h,
int s, int s,
SSL *ssl) SSL *ssl)
{ {
@ -814,6 +837,10 @@ accept_badrequest(clicon_handle h,
* @retval -1 Error Terminates backend and is never called). Instead errors are * @retval -1 Error Terminates backend and is never called). Instead errors are
* propagated back to client. * propagated back to client.
* @see restconf_accept_client where this callback is registered * @see restconf_accept_client where this callback is registered
* @note read buffer is limited. More data can be read in two ways: either evhtp returns a buffer
* with 100 Continue, in which case that is replied and the function returns and the client sends
* more data.
* OR evhtp returns 0 with no reply, then this is assumed to mean read more data from the socket.
*/ */
static int static int
restconf_connection(int s, restconf_connection(int s,
@ -822,9 +849,10 @@ restconf_connection(int s,
int retval = -1; int retval = -1;
evhtp_connection_t *conn = NULL; evhtp_connection_t *conn = NULL;
ssize_t n; ssize_t n;
char buf[1024]; char buf[BUFSIZ]; /* from stdio.h, typically 8K */
SSL *ssl; SSL *ssl;
clicon_handle h; clicon_handle h;
int readmore = 1;
clicon_debug(1, "%s", __FUNCTION__); clicon_debug(1, "%s", __FUNCTION__);
if ((conn = (evhtp_connection_t*)arg) == NULL){ if ((conn = (evhtp_connection_t*)arg) == NULL){
@ -832,74 +860,85 @@ restconf_connection(int s,
goto done; goto done;
} }
h = (clicon_handle*)conn->htp->arg; h = (clicon_handle*)conn->htp->arg;
/* Example: curl -Ssik -u wilma:bar -X GET https://localhost/restconf/data/example:x */ while (readmore) {
if (conn->ssl){ readmore = 0;
/* Non-ssl gets n == 0 here! /* Example: curl -Ssik -u wilma:bar -X GET https://localhost/restconf/data/example:x */
curl -Ssik --key /var/tmp/./test_restconf_ssl_certs.sh/certs/limited.key --cert /var/tmp/./test_restconf_ssl_certs.sh/certs/limited.crt -X GET https://localhost/restconf/data/example:x if (conn->ssl){
*/ /* Non-ssl gets n == 0 here!
if ((n = SSL_read(conn->ssl, buf, sizeof(buf))) < 0){ curl -Ssik --key /var/tmp/./test_restconf_ssl_certs.sh/certs/limited.key --cert /var/tmp/./test_restconf_ssl_certs.sh/certs/limited.crt -X GET https://localhost/restconf/data/example:x
clicon_err(OE_XML, errno, "SSL_read"); */
goto done; if ((n = SSL_read(conn->ssl, buf, sizeof(buf))) < 0){
clicon_err(OE_XML, errno, "SSL_read");
goto done;
}
} }
} else{
else{ if ((n = read(conn->sock, buf, sizeof(buf))) < 0){ /* XXX atomicio ? */
if ((n = read(conn->sock, buf, sizeof(buf))) < 0){ /* XXX atomicio ? */ clicon_err(OE_XML, errno, "SSL_read");
clicon_err(OE_XML, errno, "SSL_read"); goto done;
goto done; }
} }
} if (n == 0){
if (n == 0){ clicon_debug(1, "%s n=0 closing socket", __FUNCTION__);
clicon_debug(1, "%s n=0 closing socket", __FUNCTION__); ssl = conn->ssl;
ssl = conn->ssl; conn->ssl = NULL;
conn->ssl = NULL; evhtp_connection_free(conn); /* evhtp */
evhtp_connection_free(conn); /* evhtp */ if (close_openssl_socket(s, ssl) < 0)
if (close_openssl_socket(s, ssl) < 0) goto done;
goto done; goto ok;
goto ok; }
} /* parse incoming packet
/* parse incoming packet * signature:
* signature: */
*/ if (connection_parse_nobev(buf, n, conn) < 0){
if (connection_parse_nobev(buf, n, conn) < 0){ clicon_debug(1, "%s connection_parse error", __FUNCTION__);
clicon_debug(1, "%s connection_parse error", __FUNCTION__); if (send_badrequest(h, s, conn->ssl) < 0)
if (accept_badrequest(h, s, conn->ssl) < 0) goto done;
goto done; SSL_free(conn->ssl);
SSL_free(conn->ssl); if (close_openssl_socket(s, NULL) < 0)
if (close_openssl_socket(s, NULL) < 0) goto done;
goto done; conn->ssl = NULL;
conn->ssl = NULL; evhtp_connection_free(conn);
evhtp_connection_free(conn); goto ok;
goto ok; }
} clicon_debug(1, "%s connection_parse OK", __FUNCTION__);
if (conn->bev != NULL){ if (conn->bev != NULL){
char *buf = NULL; char *buf = NULL;
size_t buflen; size_t buflen;
struct evbuffer *ev; struct evbuffer *ev;
if ((ev = bufferevent_get_output(conn->bev)) != NULL){ if ((ev = bufferevent_get_output(conn->bev)) != NULL){
buf = (char*)evbuffer_pullup(ev, -1); buf = (char*)evbuffer_pullup(ev, -1);
buflen = evbuffer_get_length(ev); buflen = evbuffer_get_length(ev);
if (buflen){ clicon_debug(1, "buf:%s", buf);
if (buf_write(buf, buflen, conn->sock, conn->ssl) < 0) if (buflen){
goto done; if (buf_write(buf, buflen, conn->sock, conn->ssl) < 0)
goto done;
}
else{
#if 1
/* Return 0 from evhtp parser can be that it needs more data.
*/
readmore = 1; /* Readmore */
#else
clicon_debug(1, "%s bev is NULL 1", __FUNCTION__);
if (send_badrequest(h, s, conn->ssl) < 0) /* actually error */
goto done;
#endif
}
} }
else{ else{
clicon_debug(1, "%s bev is NULL 1", __FUNCTION__); clicon_debug(1, "%s bev is NULL 2", __FUNCTION__);
if (accept_badrequest(h, s, conn->ssl) < 0) /* actually error */ if (send_badrequest(h, s, conn->ssl) < 0) /* actually error */
goto done; goto done;
} }
} }
else{ else{
clicon_debug(1, "%s bev is NULL 2", __FUNCTION__); clicon_debug(1, "%s bev is NULL 3", __FUNCTION__);
if (accept_badrequest(h, s, conn->ssl) < 0) /* actually error */ if (send_badrequest(h, s, conn->ssl) < 0) /* actually error */
goto done; goto done;
} }
} } /* while moredata */
else{
clicon_debug(1, "%s bev is NULL 3", __FUNCTION__);
if (accept_badrequest(h, s, conn->ssl) < 0) /* actually error */
goto done;
}
ok: ok:
retval = 0; retval = 0;
done: done:
@ -1057,7 +1096,7 @@ restconf_accept_client(int fd,
switch (e){ switch (e){
case SSL_ERROR_SSL: /* 1 */ case SSL_ERROR_SSL: /* 1 */
clicon_debug(1, "%s SSL_ERROR_SSL (not ssl mesage on ssl socket)", __FUNCTION__); clicon_debug(1, "%s SSL_ERROR_SSL (not ssl mesage on ssl socket)", __FUNCTION__);
if (accept_badrequest(h, s, NULL) < 0) if (send_badrequest(h, s, NULL) < 0)
goto done; goto done;
SSL_free(ssl); SSL_free(ssl);
if (close_openssl_socket(s, NULL) < 0) if (close_openssl_socket(s, NULL) < 0)
@ -1089,6 +1128,8 @@ restconf_accept_client(int fd,
case SSL_ERROR_WANT_CLIENT_HELLO_CB: /* 11 */ case SSL_ERROR_WANT_CLIENT_HELLO_CB: /* 11 */
#endif #endif
break; break;
default:
break;
} }
clicon_err(OE_SSL, 0, "SSL_accept:%d", e); clicon_err(OE_SSL, 0, "SSL_accept:%d", e);
goto done; goto done;
@ -1100,7 +1141,7 @@ restconf_accept_client(int fd,
*/ */
if (restconf_auth_type_get(h) == CLIXON_AUTH_CLIENT_CERTIFICATE && if (restconf_auth_type_get(h) == CLIXON_AUTH_CLIENT_CERTIFICATE &&
SSL_get_peer_certificate(ssl) == NULL) { /* Get certificates (if available) */ SSL_get_peer_certificate(ssl) == NULL) { /* Get certificates (if available) */
if (accept_badrequest(h, s, ssl) < 0) if (send_badrequest(h, s, ssl) < 0)
goto done; goto done;
if (ssl){ if (ssl){
if ((ret = SSL_shutdown(ssl)) < 0){ if ((ret = SSL_shutdown(ssl)) < 0){

1
example/main/example_cli.cli
View file

@ -56,6 +56,7 @@ no("Negate or remove") debug("Debugging parts of the system"), cli_debug_cli((in
debug("Debugging parts of the system"), cli_debug_cli((int32)1);{ debug("Debugging parts of the system"), cli_debug_cli((int32)1);{
level("Set debug level: 1..n") <level:int32>("Set debug level (0..n)"), cli_debug_backend(); level("Set debug level: 1..n") <level:int32>("Set debug level (0..n)"), cli_debug_backend();
} }
shell("System command") <source:rest>, cli_start_shell();
copy("Copy and create a new object") { copy("Copy and create a new object") {
running("Copy from running db") startup("Copy to startup config"), db_copy("running", "startup"); running("Copy from running db") startup("Copy to startup config"), db_copy("running", "startup");
interface("Copy interface"){ interface("Copy interface"){

3
lib/src/clixon_yang_parse.l
View file

@ -200,7 +200,7 @@ identifier [A-Za-z_][A-Za-z0-9_\-\.]*
<UNKNOWN>: { return *yytext; } <UNKNOWN>: { return *yytext; }
<UNKNOWN>; { BEGIN(KEYWORD); return *yytext; } <UNKNOWN>; { BEGIN(KEYWORD); return *yytext; }
<UNKNOWN>\{ { BEGIN(KEYWORD); return *yytext; } <UNKNOWN>\{ { BEGIN(KEYWORD); return *yytext; }
<UNKNOWN>[ \t\n]+ { BEGIN(UNKNOWN2);return SEP; } <UNKNOWN>[ \t\n]+ { BEGIN(UNKNOWN2); return WS; /* mandatory sep for string */ }
<UNKNOWN>[^{"';: \t\n]+ { clixon_yang_parselval.string = strdup(yytext); <UNKNOWN>[^{"';: \t\n]+ { clixon_yang_parselval.string = strdup(yytext);
return CHARS; } return CHARS; }
@ -208,6 +208,7 @@ identifier [A-Za-z_][A-Za-z0-9_\-\.]*
<UNKNOWN2>\" { _YY->yy_lex_string_state =STRING; BEGIN(STRINGDQ); return *yytext; } <UNKNOWN2>\" { _YY->yy_lex_string_state =STRING; BEGIN(STRINGDQ); return *yytext; }
<UNKNOWN2>\' { _YY->yy_lex_string_state =STRING; BEGIN(STRINGSQ); return SQ; } <UNKNOWN2>\' { _YY->yy_lex_string_state =STRING; BEGIN(STRINGSQ); return SQ; }
<UNKNOWN2>\{ { BEGIN(KEYWORD); return *yytext; } <UNKNOWN2>\{ { BEGIN(KEYWORD); return *yytext; }
<UNKNOWN2>[ \t\n]+ { return WS; }
<UNKNOWN2>[^{"'; \t\n]+ { clixon_yang_parselval.string = strdup(yytext); <UNKNOWN2>[^{"'; \t\n]+ { clixon_yang_parselval.string = strdup(yytext);
return CHARS; } return CHARS; }

22
lib/src/clixon_yang_parse.y
View file

@ -57,8 +57,9 @@
%token MY_EOF %token MY_EOF
%token SQ /* Single quote: ' */ %token SQ /* Single quote: ' */
%token SEP /* Separators (at least one) */ %token WS /* white space (at least one) */
%token <string> CHARS %token <string> CHARS
%token <string> ERRCHARS /* Error chars */
%token <string> IDENTIFIER %token <string> IDENTIFIER
%token <string> BOOL %token <string> BOOL
%token <string> INT %token <string> INT
@ -1574,10 +1575,10 @@ unknown_stmt : ustring ':' ustring optsep ';'
if (ysp_add(_yy, Y_UNKNOWN, id, NULL) == NULL) _YYERROR("unknown_stmt"); if (ysp_add(_yy, Y_UNKNOWN, id, NULL) == NULL) _YYERROR("unknown_stmt");
_PARSE_DEBUG("unknown-stmt -> ustring : ustring ;"); _PARSE_DEBUG("unknown-stmt -> ustring : ustring ;");
} }
| ustring ':' ustring SEP string optsep ';' | ustring ':' ustring sep string optsep ';'
{ char *id; if ((id=string_del_join($1, ":", $3)) == NULL) _YYERROR("unknown_stmt"); { char *id; if ((id=string_del_join($1, ":", $3)) == NULL) _YYERROR("unknown_stmt");
if (ysp_add(_yy, Y_UNKNOWN, id, $5) == NULL){ _YYERROR("unknown_stmt"); } if (ysp_add(_yy, Y_UNKNOWN, id, $5) == NULL){ _YYERROR("unknown_stmt"); }
_PARSE_DEBUG("unknown-stmt -> ustring : ustring SEP string ;"); _PARSE_DEBUG("unknown-stmt -> ustring : ustring sep string ;");
} }
| ustring ':' ustring optsep | ustring ':' ustring optsep
{ char *id; if ((id=string_del_join($1, ":", $3)) == NULL) _YYERROR("unknown_stmt"); { char *id; if ((id=string_del_join($1, ":", $3)) == NULL) _YYERROR("unknown_stmt");
@ -1585,7 +1586,7 @@ unknown_stmt : ustring ':' ustring optsep ';'
'{' yang_stmts '}' '{' yang_stmts '}'
{ if (ystack_pop(_yy) < 0) _YYERROR("unknown_stmt"); { if (ystack_pop(_yy) < 0) _YYERROR("unknown_stmt");
_PARSE_DEBUG("unknown-stmt -> ustring : ustring { yang-stmts }"); } _PARSE_DEBUG("unknown-stmt -> ustring : ustring { yang-stmts }"); }
| ustring ':' ustring SEP string optsep | ustring ':' ustring sep string optsep
{ char *id; if ((id=string_del_join($1, ":", $3)) == NULL) _YYERROR("unknown_stmt"); { char *id; if ((id=string_del_join($1, ":", $3)) == NULL) _YYERROR("unknown_stmt");
if (ysp_add_push(_yy, Y_UNKNOWN, id, $5) == NULL) _YYERROR("unknown_stmt"); } if (ysp_add_push(_yy, Y_UNKNOWN, id, $5) == NULL) _YYERROR("unknown_stmt"); }
'{' yang_stmts '}' '{' yang_stmts '}'
@ -1777,6 +1778,8 @@ ustring : ustring CHARS
} }
| CHARS | CHARS
{ _PARSE_DEBUG1("ustring-> CHARS(%s)", $1); $$=$1; } { _PARSE_DEBUG1("ustring-> CHARS(%s)", $1); $$=$1; }
| ERRCHARS
{ _PARSE_DEBUG1("ustring-> ERRCHARS(%s)", $1); _YYERROR("Invalid string chars"); }
; ;
abs_schema_nodeid : abs_schema_nodeid '/' node_identifier abs_schema_nodeid : abs_schema_nodeid '/' node_identifier
@ -1855,10 +1858,19 @@ node_identifier : IDENTIFIER
identifier_ref : node_identifier { $$=$1;} identifier_ref : node_identifier { $$=$1;}
; ;
optsep : SEP /* optsep = *(WSP / line-break) */
optsep : sep
| |
; ;
/* sep = 1*(WSP / line-break)
* Note WS can in turn contain multiple white-space.
* Reason for doing list here is that the lex stage filters comments,
* For example, " // foo\n \t " will return WS WS
*/
sep : sep WS
| WS
;
stmtend : ';' stmtend : ';'
| '{' '}' | '{' '}'